Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions .claude/board/EPIPHANIES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,74 @@
## 2026-08-09

## E-A-RECONCILED-HEAD-IS-NOT-A-PUBLICATION-1 (2026-08-09)

**Status:** FINDING. **Confidence:** high — falsified both ways.

Renaming a field is not the same as removing the confusion it names.
`CommitOutcome::Reconciled.current_head` was deliberately NOT called
`version` (#912) precisely because it is the store head AT RECONCILIATION
TIME. One layer up, `seal_cycle` then adopted it as `SealedCycle.version`
anyway — so retrying cycle 1 while the head stood at V5 recorded cycle 1
as "sealed into V5". The careful name survived; the meaning did not.

The repair is structural, not documentary: `SealedCycle` now carries
`publication_version: Option<DatasetVersion>` (`Some` ONLY for a fresh
`Committed`) beside `observed_head: Option<DatasetVersion>` (`Some` only
when a sink actually observed one — `None` on `NoChange`, which calls no
sink and whose `head` is the caller's asserted base). A publication
position that was never observed stays **unknown**; the durable identity
is `(cycle, batch_hash)` and the position is an audit-path read.

**The general shape:** when a type distinguishes two things a consumer
will conflate, the distinction has to be *unrepresentable* at the
consumer, not merely *documented* at the producer. A doc comment warning
"this is not X" is evidence that the next layer will use it as X.

Falsifier: `a_reconciled_retry_never_reports_the_current_head_as_publication`
(cycle 1 → V1, cycle 2 → V2, retry cycle 1 → `Reconciled`, observed head
V2, publication `None`, zero extra appends).

## E-A-DOC-COMMENT-IS-NOT-AN-ENFORCEMENT-1 (2026-08-09)

**Status:** FINDING. **Confidence:** high.

`FleetRecovery::foreign_min_cycle` shipped with the correct rule written
in its doc comment: *"the caller must NEVER raise its durable
`after_cycle` bound to or past this cycle."* A rule stated in prose to a
caller who holds a plain `Option<CycleId>` is an instruction, not a
guard — the unsafe checkpoint stays one obvious line away.

`FleetRecovery::checkpoint_bound(recovered_through)` is the same rule as
an API: it returns the bound the caller MAY store, capped strictly below
any foreign landing. The caller can still ignore it, but the safe path is
now the shortest one.

Companion: the same session made the writer's single-owner claim
enforceable rather than narrated (lexical `store_identity` so
`x/./s.lance` cannot claim a second slot beside `x/s.lance`; an RAII
`WriterClaim` taken before the first `.await`, so a cancelled `open`
cannot leak a reservation). Both are the same move — *carry the
invariant in a value, not in a sentence.*

## E-A-A-PERMANENT-FAULT-REPORTED-AS-RETRYABLE-IS-AN-INFINITE-LOOP-1 (2026-08-09)

**Status:** FINDING. **Confidence:** high.

A 511-byte artifact payload violates the writer's 512-byte ABI. It was
reported as `CommitError::Io` — the variant whose documented meaning is
"nothing published, safe to REGENERATE". A caller obeying that contract
regenerates the identical malformed batch and fails identically, forever:
the error classification, not the bug, is what makes it unbounded.

`CommitError::InvalidArtifact { row, len }` is permanent by construction.
The taxonomy rule this instance teaches: **an error variant's retry
semantics are part of its contract**, so a producer-side defect must never
borrow a transport-side variant merely because both mean "did not
commit".

Falsifier: `a_malformed_artifact_is_refused_permanently_not_as_retryable_io`
(refused twice, identically, store untouched).

### E-THE-ARTIFACT-WRITE-DECIDES-WHAT-KANBAN-PROGRESS-BECOMES-DURABLE-1

**FINDING (operator-ruled, implemented Phase A).** The persistence question was
Expand Down Expand Up @@ -15591,3 +15660,4 @@ Cross-ref: W2's sprint-2 deliverable (Tier-0 "what's shipped" index); `.claude/p
Three review findings on the #629 doc arc, accepted and folded into the V3 docs (routing.md §1/§5, mailbox-kanban-model.md, INTEGRATION-PLAN W6a): (1) **Clustered-index caveat** — `NodeGuid` stores classid via `to_le_bytes`, so RAW key-byte prefixes order by the custom byte first; domain range-scans hold over the DECODED u32 (or an order-preserving big-endian rendering), never raw LE byte prefixes/tries (E-CLASSID-CANON-HIGH-IS-A-CLUSTERED-INDEX reads with this caveat). (2) **Corpus-proof scanner scope** — "old-form" = ALL THREE legacy shapes: `0x0000_DDCC`, `0x1000_DDCC`, AND `0xAAAA_DDCC` (legacy app/render prefix high, e.g. `0x0005_0901`) — exactly the set `classid_canon_compat` routes CanonLow; scanning only the first two can falsely prove the corpus clean (E-V3-MARKER-IS-A-MONITOR reads with this scope). (3) **ractor wording sharpened** — "compile-time ownership dummy / never a hot-path bus" scopes the DATA plane; the actor mailbox remains the runtime serialized single-writer CONTROL path (one-message Advance/MulAdvance serialization = the codex #578 atomicity mechanism). The operator ruling's meaning is unchanged; the wording now says both halves.

Cross-ref: PR #629 review threads (2 codex P2 + 1 coderabbit); E-MAILBOX-KANBAN-NO-COLLAPSEGATE.

33 changes: 33 additions & 0 deletions .claude/board/PR_ARC_INVENTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -3157,3 +3157,36 @@ Removes `crate-ci/typos` spell-check job from `style.yml`; `cargo fmt --check` r
- LATEST_STATE Current Contract Inventory annotated with `WitnessTable` + `MailboxSoA` additions.

**Confidence (2026-05-28):** working — `cargo check -p cognitive-shader-driver -p lance-graph-contract` clean (only pre-existing ontology deprecation warnings); `cargo test -p cognitive-shader-driver -p lance-graph-contract --lib` 457 passed / 0 failed at merge time. Codex P1 (`f541b280`) addressed before merge. No consumer surfaces touched.

## Follow-up gates after #912 (branch `claude/persistence-follow-up-gates`, 2026-08-09)

**Added.** `SealedCycle.publication_version` / `observed_head` (both
`Option`, replacing the conflated `version`); `FleetRecovery::checkpoint_bound`
(the enforceable latecomer fence); `CommitError::InvalidArtifact` (permanent,
never retryable); `store_identity` + RAII `WriterClaim` (lexical store identity;
claim held before the first `.await`); `LanceCycleWriter::max_cycle` (streaming
O(1)-memory startup seed, replacing `timeline().max()`); full schema guard
(types + nullability, not names only); a `#[cfg(test)]` fault-injection seam for
the append/reopen/reconcile branch.

**Falsifiers added (8).** Reconciled-retry-never-claims-publication;
checkpoint_bound both ways; permanent-InvalidArtifact (twice, identically);
alternate path spellings refused; failed open leaks no reservation; injected
unpublished append → Io + regenerable; injected published append → Reconciled,
exactly one durable frame; injected reconcile-read failure → Ambiguous, resolved
by re-submitting the same frozen batch.

**Also.** `scan_sealed` is payload-free BY CONTRACT — documented on the trait
and modelled by all seven fakes (they previously proved a property the real
writer lacks); `persist_cycle`'s `NoChange.head` provenance documented;
`content_hash`'s inclusion of `base_version` documented as deliberate (a
re-derived frame is a DIFFERENT assertion and must fail closed, not launder).

**Deferred (named, not silently dropped).** The typed
`IntentOnly | Artifact512` boundary (the gate still tests payload PRESENCE);
the first-Create ambiguity state machine (an unknown `Create` still treats a
later `NotFound` as absence); `run_cycle`'s borrow-over-`.await` prose; the
landing-row rollup. These are Phase-B/C/D work, tracked in the plan.

**Confidence:** high on everything with a falsifier; the deferrals are the
honest remainder.
2 changes: 1 addition & 1 deletion .claude/plans/persistence-artifact-backed-commit-v1.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ competition.**
## 3. No rollback, no compensating delete — reconciliation is authoritative

**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`
(`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.
Expand Down
14 changes: 8 additions & 6 deletions .claude/plans/persistence-cycle-wal-bootstrap-v1.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,14 @@ two-dimensional upgrade path is §3–§4; the accepted debts are §5.

---

## 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, #912):** the sparse-delta rule
> is **RATIFIED as architecture** and **IMPLEMENTED** in
> `lance_graph::graph::cycle_sink::LanceCycleWriter` (Phase A) — landing
> metadata rows + one coalesced image row per dirty row. The #878 bootstrap
> remains SHIPPED; the body text below predates the concrete sink and is kept
> append-only.

**"One complete cycle image" must NEVER be read as serializing every row merely
because every participant belonged to the cycle.** The load-bearing distinction:
Expand Down Expand Up @@ -372,7 +374,7 @@ solves the final temporal model:
| Shadow temporal-coherence correction pass | **PLANNED** (§4) |
| `revision.rs` forward-correction mechanism | **PLANNED** (§4) |
| Concrete Lance sink (real crash durability) | **DEFERRED** — gated on crash falsifiers (§5) |
| 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 #912, §2) |

The bootstrap exists so the rest can be built on a running, durable seam. The
scalar order is a load-bearing placeholder, and this document is the record that
Expand Down
7 changes: 6 additions & 1 deletion crates/lance-graph-planner/examples/blw_fusion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,12 @@ impl WalSink for MemWal {
.flat_map(|s| {
s.landings.iter().map(|slot| LandedSlot {
cycle: s.cycle,
slot: slot.clone(),
// `scan_sealed` is payload-free by contract; payloads
// live in the image read.
slot: SweepSlot {
payload: Vec::new(),
..slot.clone()
},
})
})
.collect())
Expand Down
7 changes: 6 additions & 1 deletion crates/lance-graph-planner/examples/blw_tenant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,12 @@ impl WalSink for MemWal {
.flat_map(|s| {
s.landings.iter().map(|slot| LandedSlot {
cycle: s.cycle,
slot: slot.clone(),
// `scan_sealed` is payload-free by contract; payloads
// live in the image read.
slot: SweepSlot {
payload: Vec::new(),
..slot.clone()
},
})
})
.collect())
Expand Down
46 changes: 43 additions & 3 deletions crates/lance-graph-planner/src/persist_sink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,10 @@
//! real durability. **`compile+test green ≠ storage proven`** (the Ladybug
//! lesson). The concrete sink is `lance_graph::graph::cycle_sink::LanceCycleWriter`.
//!
//! ## 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.
//!
//! **No artifact-backed semantic change → no write → no new [`DatasetVersion`].**
//!
Expand Down Expand Up @@ -270,6 +272,15 @@ pub enum CommitError {
/// I/O failed with provably NOTHING published — safe to regenerate from
/// the unchanged horizon.
Io(WriteFailed),
/// An artifact payload violates the concrete writer's binary ABI (the
/// canonical witness row is exactly 512 bytes). **PERMANENT, not
/// retryable:** nothing was written, and re-submitting or regenerating
/// the same malformed batch can never succeed — classifying this as
/// [`Io`](CommitError::Io) sends the caller into an endless regenerate
/// loop. Fix the producer. (The [`persist_cycle`] artifact gate tests
/// payload PRESENCE only; the size ABI is enforced by the writer — the
/// typed `IntentOnly | Artifact512` split is the Phase-B refinement.)
InvalidArtifact { row: u64, len: usize },
Comment on lines +275 to +283

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add InvalidArtifact to the PersistError::Commit retry-class doc.

The doc on PersistError::Commit at lines 438-444 enumerates the honest sub-states for retry logic: Fenced, Io, Ambiguous, and HashConflict. It does not mention InvalidArtifact. A caller that builds its retry policy from that list finds no rule for the new variant. The whole purpose of InvalidArtifact is to stop the endless regenerate loop, so the classification must be reachable from the type the caller actually matches on.

📝 Proposed doc addition on PersistError::Commit
     /// The durable commit did not yield an outcome — see [`CommitError`] for
     /// the honest sub-states ([`Fenced`](CommitError::Fenced) = regenerate
     /// against the new head; [`Io`](CommitError::Io) = nothing landed, safe
     /// regenerate; [`Ambiguous`](CommitError::Ambiguous) = re-submit the SAME
     /// frozen batch, reconciliation decides; [`HashConflict`](CommitError::HashConflict)
-    /// = fail closed).
+    /// = fail closed; [`InvalidArtifact`](CommitError::InvalidArtifact) =
+    /// PERMANENT, never retry — fix the producer).
     Commit(CommitError),
🤖 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 275 - 283,
Update the retry-classification documentation on PersistError::Commit to include
CommitError::InvalidArtifact alongside Fenced, Io, Ambiguous, and HashConflict,
explicitly identifying it as permanent and non-retryable so callers stop
regeneration attempts for malformed artifacts.</code>

/// The append's outcome could not be determined AND reconciliation itself
/// failed. Re-submit the SAME frozen batch: `commit_cycle` reconciles
/// first, so the retry cannot double-append.
Expand All @@ -295,6 +306,11 @@ impl std::fmt::Display for CommitError {
"cycle {cycle:?} durable with hash {stored_hash:#018x}, offered {offered_hash:#018x} — fail closed"
),
Self::Io(e) => write!(f, "commit I/O (nothing published): {e}"),
Self::InvalidArtifact { row, len } => write!(
f,
"artifact payload for row {row} is {len} bytes, violating the writer's \
ABI — permanent, fix the producer (nothing written, do NOT retry)"
),
Self::Ambiguous {
cycle,
batch_hash,
Expand Down Expand Up @@ -374,6 +390,14 @@ impl DetachedCycleBatch {
}

/// FNV-1a 64 over the frame identity + canonical landing content.
///
/// **The EXACT frame — `base_version` included — is part of the
/// idempotency identity, deliberately.** A retry after a lost
/// acknowledgement must resubmit the SAME frozen [`DetachedCycleBatch`],
/// never re-freeze from a re-derived frame: a caller that re-reads the
/// head and re-freezes has changed what it is asserting, and reconciling
/// that as "the same batch" would launder the divergence. Such a resubmit
/// fails closed ([`CommitError::HashConflict`] / `Fenced`) by design.
fn content_hash(frame: CycleFrame, canonical: &[SweepSlot]) -> u64 {
const OFFSET: u64 = 0xcbf2_9ce4_8422_2325;
const PRIME: u64 = 0x0000_0100_0000_01b3;
Expand Down Expand Up @@ -533,6 +557,14 @@ pub trait WalSink {
/// canonical order — this seam does NOT sort. `after_cycle` bounds the read
/// to cycles strictly after it (the recovery tail bound); implementations
/// push the bound into the storage scan, never full-scan-and-filter.
///
/// **Returned landings carry NO payload** (`SweepSlot::payload` is empty):
/// landing rows are transition METADATA; durable payloads live only in the
/// coalesced image read. Consequently an empty payload on a recovered
/// landing does NOT mean "intent-only" — that classification (the artifact
/// gate) applies to live casts in [`persist_cycle`], never to recovered
/// slots. Fakes must model this too, or they prove a property the real
/// writer does not hold.
async fn scan_sealed(
&self,
after_cycle: Option<CycleId>,
Expand Down Expand Up @@ -589,6 +621,9 @@ pub async fn persist_cycle<S: WalSink>(
.filter(|c| !c.payload.is_empty())
.collect();
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,
});
Expand Down Expand Up @@ -860,6 +895,8 @@ mod tests {
after_cycle: Option<CycleId>,
) -> Result<Vec<LandedSlot>, WriteFailed> {
// Returned in STORED order — no sort. (The order was fixed at seal.)
// Payload-free by contract: landing rows are metadata; the fake
// must model the production projection, not improve on it.
Ok(self
.sealed
.lock()
Expand All @@ -869,7 +906,10 @@ mod tests {
.flat_map(|s| {
s.landings.iter().map(|slot| LandedSlot {
cycle: s.frame.cycle,
slot: slot.clone(),
slot: SweepSlot {
payload: Vec::new(),
..slot.clone()
},
})
})
.collect())
Expand Down
14 changes: 10 additions & 4 deletions crates/lance-graph-supervisor/examples/measure_wal_curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,8 @@ mod measure {
cycle: frame.cycle,
batch_hash: 0,
},
version: Some(version),
publication_version: Some(version),
observed_head: Some(version),
transitions,
next_position_base,
}
Expand Down Expand Up @@ -1454,12 +1455,13 @@ mod measure {
batch: DetachedCycleBatch,
) -> Result<CommitOutcome, CommitError> {
let mut sealed = self.sealed.lock().expect("MemWal poisoned");
let head = sealed.last().map_or(DatasetVersion(0), |s| s.version);
// Reconciliation-first: an already-durable (cycle, hash) is success,
// a matching cycle with a different hash fails closed.
if let Some(rec) = sealed.iter().find(|s| s.frame.cycle == batch.frame.cycle) {
return if rec.batch_hash == batch.batch_hash {
Ok(CommitOutcome::Reconciled {
current_head: rec.version,
current_head: head,
cycle: batch.frame.cycle,
batch_hash: batch.batch_hash,
})
Expand All @@ -1471,7 +1473,6 @@ mod measure {
})
};
}
let head = sealed.last().map_or(DatasetVersion(0), |s| s.version);
if batch.frame.base_version != head {
return Err(CommitError::Fenced { current_head: head });
}
Expand Down Expand Up @@ -1504,7 +1505,12 @@ mod measure {
.flat_map(|s| {
s.landings.iter().map(|slot| LandedSlot {
cycle: s.frame.cycle,
slot: slot.clone(),
// `scan_sealed` is payload-free by contract; payloads
// live in the image read.
slot: SweepSlot {
payload: Vec::new(),
..slot.clone()
},
})
})
.collect())
Expand Down
Loading
Loading