Phase A: artifact-backed commits + the sole owned Lance writer (supersedes the #911 cycle contract) - #912
Merged
Merged
Conversation
…riter Implements the canonical persistence contract (operator ruling 2026-08-09), superseding the cycle-persistence model merged in #911. THE GOVERNING RULE. No artifact-backed semantic change -> no write -> no new DatasetVersion. Thinking is cheaper than persisting its intermediate control flow: a thought runs its whole Rubicon ladder transiently and only a semantic artifact becomes durable; kanban progress rides along in the commit that was happening anyway. Mechanically the gate is the cast payload — non-empty is an artifact (persisted), empty is intent-only (ephemeral). persist_cycle partitions intent-only casts out before the freeze, so zero artifact casts yields CommitOutcome::NoChange with the sink never called. restage_held's empty payload is therefore the ephemerality mechanism, not a gate violation to pad around; the deliberate empty-cycle versioning is removed and its falsifier inverted. ONE LOGICAL WRITER, SPLIT BY CAPABILITY. Producer submission and read-only projections keep Clone + &self (fire-and-forget: producers get no acknowledgement). The concrete LanceCycleWriter is non-Clone, owns a long-lived Dataset handle plus an in-memory head, and commits through &mut self — two application commits cannot interleave through the type boundary, and the sole writer fully honors every result. NO ROLLBACK, NO COMPENSATING DELETE. A published manifest is history; Dataset::delete mints another version and is not rollback (and #911's (cycle, base_version) predicate could destroy a concurrent same-cycle winner). Measured: lance 9 has no atomic expected-version fence for Append — the rebase runs even single-attempt, strict mode is Overwrite-only (lance-9.0.0/src/io/commit.rs:914-950) — stated rather than papered over. Idempotency is durable and in-band: (cycle, batch_hash) commits with the rows and is reconciled FIRST, so re-submitting the same frozen batch after a lost acknowledgement returns Reconciled instead of double-appending. Honest states: NoChange / Committed / Reconciled, and Fenced / HashConflict / Io / Ambiguous. No error promises "nothing landed" when failure could follow publication. ZERO RELOAD ON THE NORMAL PATH, INSTRUMENTED. opens() counts every Dataset::open (startup + ambiguity resolution only). Reads are bounded (scan_sealed takes an after_cycle bound pushed into the scan; recover_fleet takes it too) and projected (timeline never touches the payload column; landing rows carry no payload). Layout: frame row, landing-metadata rows, and coalesced image rows — 64 transient breaths on one row cost 512 durable bytes, not 64 x 512, measured by a falsifier. Gates: 11 reopened-store falsifiers in cycle_sink, 5 contract falsifiers in persist_sink, 20 driver tests green, planner suite 353 green, fmt + clippy clean on default features. Honestly deferred: the credentialed object-store RUN (the aws feature is already active via lance's default features, so s3:// compiles and routes — the measurement is what is missing, and no durability claim is made until it runs); true zero-copy (the Arrow-materialization copy boundary is documented and isolated); Phases B-F. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KCGhDYoQBXs3poaR7sFuqp
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c01cca78-2ac6-450c-9b41-6ceb763ed7f1) |
|
Warning Review limit reached
Next review available in: 34 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe PR changes cycle persistence from version-based commits to artifact-gated commits. It adds ChangesArtifact-backed cycle persistence
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The d_ign_b_lenses and probe_ignition harnesses each carry their own in-memory WalSink. Both are migrated to the reshaped trait: commit_cycle takes &mut self and only the batch, reconciles (cycle, batch_hash) BEFORE appending (Reconciled on a matching retry, HashConflict fail-closed), fences against the stored head with Fenced, and returns CommitOutcome; scan_sealed takes the after_cycle bound and yields cycle-keyed LandedSlots; versions() becomes timeline() -> Vec<FrameMeta>. Landed separately from a4697b7 because the migration completed after that commit was staged. Both probes pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KCGhDYoQBXs3poaR7sFuqp
Formatting-only follow-up to 6f3a8f5 (import wrapping in the three probe files). Both probes still pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KCGhDYoQBXs3poaR7sFuqp
…both ways) The Phase A bound was threaded through recover_fleet but every existing test passes None, so nothing proved it bites — a parameter that changes nothing is decoration (CLAUDE.md falsifiability rule: a bound needs an inertness test in both directions). The new test drives ONE durable history twice: bounded past the landing's cycle, nothing replays and the owner stays at Planning (EXCLUDES); bounded below it, the same landing replays and the owner advances (ADMITS). So the assertion cannot be satisfied by an implementation that ignores the bound. Mutation-tested in session: replacing scan_sealed(after_cycle) with scan_sealed(None) fails this test and only this test; restored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KCGhDYoQBXs3poaR7sFuqp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4697b7318
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Field::new("kind", DataType::UInt8, false), | ||
| Field::new("cycle", DataType::UInt64, false), | ||
| Field::new("base_version", DataType::UInt64, false), | ||
| Field::new("batch_hash", DataType::UInt64, false), |
There was a problem hiding this comment.
When this writer opens a dataset created by the preceding LanceCycleSink, Dataset::open succeeds but the stored schema has no batch_hash column and uses a non-null Binary payload. The first commit then fails in find_frame while projecting batch_hash, and timeline and appends are likewise incompatible, so upgrading makes every existing cycle store unreadable and unwritable. Add an explicit migration/compatibility path or use a versioned dataset location before requiring the new schema.
Useful? React with 👍 / 👎.
Comment on lines
+833
to
+834
| let sealed: Vec<LandedSlot> = sink | ||
| .scan_sealed(after_cycle) |
There was a problem hiding this comment.
When a bounded recovery includes a landing for an owner that is absent from fleet_ids or not yet registered, the landing is partitioned and then silently ignored while the function still returns success. A caller can consequently persist the supplied tail's highest cycle as its next after_cycle; if that owner registers later, scan_sealed(after_cycle) excludes its unapplied move forever unless the caller performs an otherwise-forbidden full-history scan. Return the unprocessed owner/cycle information or maintain per-owner recovery bounds so the global checkpoint cannot skip such tails.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/lance-graph-planner/src/persist_sink.rs (2)
460-467: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the
source()doc: there are now three wrapping variants.Line 467 adds
Commit(e) => Some(e). The doc comment above still says "the two wrapping variants (Write/Illegal)".📝 Proposed fix
/// Expose the wrapped cause so callers can walk the error chain — the two - /// wrapping variants ([`Write`](PersistError::Write) / - /// [`Illegal`](PersistError::Illegal)) forward to their inner error; the - /// self-describing variants have no deeper cause. + /// Expose the wrapped cause so callers can walk the error chain — the three + /// wrapping variants ([`Write`](PersistError::Write) / + /// [`Commit`](PersistError::Commit) / [`Illegal`](PersistError::Illegal)) + /// forward to their inner error; the self-describing variants have no + /// deeper cause.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph-planner/src/persist_sink.rs` around lines 460 - 467, Update the documentation above PersistError::source to describe all three wrapping variants, adding Commit alongside Write and Illegal; keep the implementation unchanged.
1478-1494: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename
a_fenced_cycle_writes_nothing: it assertsIo, notFenced.The test builds
FakeWalSink::failing()and assertsErr(PersistError::Commit(CommitError::Io(_))).CommitError::Fencedis never produced here. The two errors carry different contracts:Iomeans nothing was published and the caller may regenerate, whileFencedmeans the head moved and the caller must rebase. The real fence is covered bya_cycle_reads_the_sealed_predecessor_not_an_in_flight_sibling.The plan file lists falsifiers by name, so an accurate name matters.
📝 Proposed fix
#[tokio::test] - async fn a_fenced_cycle_writes_nothing() { + async fn a_failed_commit_publishes_nothing_and_takes_no_step() { let mut sink = FakeWalSink::failing();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph-planner/src/persist_sink.rs` around lines 1478 - 1494, Rename the test function a_fenced_cycle_writes_nothing to accurately describe the failing WAL I/O scenario, such as a_cycle_writes_nothing_on_io_error. Keep its assertions and setup unchanged, and ensure any plan or falsifier references use the new test name.crates/lance-graph-supervisor/src/cycle_driver.rs (1)
810-836: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd coverage for a non-
Noneafter_cycle, and state the safety condition that couples it to the watermark.Every call site in this file's tests passes
None(lines 1323, 1357, 1522, 1863, 1871, 1882). The new bounded path is therefore untested here, even though it is the stated contract.The bound also interacts with the per-owner watermark in a way the doc does not make explicit.
scan_sealed(Some(c))drops every landing at or below cyclecbeforerecover_and_applyever sees it. If a caller passes a cycle higher than what some owner actually applied, that owner's pending landings are skipped permanently and its watermark stays stale. A later pass from a lower bound then fails withPersistError::StalePhase. The safety condition is thatafter_cyclemust be at or below the lowest cycle fully applied across every owner infleet_ids, not the highest cycle sealed.Add a test that seals cycles 1 through 3, recovers with
Some(CycleId(1)), and asserts only the cycle-2 and cycle-3 landings replay. Also add the safety condition to the doc comment.As per coding guidelines: "Add Rust unit tests alongside implementations via
#[cfg(test)]modules; prefer focused scenarios over broad integration tests".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph-supervisor/src/cycle_driver.rs` around lines 810 - 836, Update the `recover_fleet` documentation to state that `after_cycle` must not exceed the lowest cycle fully applied by every owner in `fleet_ids`, since skipped landings cannot be recovered later. In the nearby `#[cfg(test)]` module, add a focused test that seals cycles 1 through 3, invokes `recover_fleet` with `Some(CycleId(1))`, and verifies only cycles 2 and 3 are replayed.Source: Coding guidelines
crates/lance-graph/src/graph/cycle_sink.rs (1)
124-144: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAdd an explicit version gate before appending to the cycle store.
The store may be opened from any existing dataset and then appended with
cycle_store_schema(), which adds non-nullbatch_hashand changespayloadtoFixedSizeBinary(512). An older store without this layout will fail at append with a raw schema error, and reconciliation paths that projectbatch_hashwill also fail. Use a schema metadata version key or a dedicated version column, write it for the new layout, check it on open, and fail with a migration-ready error for older stores.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph/src/graph/cycle_sink.rs` around lines 124 - 144, Update cycle_store_schema and the cycle-store open/append path to define and persist an explicit layout version, then validate that version before any append or reconciliation projection uses batch_hash. Reject older or missing-version stores with a clear migration-ready error instead of allowing a raw schema mismatch.Source: Coding guidelines
🧹 Nitpick comments (5)
crates/lance-graph-planner/src/persist_sink.rs (2)
513-520: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
after_cyclebounds cycles, not rows.
scan_sealedreturnsVec<LandedSlot>fully materialized.after_cyclelimits how many cycles are scanned, but a single cycle can carry one landing per artifact cast, and the plan targets a 64k-owner fleet.cycle_driver::recover_fleetthen copies the whole result into aHashMap<MailboxId, Vec<LandedSlot>>, so peak memory is roughly two copies of the tail.The plan defers 64k-scale measurement, so this is not a blocker for Phase A. Consider recording the row-count bound as a named deferral on this method, next to the cycle bound, so a later caller does not read "bounded" as "bounded in memory". A streaming return type is the eventual fix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph-planner/src/persist_sink.rs` around lines 513 - 520, The scan_sealed contract currently documents only the cycle bound, not the potentially unbounded materialized row count. Add a named deferral beside the existing after_cycle documentation stating that row-count/memory bounding is deferred and that streaming is the eventual fix; preserve the current Vec<LandedSlot> return type and behavior.
1576-1644: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a falsifier for
CommitError::Ambiguous.The new tests cover
NoChange,Committed,Reconciled,Fenced,Io, andHashConflict.Ambiguoushas no gate, and it carries the strongest safety claim in the contract: "Re-submit the SAME frozen batch:commit_cyclereconciles first, so the retry cannot double-append." Nothing proves that a re-submit afterAmbiguousreconciles instead of appending a second time.
FakeWalSinkcannot produceAmbiguoustoday. Add an injection flag that records the commit durably and then returnsAmbiguous, which is exactly the real hazard: the append landed but the response was lost.🧪 Proposed falsifier
struct FakeWalSink { succeed: bool, + /// Commit durably, then report `Ambiguous` — the real hazard shape + /// (the append landed, the response did not come back). Cleared after + /// one use so the retry takes the normal reconciliation path. + ambiguous_once: bool, sealed: Mutex<Vec<SealedRec>>,impl FakeWalSink { fn new() -> Self { Self { succeed: true, + ambiguous_once: false, sealed: Mutex::new(Vec::new()),In
commit_cycle, after the row is pushed and before returningCommitted:if self.ambiguous_once { self.ambiguous_once = false; return Err(CommitError::Ambiguous { cycle, batch_hash, cause: "response lost after publication".into(), }); }// ── FALSIFIER (Phase A): a re-submit after Ambiguous reconciles, never doubles #[tokio::test] async fn resubmitting_after_ambiguous_reconciles_never_double_appends() { let mut sink = FakeWalSink::new(); sink.ambiguous_once = true; let casts = || vec![slot(42, 1, 0, 0, None), slot(42, 1, 1, 1, None)]; let first = persist_cycle( &mut sink, CycleFrame::new(CycleId(1), DatasetVersion(0)), casts(), ) .await; assert!( matches!( first, Err(PersistError::Commit(CommitError::Ambiguous { cycle: CycleId(1), .. })) ), "the publication outcome is genuinely unknown: {first:?}" ); // The batch DID land. The contract says re-submit the SAME frozen batch. let retry = persist_cycle( &mut sink, CycleFrame::new(CycleId(1), DatasetVersion(0)), casts(), ) .await .unwrap(); assert!( matches!(retry, CommitOutcome::Reconciled { cycle: CycleId(1), .. }), "the retry must reconcile, not append twice: {retry:?}" ); assert_eq!(sink.wal_writes(), 1, "exactly one physical commit"); assert_eq!( sink.scan_sealed(None).await.unwrap().len(), 2, "no duplicate landings after an ambiguous outcome" ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph-planner/src/persist_sink.rs` around lines 1576 - 1644, Extend FakeWalSink with an ambiguous-once injection flag and update commit_cycle to durably record the batch before returning CommitError::Ambiguous once, then clear the flag. Add a test near the existing retry and conflict falsifiers that verifies the first persist returns Ambiguous, resubmitting the identical frozen batch returns Reconciled, and wal_writes plus sealed rows confirm no duplicate append.Source: Coding guidelines
crates/lance-graph-supervisor/src/cycle_driver.rs (1)
129-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
outcomeandversionare two public fields that must agree, with nothing enforcing it.
versionis derived fromoutcomeat construction inseal_cycle, but both fields arepubon apub struct. A caller can buildSealedCycle { outcome: CommitOutcome::NoChange { .. }, version: Some(DatasetVersion(1)), .. }and no code rejects it. The test fixtures at lines 1577-1583, 1606-1612, and 1645-1651 and thebuild_sealed_locallyhelper incrates/lance-graph-supervisor/examples/measure_wal_curve.rsall set the pair by hand today.The doc comment states the intent: keep
sealed.versionreadable at call sites. A constructor keeps that readability and removes the hazard.♻️ Proposed shape
impl SealedCycle { #[must_use] pub fn new( outcome: CommitOutcome, transitions: Vec<SealedTransition>, next_position_base: u64, ) -> Self { let version = match outcome { CommitOutcome::NoChange { .. } => None, CommitOutcome::Committed { version, .. } | CommitOutcome::Reconciled { version, .. } => Some(version), }; Self { outcome, version, transitions, next_position_base } } }
seal_cycleand every literal construction then route throughSealedCycle::new, and the derivation lives in exactly one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph-supervisor/src/cycle_driver.rs` around lines 129 - 147, Encapsulate SealedCycle construction so outcome and version cannot diverge: add a public SealedCycle::new constructor that derives version from outcome, then update seal_cycle, the listed test fixtures, and build_sealed_locally to use it instead of struct literals. Keep sealed.version publicly readable while removing direct public-field construction that permits inconsistent pairs.crates/lance-graph/src/graph/cycle_sink.rs (2)
565-607: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the typed-column accessor that four readers now duplicate.
find_frame,scan_sealed,timeline, andscan_imageeach repeatcolumn_by_name(...).and_then(downcast_ref).ok_or_else(...). A single generic helper removes four copies and makes the error text consistent.♻️ Sketch of the helper
fn typed_col<'a, A: Array + 'static>( b: &'a RecordBatch, name: &str, ) -> Result<&'a A, WriteFailed> { b.column_by_name(name) .and_then(|c| c.as_any().downcast_ref::<A>()) .ok_or_else(|| WriteFailed(format!("missing column {name}"))) }Call sites become
typed_col::<UInt64Array>(b, "cycle")?.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph/src/graph/cycle_sink.rs` around lines 565 - 607, Extract a shared generic typed-column accessor near the duplicated readers, using RecordBatch, column name, and array type parameters to return the downcast array or a consistent “missing column {name}” WriteFailed error. Replace the repeated column_by_name/downcast_ref/ok_or_else logic in find_frame, scan_sealed, timeline, and scan_image with this helper, including the timeline accesses for cycle, base_version, and batch_hash.
325-334: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueCheck
num_rows()before you downcast.The loop resolves and downcasts
batch_hashfirst, then testsb.num_rows() > 0. An empty batch that carries no column produces a misleadingmissing column batch_hasherror. Move the emptiness test first.♻️ Proposed reorder
for b in &batches { + if b.num_rows() == 0 { + continue; + } let h: &UInt64Array = b .column_by_name("batch_hash") .and_then(|c| c.as_any().downcast_ref()) .ok_or_else(|| WriteFailed("missing column batch_hash".into()))?; - if b.num_rows() > 0 { - return Ok(Some(h.value(0))); - } + return Ok(Some(h.value(0))); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph/src/graph/cycle_sink.rs` around lines 325 - 334, In the batch loop, check b.num_rows() before resolving or downcasting the batch_hash column. Skip empty batches immediately, then perform the existing batch_hash lookup and return the first value for non-empty batches while preserving the current missing-column error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/plans/persistence-artifact-backed-commit-v1.md:
- Around line 121-125: Fix the unmatched Markdown backtick in the “Measured, not
assumed” statement around the Append fence description by removing the stray
closing backtick after Append or otherwise balancing the intended inline-code
span, while preserving the existing Lance and Overwrite references.
In @.claude/plans/persistence-cycle-wal-bootstrap-v1.md:
- Around line 10-17: Update the three remaining §2/§7 status references in the
persistence-cycle document: the §2 heading, its status blockquote, and the §7
status-table row. Replace “UNIMPLEMENTED” wording with the current contract
identifying LanceCycleWriter as the implemented concrete sink, while preserving
all surrounding body text and append-only content.
In `@crates/lance-graph-planner/src/persist_sink.rs`:
- Around line 70-71: Update the wrapped rustdoc heading beginning “The governing
storage rule” so it is emitted as one Markdown heading rather than two; remove
the duplicate heading marker on the continuation line and keep the full text
intact.
- Around line 572-576: Update the documentation for CommitOutcome::NoChange to
state that its head is caller-asserted from frame.base_version, not observed
from the store, and may be stale when no artifacts are written. Note that this
outcome does not perform a fence check and is not adopted as a fresh version by
cycle_driver::seal_cycle.
- Around line 357-369: Update content_hash to remove frame.base_version from the
hashed input, keeping only the cycle identity and canonical landings so retries
produce the same durable idempotency key. Preserve the existing FNV-1a algorithm
and hashing of frame.cycle and canonical.
In `@crates/lance-graph-supervisor/examples/measure_wal_curve.rs`:
- Around line 1461-1465: Update the CommitOutcome::Reconciled construction in
the reconciliation path to populate version from the current store head at
reconciliation time rather than rec.version. Preserve the existing cycle and
batch_hash fields, and use the same current-head source as the production writer
so retries report the latest committed horizon.
In `@crates/lance-graph-supervisor/src/cycle_driver.rs`:
- Around line 994-1008: Update the reconciled-success branch in the fake commit
flow to set CommitOutcome::Reconciled.version to the current store head,
matching LanceCycleWriter::commit_cycle and the documented contract, rather than
rec.version. Preserve the existing hash comparison and conflict behavior.
- Around line 390-392: Update the cycle persistence flow around frozen and
persist_cycle so successful commits do not eagerly clone the full casts payload.
Preserve the payload needed for Ambiguous/commit-error retries by returning or
retaining the frozen artifact batch only when persist_cycle fails, using a
ref-counted payload pointer if compatible with the existing retry
representation, while keeping success-path behavior unchanged.
In `@crates/lance-graph/src/graph/cycle_sink.rs`:
- Around line 117-123: Update the schema table documentation to record image
rows as having stream_position 0, matching the existing push_common call in the
image-row path. Keep the current storage behavior unchanged and revise only the
`stream_position / owner / row` entry near the schema table.
- Around line 472-483: Define in the WalSink::scan_sealed documentation that
returned LandedSlot/SweepSlot values carry no payload. In
crates/lance-graph/src/graph/cycle_sink.rs lines 472-483, retain the
payload-free projection and verify no consumer uses payload.is_empty() to
classify recovered landings. In
crates/lance-graph-supervisor/src/cycle_driver.rs lines 1028-1046, clear payload
in FakeWalSink::scan_sealed, and update the whole-slot assertion around line
1269 to compare only owner, stream_position, row, and paired_move.
- Around line 895-1028: Add a test-only fault-injection flag to LanceCycleWriter
so the append operation used by persist_cycle can deterministically return an
error. Add focused unit tests that exercise the append-error, reopen, and
reconciliation path, covering Reconciled, HashConflict, Io, and Ambiguous
outcomes, including the reopen-NotFound case. Verify failed appends preserve the
no-rollback and no-write guarantees.
- Around line 370-394: The normal commit path currently always invokes
find_frame, contradicting the zero-scan invariant and bypassing the existing
opens() instrumentation. Update commit_cycle to avoid reconciliation scans for
cycles this process can identify as unambiguously committed, or add explicit
scan accounting that the invariant and tests use; then revise the documented
invariant to match the actual behavior.
- Around line 337-352: Update reopen so a DatasetNotFound result is treated as
an error when self.ds was already Some, preserving the existing dataset handle
and returning WriteFailed for the ambiguity path; only allow the empty-store
downgrade to self.ds = None when no dataset has previously been held. Keep the
successful reopen behavior and open counting unchanged.
---
Outside diff comments:
In `@crates/lance-graph-planner/src/persist_sink.rs`:
- Around line 460-467: Update the documentation above PersistError::source to
describe all three wrapping variants, adding Commit alongside Write and Illegal;
keep the implementation unchanged.
- Around line 1478-1494: Rename the test function a_fenced_cycle_writes_nothing
to accurately describe the failing WAL I/O scenario, such as
a_cycle_writes_nothing_on_io_error. Keep its assertions and setup unchanged, and
ensure any plan or falsifier references use the new test name.
In `@crates/lance-graph-supervisor/src/cycle_driver.rs`:
- Around line 810-836: Update the `recover_fleet` documentation to state that
`after_cycle` must not exceed the lowest cycle fully applied by every owner in
`fleet_ids`, since skipped landings cannot be recovered later. In the nearby
`#[cfg(test)]` module, add a focused test that seals cycles 1 through 3, invokes
`recover_fleet` with `Some(CycleId(1))`, and verifies only cycles 2 and 3 are
replayed.
In `@crates/lance-graph/src/graph/cycle_sink.rs`:
- Around line 124-144: Update cycle_store_schema and the cycle-store open/append
path to define and persist an explicit layout version, then validate that
version before any append or reconciliation projection uses batch_hash. Reject
older or missing-version stores with a clear migration-ready error instead of
allowing a raw schema mismatch.
---
Nitpick comments:
In `@crates/lance-graph-planner/src/persist_sink.rs`:
- Around line 513-520: The scan_sealed contract currently documents only the
cycle bound, not the potentially unbounded materialized row count. Add a named
deferral beside the existing after_cycle documentation stating that
row-count/memory bounding is deferred and that streaming is the eventual fix;
preserve the current Vec<LandedSlot> return type and behavior.
- Around line 1576-1644: Extend FakeWalSink with an ambiguous-once injection
flag and update commit_cycle to durably record the batch before returning
CommitError::Ambiguous once, then clear the flag. Add a test near the existing
retry and conflict falsifiers that verifies the first persist returns Ambiguous,
resubmitting the identical frozen batch returns Reconciled, and wal_writes plus
sealed rows confirm no duplicate append.
In `@crates/lance-graph-supervisor/src/cycle_driver.rs`:
- Around line 129-147: Encapsulate SealedCycle construction so outcome and
version cannot diverge: add a public SealedCycle::new constructor that derives
version from outcome, then update seal_cycle, the listed test fixtures, and
build_sealed_locally to use it instead of struct literals. Keep sealed.version
publicly readable while removing direct public-field construction that permits
inconsistent pairs.
In `@crates/lance-graph/src/graph/cycle_sink.rs`:
- Around line 565-607: Extract a shared generic typed-column accessor near the
duplicated readers, using RecordBatch, column name, and array type parameters to
return the downcast array or a consistent “missing column {name}” WriteFailed
error. Replace the repeated column_by_name/downcast_ref/ok_or_else logic in
find_frame, scan_sealed, timeline, and scan_image with this helper, including
the timeline accesses for cycle, base_version, and batch_hash.
- Around line 325-334: In the batch loop, check b.num_rows() before resolving or
downcasting the batch_hash column. Skip empty batches immediately, then perform
the existing batch_hash lookup and return the first value for non-empty batches
while preserving the current missing-column error behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ddb5eb2a-f77b-4741-971f-2eda7772010c
📒 Files selected for processing (12)
.claude/board/EPIPHANIES.md.claude/board/INTEGRATION_PLANS.md.claude/board/LATEST_STATE.md.claude/board/PR_ARC_INVENTORY.md.claude/plans/persistence-artifact-backed-commit-v1.md.claude/plans/persistence-cycle-wal-bootstrap-v1.mdcrates/lance-graph-planner/examples/blw_fusion.rscrates/lance-graph-planner/examples/blw_tenant.rscrates/lance-graph-planner/src/persist_sink.rscrates/lance-graph-supervisor/examples/measure_wal_curve.rscrates/lance-graph-supervisor/src/cycle_driver.rscrates/lance-graph/src/graph/cycle_sink.rs
Comment on lines
+121
to
+125
| **Measured, not assumed:** Lance 9 has **no atomic expected-version fence for | ||
| Append`. The conflict rebase runs even on a single-attempt commit; strict | ||
| no-rebase mode exists only for `Overwrite` | ||
| (`lance-9.0.0/src/io/commit.rs:914-950`). This is stated honestly rather than | ||
| papered over with a read-check pretending to be compare-and-swap. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the unmatched backtick in the Append fence statement.
Line 122 ends Append with a closing backtick but has no opening backtick. That stray backtick opens a code span that runs to the backtick before Overwrite on line 123. The rendered text is corrupted, and markdownlint reports MD038 on line 123.
📝 Proposed fix
**Measured, not assumed:** Lance 9 has **no atomic expected-version fence for
-Append`. The conflict rebase runs even on a single-attempt commit; strict
+`Append`. The conflict rebase runs even on a single-attempt commit; strict
no-rebase mode exists only for `Overwrite`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **Measured, not assumed:** Lance 9 has **no atomic expected-version fence for | |
| Append`. The conflict rebase runs even on a single-attempt commit; strict | |
| no-rebase mode exists only for `Overwrite` | |
| (`lance-9.0.0/src/io/commit.rs:914-950`). This is stated honestly rather than | |
| papered over with a read-check pretending to be compare-and-swap. | |
| **Measured, not assumed:** Lance 9 has **no atomic expected-version fence for | |
| `Append`. The conflict rebase runs even on a single-attempt commit; strict | |
| no-rebase mode exists only for `Overwrite` | |
| (`lance-9.0.0/src/io/commit.rs:914-950`). This is stated honestly rather than | |
| papered over with a read-check pretending to be compare-and-swap. |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 123-123: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/plans/persistence-artifact-backed-commit-v1.md around lines 121 -
125, Fix the unmatched Markdown backtick in the “Measured, not assumed”
statement around the Append fence description by removing the stray closing
backtick after Append or otherwise balancing the intended inline-code span,
while preserving the existing Lance and Overwrite references.
Source: Linters/SAST tools
Comment on lines
+10
to
+17
| > across the commit await). The §2 sparse-delta rule is now IMPLEMENTED — the | ||
| > concrete sink is `lance_graph::graph::cycle_sink::LanceCycleWriter`, whose | ||
| > coalesced image rows are the durable end-form. Append-only: nothing below is | ||
| > deleted; it is read through the newer contract. | ||
| > | ||
| > **Status:** ACTIVE (bootstrap SHIPPED in PR #878; upgrade phases PLANNED; the | ||
| > §2 sparse-delta storage rule is RATIFIED architecture, UNIMPLEMENTED in a | ||
| > concrete Lance sink). | ||
| > §2 sparse-delta storage rule is RATIFIED architecture, IMPLEMENTED in | ||
| > `LanceCycleWriter` as of Phase A 2026-08-09). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the §2 status with the surviving "UNIMPLEMENTED" statements.
The new header declares the §2 sparse-delta rule IMPLEMENTED in LanceCycleWriter. Three unchanged places in the same document still declare it UNIMPLEMENTED:
- Line 82, the §2 heading: "RATIFIED architecture, UNIMPLEMENTED in a concrete sink".
- Lines 84-87, the §2 status blockquote: "UNIMPLEMENTED in a concrete Lance sink".
- Line 375, the §7 status table row.
A reader who reaches §2 or §7 directly gets the opposite status. The append-only rule preserves the body text, but status lines are the permitted exception. Update those three status lines to point at the new contract.
📝 Proposed status-line updates (outside the reviewed range)
-## 2. A complete logical cycle is physically SPARSE (RATIFIED architecture, UNIMPLEMENTED in a concrete sink)
+## 2. A complete logical cycle is physically SPARSE (RATIFIED architecture, IMPLEMENTED in `LanceCycleWriter`)
-> **Status of this section:** the sparse-delta rule is **RATIFIED as
-> architecture** and **UNIMPLEMENTED in a concrete Lance sink**. The `#878`
-> bootstrap remains SHIPPED; this section governs the *future* concrete sink,
-> not the merged contract-probe.
+> **Status of this section (updated 2026-08-09):** the sparse-delta rule is
+> **RATIFIED as architecture** and **IMPLEMENTED** in
+> `lance_graph::graph::cycle_sink::LanceCycleWriter` (Phase A), whose coalesced
+> image rows are the durable end-form. The `#878` bootstrap remains SHIPPED.-| Sparse-delta storage rule (complete cycle ≠ full rewrite) | **RATIFIED architecture, UNIMPLEMENTED in a concrete sink** (§2) |
+| Sparse-delta storage rule (complete cycle ≠ full rewrite) | **RATIFIED architecture, IMPLEMENTED** in `LanceCycleWriter` (Phase A, §2) |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/plans/persistence-cycle-wal-bootstrap-v1.md around lines 10 - 17,
Update the three remaining §2/§7 status references in the persistence-cycle
document: the §2 heading, its status blockquote, and the §7 status-table row.
Replace “UNIMPLEMENTED” wording with the current contract identifying
LanceCycleWriter as the implemented concrete sink, while preserving all
surrounding body text and append-only content.
Source: Coding guidelines
Comment on lines
+70
to
+71
| //! ## The governing storage rule (operator-ruled 2026-08-09 — supersedes the | ||
| //! ## earlier "one version per cycle, empty cycles included" contract) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the duplicated ## on the wrapped heading.
Lines 70 and 71 each start a Markdown heading. Rustdoc renders one heading per line, so this one logical heading becomes two headings, and the second reads "earlier "one version per cycle, empty cycles included" contract)". Put the heading on one line.
📝 Proposed fix
-//! ## The governing storage rule (operator-ruled 2026-08-09 — supersedes the
-//! ## earlier "one version per cycle, empty cycles included" contract)
+//! ## The governing storage rule
+//!
+//! Operator-ruled 2026-08-09. Supersedes the earlier "one version per cycle,
+//! empty cycles included" contract.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| //! ## The governing storage rule (operator-ruled 2026-08-09 — supersedes the | |
| //! ## earlier "one version per cycle, empty cycles included" contract) | |
| //! ## The governing storage rule | |
| //! | |
| //! Operator-ruled 2026-08-09. Supersedes the earlier "one version per cycle, | |
| //! empty cycles included" contract. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph-planner/src/persist_sink.rs` around lines 70 - 71, Update
the wrapped rustdoc heading beginning “The governing storage rule” so it is
emitted as one Markdown heading rather than two; remove the duplicate heading
marker on the continuation line and keep the full text intact.
Comment on lines
+357
to
+369
| /// FNV-1a 64 over the frame identity + canonical landing content. | ||
| fn content_hash(frame: CycleFrame, canonical: &[SweepSlot]) -> u64 { | ||
| const OFFSET: u64 = 0xcbf2_9ce4_8422_2325; | ||
| const PRIME: u64 = 0x0000_0100_0000_01b3; | ||
| let mut h = OFFSET; | ||
| let mut eat = |bytes: &[u8]| { | ||
| for b in bytes { | ||
| h ^= u64::from(*b); | ||
| h = h.wrapping_mul(PRIME); | ||
| } | ||
| }; | ||
| eat(&frame.cycle.0.to_le_bytes()); | ||
| eat(&frame.base_version.0.to_le_bytes()); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find every construction of CycleFrame on a retry path to see whether any caller re-derives base_version instead of reusing the frozen frame.
set -euo pipefail
echo '== CycleFrame::new call sites =='
rg -nP --type=rust -C4 'CycleFrame::new\s*\(' crates
echo '== SealFailure handling / retry loops =='
rg -nP --type=rust -C8 '\bSealFailure\b' crates
echo '== HashConflict handling by callers =='
rg -nP --type=rust -C5 '\bHashConflict\b' cratesRepository: AdaWorldAPI/lance-graph
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== persist_sink relevant sections =='
sed -n '300,390p' crates/lance-graph-planner/src/persist_sink.rs
echo
sed -n '700,900p' crates/lance-graph-planner/src/persist_sink.rs
echo
sed -n '1579,1645p' crates/lance-graph-planner/src/persist_sink.rs
echo '== cycle_driver seal/cycle handling =='
rg -n --type=rust -C6 'enum SealFailure|struct SealFailure|seal_cycle|re-submit|retry|lost acknowledgement|lost ack|acknowledgement' crates/lance-graph-supervisor/src/cycle_driver.rs crates/lance-graph-supervisor/src | head -n 220
echo '== deterministic hash behavior probe from source text =='
python3 - <<'PY'
from pathlib import Path
text = Path('crates/lance-graph-planner/src/persist_sink.rs').read_text()
start = text.index('fn batch_hash')
end = text.index('\n }\n', start) + 6
print(text[start:end])
PYRepository: AdaWorldAPI/lance-graph
Length of output: 35709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== deterministic hash conflict probe ='
python3 - <<'PY'
from pathlib import Path
import re
text = Path('crates/lance-graph-planner/src/persist_sink.rs').read_text()
m = re.search(r'fn content_hash\(frame: CycleFrame, canonical: \[\]SweepSlot\)\s*-> u64\s*\{(.*?)^ \}', text, re.M | re.S)
if not m:
raise SystemExit('content_hash not found')
code = m.group(0)
print('has_base_version_eat=', 'eat(&frame.base_version.0.to_le_bytes());' in code)
print('has_cycle_eat=', 'eat(&frame.cycle.0.to_le_bytes());' in code)
PY
echo '== SealFailure docs and retry cache probe =='
sed -n '155,188p' crates/lance-graph-supervisor/src/cycle_driver.rs
sed -n '348,394p' crates/lance-graph-supervisor/src/cycle_driver.rs
sed -n '1227,1248p' crates/lance-graph-supervisor/src/cycle_driver.rs
echo '== HashConflict documentation occurrences =='
rg -n --type=rust -C3 'CommitError::HashConflict|HashConflict is a fail-closed|different hash fails closed|different hash.*fail-closed|corruption|idempotency key|Deterministic content hash' crates/lance-graph-supervisor crates/lance-graph-plannerRepository: AdaWorldAPI/lance-graph
Length of output: 223
Remove the position-dependent field from batch_hash.
batch_hash is documented as the durable idempotency key, but content_hash includes frame.base_version. A cycle that commits at V1 while the retry sees V1 produces the same canonical landings with a different batch hash, so reconciliation returns CommitError::HashConflict instead of Reconciled. Hash only the cycle identity plus the canonical landings, or explicitly make the exact frame part of the contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph-planner/src/persist_sink.rs` around lines 357 - 369,
Update content_hash to remove frame.base_version from the hashed input, keeping
only the cycle identity and canonical landings so retries produce the same
durable idempotency key. Preserve the existing FNV-1a algorithm and hashing of
frame.cycle and canonical.
Comment on lines
+572
to
+576
| if artifacts.is_empty() { | ||
| return Ok(CommitOutcome::NoChange { | ||
| head: frame.base_version, | ||
| }); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
NoChange.head is caller-asserted, not store-observed.
Every other outcome comes from the sink. NoChange is returned without calling the sink, so head is whatever the caller put in frame.base_version. If that base is stale, persist_cycle reports a stale head and no fence check runs. Nothing is written, so there is no durability risk, and cycle_driver::seal_cycle maps NoChange to version: None so the driver does not adopt it.
Record the provenance in the doc so a future caller does not treat NoChange.head as a fresh read of the store head.
📝 Proposed doc fix
if artifacts.is_empty() {
+ // The sink is deliberately NOT called, so this head is the caller's
+ // asserted `frame.base_version`, never a fresh store read. No fence
+ // runs, which is sound only because nothing is written.
return Ok(CommitOutcome::NoChange {
head: frame.base_version,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if artifacts.is_empty() { | |
| return Ok(CommitOutcome::NoChange { | |
| head: frame.base_version, | |
| }); | |
| } | |
| if artifacts.is_empty() { | |
| // The sink is deliberately NOT called, so this head is the caller's | |
| // asserted `frame.base_version`, never a fresh store read. No fence | |
| // runs, which is sound only because nothing is written. | |
| return Ok(CommitOutcome::NoChange { | |
| head: frame.base_version, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph-planner/src/persist_sink.rs` around lines 572 - 576,
Update the documentation for CommitOutcome::NoChange to state that its head is
caller-asserted from frame.base_version, not observed from the store, and may be
stale when no artifacts are written. Note that this outcome does not perform a
fence check and is not adopted as a fresh version by cycle_driver::seal_cycle.
Comment on lines
+117
to
+123
| /// | column | frame | landing | image | | ||
| /// |---|---|---|---| | ||
| /// | `kind` | 0 | 1 | 2 | | ||
| /// | `cycle` / `base_version` / `batch_hash` | ✓ | ✓ | ✓ | | ||
| /// | `stream_position` / `owner` / `row` | 0 | ✓ | winner's / 0 / row | | ||
| /// | `move_*` (nullable) | null | cast's move | null | | ||
| /// | `payload` (`FixedSizeBinary(512)`, nullable) | null | null | final image | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the schema table: image rows store stream_position = 0, not the winner's position.
Line 121 documents the image column as winner's / 0 / row. Line 275 calls push_common(KIND_IMAGE, 0, 0, *row_id), so stream_position is written as 0. Either write the winning landing's stream_position or fix the table. Storing the winner's position would let an auditor tie an image row back to the landing that produced it; storing 0 is cheaper. Choose one and make the table match.
📝 Doc-only fix (keeps the current `0` behavior)
-/// | `stream_position` / `owner` / `row` | 0 | ✓ | winner's / 0 / row |
+/// | `stream_position` / `owner` / `row` | 0 | ✓ | 0 / 0 / row |Also applies to: 273-283
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph/src/graph/cycle_sink.rs` around lines 117 - 123, Update
the schema table documentation to record image rows as having stream_position 0,
matching the existing push_common call in the image-row path. Keep the current
storage behavior unchanged and revise only the `stream_position / owner / row`
entry near the schema table.
Comment on lines
+472
to
+483
| /// Committed landing METADATA in stored canonical order, bounded by | ||
| /// `after_cycle` (pushed into the Lance scan). Payloads are NOT read here | ||
| /// — landing rows carry none (the durable payloads live in the coalesced | ||
| /// image, read via [`LanceCycleWriter::scan_image`]); returned slots carry | ||
| /// empty payload vectors. | ||
| async fn scan_sealed( | ||
| &self, | ||
| ds: &Dataset, | ||
| kind_filter: u8, | ||
| ) -> Result<Vec<StoredRow>, WriteFailed> { | ||
| after_cycle: Option<CycleId>, | ||
| ) -> Result<Vec<LandedSlot>, WriteFailed> { | ||
| let Some(ds) = self.ds.as_ref() else { | ||
| return Ok(Vec::new()); | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The WalSink::scan_sealed contract does not define whether SweepSlot::payload is returned, and the two implementations chose opposite answers. The production writer omits the payload column from the projection and returns empty payloads; the supervisor's test fake clones the stored payload. Every test written against the fake therefore proves a property the real writer does not hold. Fix the contract first: state in the WalSink::scan_sealed doc that returned landings carry no payload, then align both implementations and the affected assertion.
crates/lance-graph/src/graph/cycle_sink.rs#L472-L483: keep the payload-free projection, and confirm no consumer classifies a recovered landing bypayload.is_empty()— that predicate is the artifact gate inpersist_cycle, so an empty payload now means "intent-only" to any such consumer.crates/lance-graph-supervisor/src/cycle_driver.rs#L1028-L1046: clearpayloadon the clonedSweepSlotinFakeWalSink::scan_sealedso the fake models the production contract, and change the whole-slot comparison at line 1269 to compareowner,stream_position,row, andpaired_moveonly.
📍 Affects 2 files
crates/lance-graph/src/graph/cycle_sink.rs#L472-L483(this comment)crates/lance-graph-supervisor/src/cycle_driver.rs#L1028-L1046
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph/src/graph/cycle_sink.rs` around lines 472 - 483, Define in
the WalSink::scan_sealed documentation that returned LandedSlot/SweepSlot values
carry no payload. In crates/lance-graph/src/graph/cycle_sink.rs lines 472-483,
retain the payload-free projection and verify no consumer uses
payload.is_empty() to classify recovered landings. In
crates/lance-graph-supervisor/src/cycle_driver.rs lines 1028-1046, clear payload
in FakeWalSink::scan_sealed, and update the whole-slot assertion around line
1269 to compare only owner, stream_position, row, and paired_move.
Comment on lines
+895
to
+1028
| /// F10 + F12 (the reconciliation half): re-submitting the SAME frozen batch | ||
| /// after a "lost acknowledgement" reconciles to exactly one conclusion — no | ||
| /// duplicate rows, no second version, and NO delete anywhere. | ||
| #[tokio::test] | ||
| async fn empty_cycle_advances_timeline_only() { | ||
| async fn resubmitting_the_same_batch_reconciles_to_one() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| let path = dir.path().join("witness_cycles.lance"); | ||
| let sink = LanceCycleSink::new(path.to_str().unwrap()); | ||
| let path = dir.path().join("cycles.lance"); | ||
| let mut w = LanceCycleWriter::open(path.to_str().unwrap()) | ||
| .await | ||
| .unwrap(); | ||
| let casts = || vec![artifact(1, 0, 42, 1), artifact(1, 1, 42, 2)]; | ||
|
|
||
| let v = persist_cycle( | ||
| &sink, | ||
| let first = persist_cycle( | ||
| &mut w, | ||
| CycleFrame::new(CycleId(1), DatasetVersion(0)), | ||
| vec![], | ||
| casts(), | ||
| ) | ||
| .await | ||
| .unwrap(); | ||
| assert_eq!(v, DatasetVersion(1)); | ||
| assert!(matches!( | ||
| first, | ||
| CommitOutcome::Committed { | ||
| version: DatasetVersion(1), | ||
| .. | ||
| } | ||
| )); | ||
|
|
||
| let reopened = LanceCycleSink::new(path.to_str().unwrap()); | ||
| assert!(reopened.scan_sealed(None).await.unwrap().is_empty()); | ||
| assert_eq!( | ||
| reopened.versions().await.unwrap(), | ||
| vec![(CycleId(1), DatasetVersion(1))] | ||
| // The response was lost; the caller retries the identical batch. | ||
| let retry = persist_cycle( | ||
| &mut w, | ||
| CycleFrame::new(CycleId(1), DatasetVersion(0)), | ||
| casts(), | ||
| ) | ||
| .await | ||
| .unwrap(); | ||
| assert!( | ||
| matches!( | ||
| retry, | ||
| CommitOutcome::Reconciled { | ||
| cycle: CycleId(1), | ||
| .. | ||
| } | ||
| ), | ||
| "{retry:?}" | ||
| ); | ||
| } | ||
| assert_eq!(w.head(), DatasetVersion(1), "no second version"); | ||
|
|
||
| /// An empty store is a state, not an error: nothing sealed, empty timeline. | ||
| #[tokio::test] | ||
| async fn empty_store_reads_empty() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| let path = dir.path().join("never_created.lance"); | ||
| let sink = LanceCycleSink::new(path.to_str().unwrap()); | ||
| assert!(sink.scan_sealed(None).await.unwrap().is_empty()); | ||
| assert!(sink.versions().await.unwrap().is_empty()); | ||
| let reopened = LanceCycleWriter::open(path.to_str().unwrap()) | ||
| .await | ||
| .unwrap(); | ||
| assert_eq!( | ||
| reopened.scan_sealed(None).await.unwrap().len(), | ||
| 2, | ||
| "no duplicate landings after restart" | ||
| ); | ||
| assert_eq!(reopened.timeline().await.unwrap().len(), 1, "one frame"); | ||
| } | ||
|
|
||
| /// A no-move landing round-trips as `None` (nullable move columns), and a | ||
| /// large-ish payload survives byte-exact. | ||
| /// F10 (the fail-closed half): a DIFFERENT batch for a durable cycle is | ||
| /// refused loudly and writes nothing. | ||
| #[tokio::test] | ||
| async fn move_nullability_and_payload_roundtrip() { | ||
| async fn a_conflicting_batch_for_a_durable_cycle_fails_closed() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| let path = dir.path().join("witness_cycles.lance"); | ||
| let sink = LanceCycleSink::new(path.to_str().unwrap()); | ||
|
|
||
| let witness_node = (0..=255u8).cycle().take(512).collect::<Vec<u8>>(); | ||
| let mut s = slot(1, 3, 11, 900); | ||
| s.paired_move = None; | ||
| s.payload = witness_node.clone(); | ||
| let path = dir.path().join("cycles.lance"); | ||
| let mut w = LanceCycleWriter::open(path.to_str().unwrap()) | ||
| .await | ||
| .unwrap(); | ||
| persist_cycle( | ||
| &sink, | ||
| &mut w, | ||
| CycleFrame::new(CycleId(1), DatasetVersion(0)), | ||
| vec![s], | ||
| vec![artifact(1, 0, 42, 1)], | ||
| ) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| let reopened = LanceCycleSink::new(path.to_str().unwrap()); | ||
| let sealed = reopened.scan_sealed(None).await.unwrap(); | ||
| assert_eq!(sealed.len(), 1); | ||
| assert_eq!(sealed[0].slot.paired_move, None); | ||
| assert_eq!(sealed[0].slot.payload, witness_node); | ||
| assert_eq!(sealed[0].slot.owner, 11); | ||
| assert_eq!(sealed[0].slot.row, 900); | ||
| let conflict = persist_cycle( | ||
| &mut w, | ||
| CycleFrame::new(CycleId(1), DatasetVersion(0)), | ||
| vec![artifact(1, 9, 42, 1)], | ||
| ) | ||
| .await; | ||
| assert!( | ||
| matches!( | ||
| conflict, | ||
| Err(lance_graph_planner::persist_sink::PersistError::Commit( | ||
| CommitError::HashConflict { | ||
| cycle: CycleId(1), | ||
| .. | ||
| } | ||
| )) | ||
| ), | ||
| "{conflict:?}" | ||
| ); | ||
| assert_eq!(w.head(), DatasetVersion(1), "nothing was written"); | ||
| } | ||
|
|
||
| /// A malformed witness payload (≠ 512 bytes) is refused with NOTHING | ||
| /// written — the canonical node row stride is enforced before anything | ||
| /// durable happens, and the store stays exactly as it was. | ||
| /// F11-adjacent: a stale horizon is FENCED with the current head and | ||
| /// writes nothing (never a delete, never a silent accept). | ||
| #[tokio::test] | ||
| async fn malformed_payload_is_refused_before_persistence() { | ||
| async fn a_stale_horizon_is_fenced_and_writes_nothing() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| let path = dir.path().join("witness_cycles.lance"); | ||
| let sink = LanceCycleSink::new(path.to_str().unwrap()); | ||
|
|
||
| let mut bad = slot(1, 1, 3, 50); | ||
| bad.payload = vec![0u8; 100]; | ||
| let err = persist_cycle( | ||
| &sink, | ||
| let path = dir.path().join("cycles.lance"); | ||
| let mut w = LanceCycleWriter::open(path.to_str().unwrap()) | ||
| .await | ||
| .unwrap(); | ||
| persist_cycle( | ||
| &mut w, | ||
| CycleFrame::new(CycleId(1), DatasetVersion(0)), | ||
| vec![bad], | ||
| vec![artifact(1, 0, 42, 1)], | ||
| ) | ||
| .await | ||
| .unwrap_err(); | ||
| assert!(err.to_string().contains("100 bytes"), "{err}"); | ||
| // Nothing durable: the dataset was never even created. | ||
| assert!(Dataset::open(path.to_str().unwrap()).await.is_err()); | ||
| .unwrap(); | ||
| let stale = persist_cycle( | ||
| &mut w, | ||
| CycleFrame::new(CycleId(2), DatasetVersion(0)), | ||
| vec![artifact(2, 1, 42, 2)], | ||
| ) | ||
| .await; | ||
| assert!( | ||
| matches!( | ||
| stale, | ||
| Err(lance_graph_planner::persist_sink::PersistError::Commit( | ||
| CommitError::Fenced { | ||
| current_head: DatasetVersion(1) | ||
| } | ||
| )) | ||
| ), | ||
| "{stale:?}" | ||
| ); | ||
| let reopened = LanceCycleWriter::open(path.to_str().unwrap()) | ||
| .await | ||
| .unwrap(); | ||
| assert_eq!(reopened.timeline().await.unwrap().len(), 1); | ||
| assert_eq!(reopened.scan_sealed(None).await.unwrap().len(), 1); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Add a falsifier for the ambiguous-append branch.
The tests cover NoChange, Committed, Reconciled, HashConflict, and Fenced. The failure branch at lines 433-468 — append error, then reopen, then reconcile — has no test. That branch carries the whole no-rollback contract and the four distinct outcomes it can produce (Reconciled, HashConflict, Io, Ambiguous). It is also where the reopen NotFound problem flagged at lines 337-352 becomes reachable.
A real Lance append cannot be made to fail deterministically from a test today. Introduce a fault-injection seam (a #[cfg(test)] flag on the writer that forces the append to return an error) so the branch is exercised.
Do you want me to write those falsifiers and the injection seam?
As per coding guidelines: "Add Rust unit tests alongside implementations via #[cfg(test)] modules; prefer focused scenarios over broad integration tests".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph/src/graph/cycle_sink.rs` around lines 895 - 1028, Add a
test-only fault-injection flag to LanceCycleWriter so the append operation used
by persist_cycle can deterministically return an error. Add focused unit tests
that exercise the append-error, reopen, and reconciliation path, covering
Reconciled, HashConflict, Io, and Ambiguous outcomes, including the
reopen-NotFound case. Verify failed appends preserve the no-rollback and
no-write guarantees.
Source: Coding guidelines
1. reopen() never degrades an existing store to empty: a held handle survives a transient DatasetNotFound; the outcome stays Ambiguous and Create is unreachable for a store with history. 2. One-writer topology ENFORCED in-process: a process-local path registry refuses a second live LanceCycleWriter on the same path (Drop frees); cross-process exclusivity remains a documented deployment lease. 3. The normal commit path is genuinely scan-free: committed_through cycle watermark seeded at open (one bounded frame-projected read), fresh monotonic commits append directly; reconciliation scans run only on fence mismatch, at-or-below-watermark re-submission, or ambiguity — instrumented via reconcile_scans() and falsified at zero. 4. Reconciled.version renamed current_head: the store head at reconciliation is NOT the publication version; only Committed.version is an audit-grade reference (Gotham must never cite a Reconciled head). 5. recover_fleet reports foreign_landings + foreign_min_cycle — the latecomer fence: callers must never raise the global after_cycle bound to or past the smallest foreign landing's cycle. Honest limits recorded, not claimed fixed: the artifact gate tests payload PRESENCE, not semantic CHANGE (typed IntentOnly|ArtifactChanged + digest dedup = the Phase-D conclusion-identity refinement, documented in persist_sink's module doc); the 64-breaths falsifier now asserts the 64 compact landing-metadata rows explicitly (their collapse into a per-plan rollup is Phase B/C); run_cycle's doc states the fleet borrow IS held across the await by signature — the production detached path is collect_casts -> seal_cycle (no fleet) -> apply_sealed_transitions. Also: pre-Phase-A store schemas are rejected loudly at open (never reinterpreted); restart re-submission reconciles with exactly one scan (falsified); dual-writer refusal falsified; tests now drop the prior writer before restart-reopen (true restart semantics). Suites: cycle_sink 13, persist_sink 22, cycle_driver 21 — green; fmt clean; all targets compile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KCGhDYoQBXs3poaR7sFuqp
AdaWorldAPI
pushed a commit
that referenced
this pull request
Aug 18, 2026
…t deprecated (#879/#911/#912/#913) Recorded in the RP-SEAL plan header + the pin-ruling EPIPHANIES entry; the workflow script was corrected in place (source map + the two cell briefs naming rustynum); the in-flight independent pass could not be force-stopped in this harness build, so the ruling binds consolidation as a hard filter. Deltalake removal ratified same exchange. Co-Authored-By: Claude <noreply@anthropic.com>
AdaWorldAPI
added a commit
that referenced
this pull request
Aug 18, 2026
…e.dev debug=0 (#962) * ogar_codebook: sync the ConceptDomain wire-mirror -- Ontology, Blocks, and the C-band The contract's mirror of OGAR's ConceptDomain ended at Geo (0x0F) while OGAR carries Ontology (0x03, populated by the DisMech 0x0333 mints), Blocks (0x17), and the C-band JavaRuntime/Analytics/BinaryLifting (0xC0/0xC1/0xC4 -- the altitude ruling, OGAR #276+#277; 0xC0 is Panama FFM alone, Valhalla being a property of the C0 vocabulary rather than an addressable concept). Both sides' docs demand they update together; this is the catch-up, found by the lance-graph-java session and verified independently by the ruff/R2IL session at db488f5, with ownership of the sync explicitly handed here so the ruff arc's PR3 rebases trivially. The real finding is WHY the drift guard never fired: domains_agree + assert_codebook_parity only walk ids that carry concept rows, so a reserved-EMPTY domain added to one enum but not the other is invisible to a content walk. Proven live -- the first disable-run (dropping the new BinaryLifting pair from domains_agree) stayed GREEN. Repaired with reserved_empty_domains_agree_across_the_mirror: one id per new domain, the populated 0x0333, the deliberate 0xC2-0xC3 gap pinned like OGAR's own 0x10-0x16, the band edges, and the 0x0C/0xC0 digit-swap two-sided. Both disable-runs (bridge pair dropped; contract arm dropped) now go red on exactly that test; the contract's own domain_routes_on_high_byte independently catches the arm removal. Gates: lance-graph-contract 1162/1162 + doctests, clippy --all-targets clean; lance-graph-ogar (workspace-excluded, tested via manifest-path) 64/64 incl. assert_codebook_parity green -- content parity holds, this was domain-level drift only. The crate's 11 pre-existing clippy warnings are measured identical with this diff stashed and left untouched. Board: EPIPHANIES E-OGAR-CODEBOOK-MIRROR-DOMAIN-DRIFT-SYNCED-1 prepended in the same commit, per the board-hygiene rule. * lotus Phase 0/1: frontier audit + F-ORD-REAL pre-registered falsifier Research charter deliverables 1-2 (no fix included, by design): - docs/lotus/LOTUS-FRONTIER-AUDIT.md — Phase 0 archaeology across the write path, persistence capability, placement/comma prior art, and frontier visibility, every statement graded VERIFIED / INFERENCE / HYPOTHESIS / BLOCKER. Headline findings: content_hash folds arrival-minted stream_position values into batch_hash (persist_sink.rs:414), contradicting DetachedCycleBatch's own order-independence doc; the seal is O(batch bytes) x3 passes with the batch resident up to 3x at seal; SweepSlot's caller-supplied semantic order-key contract vs collect_casts' arrival mint; the lance crate source is absent from this sandbox (prepared-artifact capability audit BLOCKED). Section 6 answers the permeability question: the cycle does not become permeable, it becomes thin — trailing-publication pipelining (Regime A) needs no epistemic weakening; rung-qualified frontier visibility (temporal.rs EpistemicMode ladder) stratifies rather than reopens the retired race; texts stay linear, tiles get derived placement, resolved per class. - docs/lotus/F-ORD-REAL-FALSIFIER.md — the defect mechanism in four verified steps + the test design (perturb the process that creates the key: permute cast() call order, never post-mint slots). - cycle_driver.rs tests: f_ord_real_defect_pin_... (GREEN, two-sided — anti-vacuity proves the perturbation reaches the key mint; semantic set + image pinned arrival-independent; batch_hash pinned arrival-DEPENDENT; fails loudly when a fix lands) and f_ord_real_publication_identity_... (#[ignore]d RED falsifier — the desired property, red under --ignored on the real chain). Gates: cargo test -p lance-graph-supervisor --features cycle-driver 28 passed / 1 ignored (+ suites green); RED verified red under --ignored; fmt clean; clippy adds zero new warnings (8 pre-existing recover_fleet lints only visible under this non-default feature). Board: EPIPHANIES E-FORD-REAL-PUBLICATION-IDENTITY-IS-ARRIVAL-DEPENDENT-1; STATUS_BOARD lotus-seal-fractal-commit-frontier section (D-LOTUS-1..9). Co-Authored-By: Claude <noreply@anthropic.com> * board: #961 arc entry + LATEST_STATE + D-LOTUS-1/2 Shipped flips Co-Authored-By: Claude <noreply@anthropic.com> * operator pin ruling: DF 54.1 only — remove broken delta feature; RP-SEAL charter + boards Ruling (E-PIN-LANCE9-LANCEDB033-DF541-ARROW58-NO-DF53-1): lance 9 / lancedb 0.33 / datafusion 54.1 (no DF 53) / arrow 58, always, across AdaWorldAPI forks, usually via [patch -> upstream repository git]. Measured before acting: the only DF-53 source was deltalake 0.32.4 (^53.1.0) behind the already-non-default, already-broken delta feature; registry check shows no DF-54 deltalake exists. Removed: delta feature, deltalake + url optional deps, DeltaTableReader (module docs + Cargo.toml carry dated removal notes; DataSourceFormat::Delta stays as a catalog tag). Post-removal Cargo.lock: exactly ONE datafusion = 54.1.0, zero deltalake entries. cargo check -p lance-graph green; fmt clean; full suite running as the PR gate. Docker pins surveyed: root + avx512 = Rust 1.97.1 + protobuf-compiler, no delta references — removal is docker-safe (stale flag noted: crates/symbiont/Dockerfile rust:1.95). Also: RP-SEAL research charter committed as .claude/plans/erasure-seals-compaction-research-v1.md (15-researcher program, independent pass dispatched as background workflow wf_ca974718-1b4; adversaries strongest-tier, builders/scouts grindwork tier); INTEGRATION_PLANS + STATUS_BOARD entries; D-LOTUS-6 BLOCKER lifted (operator-sanctioned upstream-git source consult; exact v9.0.0 tag on disk matching the lock checksum) with the audit carrying the dated lift note; CLAUDE.md 'BOTH MAJORS ARE REQUIRED' note superseded in place. Co-Authored-By: Claude <noreply@anthropic.com> * scope pivot (operator): rustynum struck — everything ndarray; symbiont deprecated (#879/#911/#912/#913) Recorded in the RP-SEAL plan header + the pin-ruling EPIPHANIES entry; the workflow script was corrected in place (source map + the two cell briefs naming rustynum); the in-flight independent pass could not be force-stopped in this harness build, so the ruling binds consolidation as a hard filter. Deltalake removal ratified same exchange. Co-Authored-By: Claude <noreply@anthropic.com> * correct the delta-removal notes: delta-rs MAIN is already DF 54 + arrow 58 Operator-pointed, verified from delta-io/delta-rs main Cargo.toml (datafusion = 54.0.0, arrow = 58): only the crates.io releases top out at DF 53. Restoration is available now via a git-pin on upstream + a reader refactor to the current builder API — as its own deliberate PR if a consumer needs Delta. The removal itself stands on the need ruling ('we don't need deltalake'), not on availability; notes in Cargo.toml, CLAUDE.md, and the EPIPHANIES entry corrected accordingly. Co-Authored-By: Claude <noreply@anthropic.com> * profile.dev debug=0 — smaller/faster builds (operator ruling) Full-debuginfo test binaries grew target/ to 17 GB and SIGBUS'd the linker on a full disk this session; debug=0 shrinks the lance/datafusion-stack test binaries ~an order of magnitude. Matches the gate already running with CARGO_PROFILE_DEV_DEBUG=0 (same resolved profile, cache reuse). line-tables-only noted as the fallback if line-numbered backtraces are ever needed. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
AdaWorldAPI
added a commit
that referenced
this pull request
Aug 18, 2026
* ogar_codebook: sync the ConceptDomain wire-mirror -- Ontology, Blocks, and the C-band The contract's mirror of OGAR's ConceptDomain ended at Geo (0x0F) while OGAR carries Ontology (0x03, populated by the DisMech 0x0333 mints), Blocks (0x17), and the C-band JavaRuntime/Analytics/BinaryLifting (0xC0/0xC1/0xC4 -- the altitude ruling, OGAR #276+#277; 0xC0 is Panama FFM alone, Valhalla being a property of the C0 vocabulary rather than an addressable concept). Both sides' docs demand they update together; this is the catch-up, found by the lance-graph-java session and verified independently by the ruff/R2IL session at db488f5, with ownership of the sync explicitly handed here so the ruff arc's PR3 rebases trivially. The real finding is WHY the drift guard never fired: domains_agree + assert_codebook_parity only walk ids that carry concept rows, so a reserved-EMPTY domain added to one enum but not the other is invisible to a content walk. Proven live -- the first disable-run (dropping the new BinaryLifting pair from domains_agree) stayed GREEN. Repaired with reserved_empty_domains_agree_across_the_mirror: one id per new domain, the populated 0x0333, the deliberate 0xC2-0xC3 gap pinned like OGAR's own 0x10-0x16, the band edges, and the 0x0C/0xC0 digit-swap two-sided. Both disable-runs (bridge pair dropped; contract arm dropped) now go red on exactly that test; the contract's own domain_routes_on_high_byte independently catches the arm removal. Gates: lance-graph-contract 1162/1162 + doctests, clippy --all-targets clean; lance-graph-ogar (workspace-excluded, tested via manifest-path) 64/64 incl. assert_codebook_parity green -- content parity holds, this was domain-level drift only. The crate's 11 pre-existing clippy warnings are measured identical with this diff stashed and left untouched. Board: EPIPHANIES E-OGAR-CODEBOOK-MIRROR-DOMAIN-DRIFT-SYNCED-1 prepended in the same commit, per the board-hygiene rule. * lotus Phase 0/1: frontier audit + F-ORD-REAL pre-registered falsifier Research charter deliverables 1-2 (no fix included, by design): - docs/lotus/LOTUS-FRONTIER-AUDIT.md — Phase 0 archaeology across the write path, persistence capability, placement/comma prior art, and frontier visibility, every statement graded VERIFIED / INFERENCE / HYPOTHESIS / BLOCKER. Headline findings: content_hash folds arrival-minted stream_position values into batch_hash (persist_sink.rs:414), contradicting DetachedCycleBatch's own order-independence doc; the seal is O(batch bytes) x3 passes with the batch resident up to 3x at seal; SweepSlot's caller-supplied semantic order-key contract vs collect_casts' arrival mint; the lance crate source is absent from this sandbox (prepared-artifact capability audit BLOCKED). Section 6 answers the permeability question: the cycle does not become permeable, it becomes thin — trailing-publication pipelining (Regime A) needs no epistemic weakening; rung-qualified frontier visibility (temporal.rs EpistemicMode ladder) stratifies rather than reopens the retired race; texts stay linear, tiles get derived placement, resolved per class. - docs/lotus/F-ORD-REAL-FALSIFIER.md — the defect mechanism in four verified steps + the test design (perturb the process that creates the key: permute cast() call order, never post-mint slots). - cycle_driver.rs tests: f_ord_real_defect_pin_... (GREEN, two-sided — anti-vacuity proves the perturbation reaches the key mint; semantic set + image pinned arrival-independent; batch_hash pinned arrival-DEPENDENT; fails loudly when a fix lands) and f_ord_real_publication_identity_... (#[ignore]d RED falsifier — the desired property, red under --ignored on the real chain). Gates: cargo test -p lance-graph-supervisor --features cycle-driver 28 passed / 1 ignored (+ suites green); RED verified red under --ignored; fmt clean; clippy adds zero new warnings (8 pre-existing recover_fleet lints only visible under this non-default feature). Board: EPIPHANIES E-FORD-REAL-PUBLICATION-IDENTITY-IS-ARRIVAL-DEPENDENT-1; STATUS_BOARD lotus-seal-fractal-commit-frontier section (D-LOTUS-1..9). Co-Authored-By: Claude <noreply@anthropic.com> * board: #961 arc entry + LATEST_STATE + D-LOTUS-1/2 Shipped flips Co-Authored-By: Claude <noreply@anthropic.com> * operator pin ruling: DF 54.1 only — remove broken delta feature; RP-SEAL charter + boards Ruling (E-PIN-LANCE9-LANCEDB033-DF541-ARROW58-NO-DF53-1): lance 9 / lancedb 0.33 / datafusion 54.1 (no DF 53) / arrow 58, always, across AdaWorldAPI forks, usually via [patch -> upstream repository git]. Measured before acting: the only DF-53 source was deltalake 0.32.4 (^53.1.0) behind the already-non-default, already-broken delta feature; registry check shows no DF-54 deltalake exists. Removed: delta feature, deltalake + url optional deps, DeltaTableReader (module docs + Cargo.toml carry dated removal notes; DataSourceFormat::Delta stays as a catalog tag). Post-removal Cargo.lock: exactly ONE datafusion = 54.1.0, zero deltalake entries. cargo check -p lance-graph green; fmt clean; full suite running as the PR gate. Docker pins surveyed: root + avx512 = Rust 1.97.1 + protobuf-compiler, no delta references — removal is docker-safe (stale flag noted: crates/symbiont/Dockerfile rust:1.95). Also: RP-SEAL research charter committed as .claude/plans/erasure-seals-compaction-research-v1.md (15-researcher program, independent pass dispatched as background workflow wf_ca974718-1b4; adversaries strongest-tier, builders/scouts grindwork tier); INTEGRATION_PLANS + STATUS_BOARD entries; D-LOTUS-6 BLOCKER lifted (operator-sanctioned upstream-git source consult; exact v9.0.0 tag on disk matching the lock checksum) with the audit carrying the dated lift note; CLAUDE.md 'BOTH MAJORS ARE REQUIRED' note superseded in place. Co-Authored-By: Claude <noreply@anthropic.com> * scope pivot (operator): rustynum struck — everything ndarray; symbiont deprecated (#879/#911/#912/#913) Recorded in the RP-SEAL plan header + the pin-ruling EPIPHANIES entry; the workflow script was corrected in place (source map + the two cell briefs naming rustynum); the in-flight independent pass could not be force-stopped in this harness build, so the ruling binds consolidation as a hard filter. Deltalake removal ratified same exchange. Co-Authored-By: Claude <noreply@anthropic.com> * correct the delta-removal notes: delta-rs MAIN is already DF 54 + arrow 58 Operator-pointed, verified from delta-io/delta-rs main Cargo.toml (datafusion = 54.0.0, arrow = 58): only the crates.io releases top out at DF 53. Restoration is available now via a git-pin on upstream + a reader refactor to the current builder API — as its own deliberate PR if a consumer needs Delta. The removal itself stands on the need ruling ('we don't need deltalake'), not on availability; notes in Cargo.toml, CLAUDE.md, and the EPIPHANIES entry corrected accordingly. Co-Authored-By: Claude <noreply@anthropic.com> * profile.dev debug=0 — smaller/faster builds (operator ruling) Full-debuginfo test binaries grew target/ to 17 GB and SIGBUS'd the linker on a full disk this session; debug=0 shrinks the lance/datafusion-stack test binaries ~an order of magnitude. Matches the gate already running with CARGO_PROFILE_DEV_DEBUG=0 (same resolved profile, cache reuse). line-tables-only noted as the fallback if line-numbered backtraces are ever needed. Co-Authored-By: Claude <noreply@anthropic.com> * drift cleanup: rustynum/symbiont no-go applied to live surfaces - lance-graph-cognitive: rustynum_accel was a name-only shim (zero rustynum dependency); renamed simd_accel (module + file + 11 call sites), rerouted through the sanctioned ndarray::simd re-export instead of ndarray::hpc::bitwise. Default compile green; the wip feature's pre-existing not-yet-compiling state has zero errors naming the rename. - docs/lotus audit: symbiont::domino::morton4 candidate row struck (census history only). - CLAUDE.md: symbiont removed from the binding-consumer lists via dated annotations. - EPIPHANIES: cleanup recorded in the pin-ruling entry. Audit also confirmed NO redo needed: A1's research report is clean (0 citations), the Java/Panama arc never touched either (ndarray::simd enforced), board history stays append-only. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <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
2 participants
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.
Implements Phase A of the canonical persistence contract (operator ruling 2026-08-09), superseding the cycle-persistence model merged in #911. Canonical record:
.claude/plans/persistence-artifact-backed-commit-v1.md.The governing rule
No artifact-backed semantic change → no write → no new
DatasetVersion.Thinking is cheaper than persisting its intermediate control flow. A thought runs its whole Rubicon/Heckhausen ladder transiently and only a semantic artifact becomes durable; kanban progress rides along in the commit that was happening anyway:
Mechanically the gate was already in the data — the cast's payload. Non-empty = artifact cast (persisted); empty = intent-only cast (held-intent re-stage, pure kanban step) which
persist_cyclepartitions out as ephemeral. Zero artifact casts ⇒CommitOutcome::NoChangewith the sink never called. This is whyrestage_held's empty payload is not a gate violation to pad around: it is the ephemerality mechanism, and it dissolves both post-merge P1s at once (an intent-only cast can never trip a payload gate it never reaches; the empty-cycle version disappears because nothing is called). #911's deliberate empty-cycle versioning is removed and its falsifier inverted.One logical writer — split by capability
Clone + &selfstays on producer submission and read-only projections (fire-and-forget: producers receive no acknowledgement). The concreteLanceCycleWriteris non-Clone, owns a long-livedDatasethandle + in-memory head, and commits through&mut self— two application commits cannot interleave through the type boundary, and the sole writer fully honors every result. It replacesLanceCycleSink(which held only a path and reopened per operation).No rollback, no compensating delete
A published manifest is history:
Dataset::deletemints another version and is not rollback — and #911's(cycle, base_version)predicate could destroy a concurrent same-cycle winner's rows. Measured, not assumed: lance 9 has no atomic expected-version fence forAppend(the conflict rebase runs even single-attempt; strict mode isOverwrite-only —lance-9.0.0/src/io/commit.rs:914-950). That is stated honestly rather than dressed as compare-and-swap.Instead idempotency is durable and in-band:
(cycle, batch_hash)commits with the rows and is reconciled first, so re-submitting the same frozen batch after a lost acknowledgement returnsReconciledinstead of double-appending. Honest states —NoChange/Committed/Reconciled, andFenced/HashConflict/Io/Ambiguous. No error promises "nothing landed" when failure could have followed publication.Zero reload on the normal path, instrumented
LanceCycleWriter::opens()counts everyDataset::openever performed (startup + ambiguity resolution only); a falsifier drives three commits and asserts it stays flat. Reads are bounded (scan_sealed(after_cycle)pushed into the scan;recover_fleettakes the bound too — the unbounded full-history scan is gone) and projected (timeline()never touches the payload column; landing rows carry no payload;scan_imageprojects payload on request).Layout: frame row (1/cycle) · landing metadata (1/artifact cast, payload NULL) · coalesced image (1 per dirty row, the final 512-byte payload). Measured consequence: 64 transient breaths on one row cost 512 durable bytes, not 64 × 512.
Gates
cycle_sink.rs(every one against a freshLanceCycleWriter::openover the same path) + 5 contract falsifiers inpersist_sink.rs.cycle_drivertests green under the new contract; planner suite 353 green;cargo fmtclean; clippy clean on default features (thedeltafeature's pre-existing deltalake-0.32 breakage is untouched and unrelated).Honestly deferred (named, not skipped)
awsfeature — ourlance = "=9.0.0"default-features pin already enables it (verified:aws-config/aws-credential-typesreachlance-io→lance→lance-graph), so ans3://store compiles and routes today. What is unmeasured is the credentialed commit / reconciliation-after-lost-response / bounded tail read. No object-store durability claim is made until that runs.to_vecreadback) is documented and isolated for a later measured PR, along with theBatchWriter<P>-descriptor-vs-Vec<u8>contradiction.Board hygiene (same commit)
New canonical plan;
⊘ PARTIALLY SUPERSEDEDheader onpersistence-cycle-wal-bootstrap-v1.md(guarantee 5 now conditional, 1–4/6 survive, §2 sparse-delta now implemented); LATEST_STATE + PR_ARC entries marking the #911 entry superseded (with its confidence line annotated rather than rewritten); INTEGRATION_PLANS entry; two EPIPHANIES.🤖 Generated with Claude Code
https://claude.ai/code/session_01KCGhDYoQBXs3poaR7sFuqp
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes