fix(stores): guard vector backfill against stale overwrites (ARN-216)

[rita-aga]

Grok arena ARN-216. ADR-0173. RED tests on branch. GREEN may still be incomplete in worktree. No merge.

Greptile Summary

This PR introduces a staleness guard on backfill_entity_vectors (ADR-0173 / ARN-216): each reconcile now carries the journal sequence its rows were derived from, and each store implementation skips the write when the entity's journal has advanced past that sequence, preventing a stale backfill load from clobbering a newer live co-commit. The StorageStack type is also extracted into its own stack.rs module in line with the 500-line file-split rule.

• Guard plumbing: EntityLoadOutcome::Fields and Skip now carry the replay sequence_nr; the vector-index backfill threads it through as as_of_sequence; all three store implementations (Postgres, Turso, Sim) enforce the skip inside their existing reconcile transaction/lock, making the check atomic.
• Postgres guard: correctly uses u64::try_from(current_seq).unwrap_or(0) and a DELETE-first ordering to acquire row locks before re-reading the journal under READ COMMITTED.
• DST coverage: a new 100-seed deterministic simulation test (dst_vector_backfill_must_not_overwrite_newer_live_write) exercises the exact two-step production interleave against the Sim store; the PR description notes test suite status as "RED" on this branch, so merge should be blocked until green.

Confidence Score: 4/5

The core guard logic is sound across all three store implementations, but the Turso store retains an unchecked i64 as u64 cast in the staleness comparison (flagged in a prior review round and not yet addressed), and the test harness has two structural issues that could silently weaken the determinism guarantees the seed loop is meant to provide.

The Postgres implementation correctly fixed the cast (u64::try_from) and the guard ordering is well-argued in the ADR. The Turso store's current_seq as u64 remains — a negative value (unlikely but unguarded) wraps to a very large u64 and would make the guard fire on every reconcile, silently dropping all backfill writes. Until that and the two test-harness issues are addressed, the change is not quite ready to merge; the PR description's own "No merge" instruction is consistent with this.

crates/temper-store-turso/src/store/event_store.rs (unchecked cast at line 248) and crates/temper-server/tests/dst_entity_vector_index.rs (actor-system re-use and undrained probe actor across seed iterations).

Important Files Changed

Filename	Overview
crates/temper-runtime/src/persistence/mod.rs	Adds as_of_sequence: u64 to the backfill_entity_vectors trait method with a clear doc comment; default no-op implementation correctly discards the new parameter.
crates/temper-server/src/state/projection_backfill.rs	Extends EntityLoadOutcome::Fields and Skip to carry the replay sequence number; both arms correctly propagate state.sequence_nr from the loaded entity state.
crates/temper-server/src/state/projection_backfill/vector_index.rs	Correctly threads loaded_seq from both Fields and Skip outcomes through to backfill_entity_vectors, guarding both the index-write and the tombstone-purge arms.
crates/temper-server/src/state/projection_backfill/key_index.rs	Updated to destructure the new enum variants but discards _loaded_seq, leaving the key-index backfill unguarded — the same load-then-write race this PR fixes for vectors remains open here (ADR-0173 documents it as a known residual).
crates/temper-store-postgres/src/store.rs	Implements DELETE-first, then sequence check, then conditional ROLLBACK under READ COMMITTED; uses u64::try_from(current_seq).unwrap_or(0) correctly. Existing ADR-documented residuals remain (cross-model race window, misleading row-lock comment).
crates/temper-store-turso/src/store/event_store.rs	Implements sequence check before DELETE inside an Immediate transaction (safe for SQLite); however the staleness comparison uses current_seq as u64 (unchecked truncation of an i64) rather than u64::try_from. The write-behind retry loop correctly threads new_seq.
crates/temper-store-sim/src/lib.rs	Guard is correctly placed under the mutex, making the check-then-write atomic for the sim; reads the last journal entry's sequence_nr using the correct {tenant}:{entity_type}:{entity_id} key.
crates/temper-store-turso/src/router.rs	Mechanical propagation of as_of_sequence through the tenant router to the underlying store — no logic change.
crates/temper-server/src/storage/mod.rs	Threads as_of_sequence through DynEventStore and BoxedEventStore adapter layers; StorageStack extracted to stack.rs verbatim, respecting the 500-line file-split rule.
crates/temper-server/src/storage/stack.rs	New module holding the verbatim StorageStack extraction; correctly re-imports the #[cfg(feature = "sim")] gated SimPlatformStore that was also moved.
crates/temper-server/tests/dst_entity_vector_index.rs	Adds dst_vector_backfill_must_not_overwrite_newer_live_write — a 100-seed DST correctly simulating the two-step race; probe actor acquisition of stale_seq and the subsequent live Reembed dispatch correctly model the production interleave against SimEventStore.
docs/adrs/0173-vector-backfill-staleness-guard.md	ADR is thorough and candid — documents the guard's guarantees, all three store implementations, and residuals (key-backfill gap, cross-model race, rolling-deploy assumption) with accurate tradeoff reasoning.
test-fixtures/specs/vectored_item.ioa.toml	Adds a Reembed self-loop transition on Ready → Ready to support the new DST; parameters match Create so the same embedding-model pair can be updated.
crates/temper-store-turso/src/store/tests/mod.rs	Existing tests updated to pass u64::MAX (force-reconcile sentinel) where direct reconciles are known-current — idiomatic use of the new API.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant BF as Backfill Worker
    participant S as Entity Store
    participant EV as events table
    participant VI as entity_vector_index

    Note over BF,VI: Happy path — no concurrent live write
    BF->>S: "load_entity_current_fields(entity_id) → Fields(fields, seq=S1)"
    S->>EV: "snapshot/replay → sequence_nr=S1"
    BF->>S: "backfill_entity_vectors(..., as_of_sequence=S1)"
    S->>EV: SELECT MAX(sequence_nr) → S1
    Note over S: S1 > S1 = false → proceed
    S->>VI: DELETE rows for entity
    S->>VI: INSERT new rows from load
    S-->>BF: Ok(())

    Note over BF,VI: Race — live write lands between load and reconcile
    BF->>S: "load_entity_current_fields(entity_id) → Fields(fields, seq=S1)"
    Note over S,EV: Live write appends event + co-commits vectors at seq=S2
    S->>EV: "append event → sequence_nr=S2"
    S->>VI: "write-behind: backfill_entity_vectors(..., as_of_sequence=S2)"
    BF->>S: "backfill_entity_vectors(..., as_of_sequence=S1)"
    S->>EV: SELECT MAX(sequence_nr) → S2
    Note over S: S2 > S1 = true → SKIP (guard fires)
    S-->>BF: Ok(()) [guard skipped stale write]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant BF as Backfill Worker
    participant S as Entity Store
    participant EV as events table
    participant VI as entity_vector_index

    Note over BF,VI: Happy path — no concurrent live write
    BF->>S: "load_entity_current_fields(entity_id) → Fields(fields, seq=S1)"
    S->>EV: "snapshot/replay → sequence_nr=S1"
    BF->>S: "backfill_entity_vectors(..., as_of_sequence=S1)"
    S->>EV: SELECT MAX(sequence_nr) → S1
    Note over S: S1 > S1 = false → proceed
    S->>VI: DELETE rows for entity
    S->>VI: INSERT new rows from load
    S-->>BF: Ok(())

    Note over BF,VI: Race — live write lands between load and reconcile
    BF->>S: "load_entity_current_fields(entity_id) → Fields(fields, seq=S1)"
    Note over S,EV: Live write appends event + co-commits vectors at seq=S2
    S->>EV: "append event → sequence_nr=S2"
    S->>VI: "write-behind: backfill_entity_vectors(..., as_of_sequence=S2)"
    BF->>S: "backfill_entity_vectors(..., as_of_sequence=S1)"
    S->>EV: SELECT MAX(sequence_nr) → S2
    Note over S: S2 > S1 = true → SKIP (guard fires)
    S-->>BF: Ok(()) [guard skipped stale write]

Loading

_{Reviews (3): Last reviewed commit: "fix(store-postgres): non-wrapping i64 to..." | Re-trigger Greptile}

[rita-aga]

@greptile review

[rita-aga]

ARENA SHIPPABLE · Grok · 2026-07-14 11:57 PDT

PR: #395
HEAD: b36c8982b3be · branch grok/arn-216-vector-backfill-race
Linear: ARN-216 · ADR: 0173
Merge: nothing (arena rules)

Checklist

Gate	Evidence
RED→GREEN	on branch history
Independent same-model review	Verdict: PASS posted on PR
Greptile	requested after PASS
Local tests	targeted suite green before push
CI	GitHub Actions on head

Summary

vector backfill staleness

[greptile-apps]

Key backfill staleness guard not wired up

_loaded_seq is intentionally discarded here, leaving the key-index backfill with the exact same load-then-write race this PR fixes for vectors. The ADR (section "Consequences") documents this as a known residual: a stale upsert landing after a concurrent live key write will clobber the newer mapping, and the completion watermark will then treat the stale entry as authoritative. The impact is worse than the vector case — a wrong key mapping can route entity lookups to the wrong entity (or produce a false-absence hit) until the entity's next write corrects it. The ADR says a "Linear issue on reconnect" is tracked for the symmetric follow-up; confirming that issue exists and is prioritized before watermarking any type with key declarations would reduce the window this is exploitable.

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/temper-server/src/state/projection_backfill/key_index.rs
Line: 144

Comment:
**Key backfill staleness guard not wired up**

`_loaded_seq` is intentionally discarded here, leaving the key-index backfill with the exact same load-then-write race this PR fixes for vectors. The ADR (section "Consequences") documents this as a known residual: a stale upsert landing after a concurrent live key write will clobber the newer mapping, and the completion watermark will then treat the stale entry as authoritative. The impact is worse than the vector case — a wrong key mapping can route entity lookups to the wrong entity (or produce a false-absence hit) until the entity's next write corrects it. The ADR says a "Linear issue on reconnect" is tracked for the symmetric follow-up; confirming that issue exists and is prioritized before watermarking any type with key declarations would reduce the window this is exploitable.

How can I resolve this? If you propose a fix, please make it concise.

[rita-aga]

Independent code review — PR #395 (ARN-216 / ADR-0173)

Scope reviewed: full diff (b36c8982), ADR-0173, DST race test, sim/postgres/turso guard implementations, call-site plumbing, IOA fixture Reembed, StorageStack extract. No code changes by this reviewer.

Plan alignment

Planned item	Status
as_of_sequence on backfill_entity_vectors	Done — trait + Dyn/Boxed + all real store impls
Loader carries sequence_nr on Fields / Skip	Done
Vector backfill + tombstone purge pass load seq	Done
Turso write-behind passes append new_seq	Done
Skip-as-stale = success (watermark-safe)	Done (returns Ok(()), does not increment failed)
RED→GREEN DST (100 seeds, exact interleave)	Done — commit history + green CI DST jobs
ADR	ADR-0173 present; residuals and alternatives recorded honestly
Key-index symmetry	Explicitly out of scope (ADR Consequences) — correct non-expansion

No problematic deviations. storage/stack.rs extract is a justified readability-ratchet move, not a behavior change.

What was done well

• Root cause match: The permanent-stale-watermark failure is exactly “load then unconditional reconcile.” Guarding inside the store transaction is the right place (not actor-spanning locks, not re-load TOCTOU).
• Postgres DELETE-then-recheck under READ COMMITTED is correctly motivated; rolling back the DELETE restores co-committed rows instead of leaving a hole.
• Turso Immediate + explicit rollback on skip avoids holding RESERVED across async Drop.
• Sim mutex path makes the DST deterministic and pins the production interleave shape.
• Skip purge arm gets the sequence too — concurrent re-create is not clobbered by a stale empty reconcile.
• ADR is unusually honest about residuals (key backfill, writer-without-vector-config, cross-model no-prior-rows window). That is the right bar for this class of fix.

Findings

Critical (must fix)

None.

Important (should fix)

None that block this PR’s stated contract.

Documented residuals (already in ADR; not regressions introduced silently):

1. Declared-key backfill still has the same load-then-write clobber shape — intentional follow-up, not silent scope creep.
2. Postgres no-prior-rows + cross-model re-embed can leave a transient stale partition until the next full reconcile — fail-safer than unguarded same-model clobber; self-healing; ADR-recorded.

Suggestions (nice to have)

1. Backend-local unit tests for the skip path (current_seq > as_of_sequence → no row change) on Postgres and Turso. The 100-seed sim DST is the right primary proof; a single skip/no-skip unit per SQL backend would catch a future “forgot the guard in one store” regression cheaply.
2. Backfill INSERT … sequence_nr = 0 (postgres/turso backfill) vs co-commit writing the real journal seq is pre-existing asymmetry. Not required for ARN-216 correctness (guard reads events, not the index column), but a later cleanup could align them so index rows are self-describing.
3. Optional assert in the DST that a current reconcile (as_of_sequence = live seq, rows = E2) still applies — documents the non-skip arm and u64::MAX force semantics beside the stale-skip case.

Architecture / quality notes

• Trait default remains a no-op for non-indexing backends; Redis/Server wrappers inherit safely.
• All production call sites of backfill_entity_vectors updated (vector backfill Fields + Skip, Turso write-behind, tests).
• Key-index match arms updated for the enum shape change without falsely claiming a key-index fix (_loaded_seq discarded).
• Comparison is > not >= — correct: rows derived at sequence N remain valid while the journal is still N.
• Watermark gating still requires failed == 0; guard-skips do not inflate failed. Correct under the “live writer reconciled” assumption documented in the ADR.

CI (at review time)

Compile & Lint, Integrity & DST Patterns, Spec Verification, DST/Platform (core/boot/consistency/random), Instrumentation Hygiene, Verification Contract: pass. Full Tests job still finishing; DST suite covering this race is already green.

---

Verdict: PASS

[rita-aga]

@greptile review

[rita-aga]

@greptile review

[rita-aga]

Addressed: non-wrapping i64→u64 for vector staleness compare. Residual P1 (key-index _loaded_seq unused): key backfill still lacks the as-of seq guard; vector path is the ARN-216 fix surface. Follow-up if key races show up under load.