Skip to content

[PR] fix(storage): align the envelope tail leg to the row tail by suffix, so folded-envelope verification can converge (Closes #6349) - #6350

Open
dimas623 wants to merge 1 commit into
ranxianglei:masterfrom
dimas623:2026-11-12_folded-tail-suffix-align
Open

dimas623 wants to merge 1 commit into
ranxianglei:masterfrom
dimas623:2026-11-12_folded-tail-suffix-align

Conversation

@dimas623

Copy link
Copy Markdown

What

BatchedViewVerification.compareMessages compared the two storage legs positionally from index 0. They do not hold the same slice of the conversation:

  • the table leg holds tail_json.tailSeqs — the raw resident tail, oldest-first, the whole lastMessages window;
  • the envelope leg holds boundedFoldedSnapshot(session) — a bounded suffix of that tail, because buildRecord always writes messagesFolded: true and boundedFoldedSnapshot drops the oldest messages until persist.tailTokens (default 16384) is satisfied.

One leg is a truncated copy of the other with the same tail end, so the correct mapping is expectedIndex = index - (seqs.length - expected.length). Comparing from 0 makes every slot mismatch as soon as the tail outgrows the envelope budget.

That mismatch is deterministic, so the sampled verification fails on every sample, driftedSessions latches, #5677 forces the full ingest path on every save, and the envelope gets frozen at DRIFT_ESCALATE_STREAK. On a ~2 200-message session the full path is a synchronous 8–9 s block; one measured episode blocked the event loop for 170.8 s across 24 consecutive ingests. Nothing is corrupted — integrity.ok stays true and the rows, refs and block ledger are intact. It is a digest-shape defect in the verifier.

Measured on two live sessions (SQLite read-only + envelope JSON, 6 samples at 5 s; offset stable, overlap element-wise identical in every sample):

session tailSeqs rows envelope messages offset ids missing from tail digest mismatches in overlap
A 1095 10 1085 0 0
B 752 22 730 0 0

Forcing the old positional comparison over one such sample yields 25/25 mismatching slots plus 1127 "tail-only" slots on A, 38/38 plus 794 on B — biased toward permanent failure by construction.

The fix

const tailLen = seqs?.length ?? 0;
const offset = expected === undefined ? 0 : Math.max(0, tailLen - expected.length);
…
const wantIndex = index - offset;
const want = expected !== undefined && wantIndex >= 0 && wantIndex < expected.length
  ? messageSignature(expected[wantIndex])
  : undefined;
if (want !== undefined && want.digest !== rec.sig.digest) { … }
  • Slots outside the envelope window are no longer flagged — they are not comparable, by construction.
  • Divergence inside the window is still detected. The regression test corrupts a single content row inside the overlap and asserts the drift is reported, so this is not a blanket mute.
  • The trailing-length check becomes expected.length > tailLen: the envelope cannot hold more messages than the tail it claims.
  • A tailSeqs pointer with no row now reports drift (tailGaps++ and changed() and index++) instead of only bumping a counter. That means the table leg is shorter than the tail it claims (#4390) — a real defect, not an alignment artifact. Without this, tests/storage-batched-verification.test.ts ("missing and retired tail rows remain visible as drift") regresses, which is how it was caught.

Why not gate on tailFolded

Tried first: tail_json.tailFolded is written from session.lastMessagesFolded (src/storage/ingest.ts), which only foldResidentTail sets, while the envelope marks messagesFolded from buildRecord whenever a snapshot exists at all. On both live sessions tailFolded is false in every row while the envelope is genuinely folded — the guard would never fire on real data. The suffix alignment does not depend on that flag.

Verification

  • New test tests/folded-tail-suffix-alignment.test.ts (2 cases): a folded envelope that is a suffix of the row tail verifies clean; a real content divergence inside the overlap still reports drift.
  • Related storage suites, 10 files / 43 tests, all pass: storage-batched-verification, issue5677-drift-field-diag, issue4323-drift-escalation, issue5408-frozen-recovery, issue5412-async-drift-latch-parity, issue4986-digest-drift-ref-order, storage-bounded-load, storage-canonical-digest, issue4392-canonical-key-order, folded-tail-suffix-alignment.
  • tsc --noEmit --project tsconfig.json clean; node scripts/check-test-import-graph.mjs clean; node scripts/check-unused-exports.mjs clean.
  • Negative control: forcing offset = 0 fails the new test with tail slot 0: expect raw-32/user len=70 sha=45d5582d, got raw-0/user len=68 sha=853761ed while the divergence case still passes.
  • Live proxy after restart with the patch: DIGEST DRIFT = 0, ESCALATED = 0, envelope writes FROZEN = 0, driftedSessions = 0; slow-ingest path=full went from 24 × 8–9 s to a single cold-start ingest, everything else path=delta at 530–932 ms. Cache hit stayed 97–99 %.
  • Branch rebased onto origin/master c26c31ac; git rev-list --left-right --count origin/master...HEAD = 0 1, and no upstream commit in between touched src/storage/shadow.ts or src/storage/ingest.ts.

No version bump — per AGENTS.md §4 that only happens on *_release-v* branches.

Closes #6349

@ranxianglei ranxianglei changed the title fix(storage): align the envelope tail leg to the row tail by suffix, so folded-envelope verification can converge (Closes #6349) [PR] fix(storage): align the envelope tail leg to the row tail by suffix, so folded-envelope verification can converge (Closes #6349) Nov 12, 2026
The sampled read-back verifier compared the two legs of a session's tail
positionally from index 0, but they intentionally hold different SLICES of the
same log:

  - the rows hold the raw tail interned at save time (`tailSeqs`, oldest-first,
    the whole resident window), and
  - the envelope holds `boundedFoldedSnapshot`, which prunes from the FRONT and
    keeps the NEWEST messages under `persist.tailTokens` (ranxianglei#2960 (5), caveat 2a).

Measured on live sessions the envelope is an exact element-wise SUFFIX of the row
tail (tail=1095/envelope=10 and tail=752/envelope=22, zero id and zero content
mismatches inside the overlap, stable across a six-sample sweep). Walking both
legs from index 0 therefore compared the oldest row against the oldest RETAINED
message — off by `tail.length - messages.length` — so `want` was undefined for
every slot outside the envelope window and `changed("lastMessages")` fired for
every tailSeq on every sampled save.

The sampled digest could then never converge: the streak latched (ranxianglei#5677), the
delta fast path was disabled for that session by design, and every save paid a
FULL ingest. Measured on one affected session: 44 full ingests, avg 4522 ms, max
9363 ms, 170.8 s of stalled event loop in a single episode.

Fix: align the envelope to its suffix position and compare only the overlap. A
genuine divergence inside the window still reports; a leg that is not a suffix
produces mismatches rather than silence. Row PRESENCE is still enforced for every
seq (ranxianglei#4390) — a recorded tail pointer with no row now also raises `changed()`,
which it previously did not, and the loop advances its index on that branch so the
suffix offset cannot drift when a gap is present.

Absence of a whole leg stays with the header digest, which already distinguishes
`undefined` from `[]` via `withoutBulk`; it is not re-litigated here.

Tests: tests/folded-tail-suffix-alignment.test.ts pins both halves — a row tail
longer than the envelope with a matching overlap verifies clean, and a content
change inside the overlap still latches drift.
@dimas623
dimas623 force-pushed the 2026-11-12_folded-tail-suffix-align branch from 80bdaf8 to 3a196c7 Compare November 12, 2026 07:38
@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷

Review of PR #6350 (Closes #6349) — mechanism concern

I've traced the code path carefully and have a concern about the mechanism claim. I'll lay out what I found, then ask for clarification.

The verifier's expected is not the envelope

The issue claims that compareMessages compares the "envelope leg" (boundedFoldedSnapshot, e.g. 10 messages) against the "table leg" (tailSeqs, e.g. 1095). But the verifier's expected is this.legacy[key] = legacy.lastMessages, which is:

  • The rawTail (e.g. 1095) when rawTail is passed to shadow.record() (i.e. session.lastMessagesFolded !== true).
  • The envelope (e.g. 10) when rawTail is not passed (i.e. session.lastMessagesFolded === true).

The issue itself confirms that tailFolded is false in every row on the live sessions, which means lastMessagesFolded was false at ingest time, which means rawTail was passed. So the verifier's expected is the rawTail (1095), not the envelope (10).

In the raw-save case, the verifier compares 1095 against 1095 → clean.
In the folded-save case, the ingest is full (10 < 1095), which rewrites tailSeqs to 10 before the verifier runs → the verifier compares 10 against 10 → clean.

I traced all code paths (raw save, folded save, delta, full, restart, convergeToDisk, restoreReplica) and cannot construct a scenario where expected.length !== seqs.length in the current master.

The drift log is internally inconsistent with the mechanism claim

The drift log shows "tail slot 1964", which requires seqs.length > 1964 and expected.length > 1964. But the mechanism claim says expected is the envelope (e.g. 10), which is < 1964. So the drift log is inconsistent with the mechanism claim.

The performance improvement is real but unexplained

The PR reports a real performance improvement (24×8-9s full ingests → single cold-start). But if the verifier was already reporting clean, what's causing the slow full ingests?

Could you clarify:

  1. What is the actual sequence of events that leads to the repeated full ingests?
  2. Is there a log or trace that shows the verifier reporting drift (verifyDrift > 0) before the patch, with the full context (what is legacy.lastMessages at that time — its length, and whether rawTail was passed)?

The code changes

The suffix alignment logic is reasonable in isolation, and the missing-row drift detection is a correct behavior change (missing rows are a real inconsistency). But if the bug is not real in the current master, the PR is fixing a non-existent problem.

I'd like to understand the actual failure mode before merging.

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷

Follow-up: CI green, tests pass locally

I've verified:

  • The PR's CI is green (18 success, 12 skipped on head 3a196c7fe3).
  • The new tests pass locally (tests/folded-tail-suffix-alignment.test.ts: 2/2 pass).

I'm still waiting for clarification on the mechanism. The core question remains: what is legacy.lastMessages at the time of the drift report?

  • If it's the rawTail (e.g. 1095), the verifier compares 1095 against 1095 → clean (no drift).
  • If it's the envelope (e.g. 10), the ingest is full (10 < 1095), which rewrites tailSeqs to 10 before the verifier runs → the verifier compares 10 against 10 → clean (no drift).

In both cases, the verifier should report clean. So I cannot construct a scenario where the verifier reports drift in the current master.

Could you provide a log or trace that shows the verifier reporting drift, with the full context:

  1. What is legacy.lastMessages at that time — its length, and whether rawTail was passed (i.e. was session.lastMessagesFolded true or false)?
  2. What is the stored tailSeqs length at that time?
  3. What is the tailFolded value in the row at that time?

This will help me understand the actual failure mode.

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷

Review verdict — PR #6350 (external, @dimas623)

PR: fix(storage): align the envelope tail leg to the row tail by suffix (Closes #6349)

What I did:

  1. Read the full diff (2 files: src/storage/shadow.ts +46/-7, new tests/folded-tail-suffix-alignment.test.ts 187 lines).
  2. Traced the verifier mechanism end-to-end against master (shadow.ts:1535 BatchedViewVerification → persist.ts buildRecord/foldResidentTail/main-save → session.ts:1708 snapshotMessages → ingest.ts:212 planIngest/planIngestDelta → store.ts:867 ingestSession).
  3. Fetched and analyzed issue [Bug][storage] Folded envelope tail is a suffix of the raw tail, but the sampled digest compares from index 0 — permanent DIGEST DRIFT latch and 8–9 s full ingest per save | 折叠信封尾是原始尾的后缀,但采样摘要从下标 0 起比对 → 漂移永久锁存 + 每次保存 8–9 秒全量 ingest #6349 (the bug report this PR claims to fix).
  4. Ran the new tests locally: 2/2 pass.
  5. Checked CI: green (18 success, 12 skipped on head 3a196c7fe3).

Key finding — the PR's mechanism claim is incorrect:

The PR claims the verifier's expected is the envelope (boundedFoldedSnapshot, e.g. 10 messages) while the stored rows hold the full raw tail (e.g. 1095). I cannot confirm this:

  • legacy.lastMessages at verify time is the rawTail when lastMessagesFolded === false (rawTail is passed to shadow.record), or the envelope when lastMessagesFolded === true (rawTail is not passed).
  • If lastMessagesFolded === true (envelope case), the ingest is full (envelope 10 < stored tailSeqs 1095), which rewrites tailSeqs to 10 before the verifier runs → verifier compares 10 vs 10 → clean.
  • If lastMessagesFolded === false (rawTail case), the verifier compares rawTail (1095) vs rows (1095) → clean.
  • The issue's own drift log (tail slot 1964) requires expected.length > 1964, which is inconsistent with expected being the 10-message envelope.

I traced all code paths (raw save, folded save, delta, full, restart, convergeToDisk, restoreReplica) and cannot construct a scenario where expected.length !== seqs.length in the current master.

The code changes are reasonable (suffix alignment + missing-row drift detection) and the tests are valid, but they fix a mechanism I cannot confirm exists in the current master. The performance improvement reported in the PR body (24×8-9s → single cold-start) is real but not explained by the stated mechanism.

Waiting on: PR author's response to my clarification request (posted above): what is legacy.lastMessages at drift time (length, rawTail passed?), stored tailSeqs length, and tailFolded value.

Verdict: Hold — mechanism unconfirmed. The code is safe to merge (no regression risk for clean cases: when lengths are equal, offset=0 and behavior is unchanged), but the bug it claims to fix is not reproduced in my code trace.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

2 participants