Skip to content

fix(aver-core): make log-first replay complete (scope, lifecycle, entity typing)#4

Open
5queezer wants to merge 1 commit into
masterfrom
fix/core-log-replay
Open

fix(aver-core): make log-first replay complete (scope, lifecycle, entity typing)#4
5queezer wants to merge 1 commit into
masterfrom
fix/core-log-replay

Conversation

@5queezer

@5queezer 5queezer commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes all six findings from the core log-replay review. Replay now reconstructs the full write-time state of a memory directory — scope, lifecycle status, contradictions, candidate pipeline state, entity classifications, and provenance — instead of the subset it managed before. Strict replay behavior is unchanged for old logs; new record kinds and fields are backward-compatible.

Findings → fixes

1. CRITICAL — scope dropped on replay

LogEntry / EventLogEntry / ObservationLogEntry now carry scope, populated at insert_claim, promote_candidate_claim, record_event_inner, and record_observation_inner. Replay persists it into claims / episodic_events / observations. Old scope-less lines replay as 'global' (matching the migration-0085 column default) via replay_scope(). Round-trip test: replay_preserves_scope_for_claims_events_observations.

(Note: the log structs are Serialize-only — replay parses serde_json::Value manually for per-field path:line diagnostics — so the "serde default" for old lines lives in the replay parsers, not in #[serde(default)]. Same applies to provenance below.)

2. CRITICAL — lifecycle writes unlogged (replay resurrected dead claims)

New log record kinds, each appended before its projection mutation, with apply_log_line handlers:

kind writer file
retire_claim retire_claim log.jsonl
add_contradiction contradict, add_contradiction (id pre-allocated, pinned in the log) log.jsonl
propose_candidate_claim propose_candidate_claim_inner events.jsonl
promote_candidate_claim promote_candidate_claim (paired with the existing add_claim line) events.jsonl
reject_candidate_claim reject_candidate_claim events.jsonl
supersede_claims consolidate_report (ids captured before UPDATE) log.jsonl
decay_confidence decay_contradicted_confidence, decay_inferred_confidence_at (concrete post-decay values) log.jsonl
merge_source_refs merge_duplicate_source_refs (merged refs + promote flag) log.jsonl

Design choices:

  • Outcomes, not formulas. Consolidation/decay log the concrete ids and post-decay confidences computed at write time. Replay asserts the same transitions rather than recomputing them from possibly-drifted state — the log stays the source of truth (ADR-0005) even if decay/supersede policy changes in a future binary. decay_contradicted_confidence was rewritten from one batch UPDATE ... ROUND(confidence - 0.10, 2) to per-row Rust computation (verified parity with SQL ROUND/MAX for the policy confidence values) so the logged values are exactly what gets written.
  • Candidate records go to events.jsonl, not log.jsonl. candidate_claims.event_id FK-references episodic_events, and ADR-0019 §4's input order replays events.jsonl after log.jsonl — candidates in log.jsonl would FK-fail at replay. Candidates are episodic-pipeline artifacts (staged from events), so the episodic log is their natural phase. The promote marker joins them (it FK-references the candidate; the paired claim replays earlier in log.jsonl).
  • Vector chunks: declared a rebuildable derived projection (not logged). Embeddings are large derived arrays (768 × f32 ≈ 6 KB JSON per chunk); logging them would bloat log.jsonl ~10× for zero audit value, since they are a deterministic function of (claim text, embedding model) via the embedding client. Replay rebuilds claims and leaves vector_chunks/vector_index empty; the existing rebuild paths (Store::add_embedded_vector_chunk_for_claim, Store::backfill_vector_embeddings) re-derive them. This boundary is documented in doc/how-it-works.md ("Derived projections are not logged") and covered by vector_chunks_are_rebuildable_after_replay, which asserts replay leaves chunks empty, recall keeps working, and re-embedding restores them.

Regression tests: replay_preserves_retired_claim_status_and_reason, replay_preserves_contradictions, replay_preserves_candidate_lifecycle, replay_preserves_consolidation_outcome.

3. CRITICAL — replay entity typing diverged

ensure_entity_for_replay (hard-coded Thing, unwrap_or(1) fallback) is deleted. Store::ensure_entity's logic now lives in shared free functions (ensure_entity_on / entity_type_id_on / infer_entity_type_name_on) used by both the live write path and replay, so a replayed DB keeps prefix: classifications and the requires_review flag. Test: replay_entity_typing_matches_live (asserts live-vs-replayed (name, type, requires_review) rows are identical, including Service:api → Service/0 and plain-thing → Thing/1).

4. MAJOR — promote appended the log before ontology validation

promote_candidate_claim now runs ontology_check(candidate.predicate, candidate.provenance, ...) before appending anything, mirroring insert_claim's validate-then-log ordering; an unknown extractor predicate fails cleanly with UnknownPredicate and leaves no poisoned add_claim line (test asserts the log has no such line and strict replay of the log succeeds). LogEntry also carries provenance explicitly; replay uses it and falls back to the agent_kind derivation only for pre-fix lines — a promoted INFERRED candidate sourced by a HUMAN event no longer replays as USER_ASSERTED. Tests: promote_rejects_unknown_predicate_before_logging, replay_preserves_promoted_claim_provenance.

5. MAJOR — claim-id pre-allocation race

insert_claim, promote_candidate_claim, add_hyperedge, record_event_inner, propose_candidate_claim_inner, contradict/add_contradiction, and retire_claim now wrap id allocation + log append + projection insert/update in BEGIN IMMEDIATE … COMMIT (rollback on error). Log-first ordering is preserved: the append always precedes the SQLite writes inside the transaction. Store::open sets busy_timeout(5s) so a concurrent second writer waits for the first to commit instead of failing with SQLITE_BUSY. Two processes can no longer compute the same MAX(id)+1 and poison the log with duplicate ids (which previously aborted future replays with ReplayDuplicateId). The hyperedge path's existing projection transaction was extended upward to cover allocation + append; ordering unchanged. Test: concurrent_writers_allocate_unique_claim_ids (two Stores, barrier-synced, 2×25 inserts → 50 unique dense ids in DB and log, then a clean strict replay; run 6× locally, stable).

6. MAJOR — no replay skip/quarantine mode

New ReplayMode::{Strict, Lenient} and replay_with_mode(memory_dir, force, mode). replay() keeps its signature and stays strict (the pinned ADR-0019 §4 default; tests/operational_policy.rs strict-failure test untouched and passing). Lenient mode collects each failing line as ReplayQuarantine { path, line, error } into ReplayReport::quarantined and continues. CLI: aver replay --lenient prints the quarantine count and per-line diagnostics on stderr. Tests: lenient_replay_quarantines_bad_lines (core: malformed JSON, unknown kind, CHECK-violating confidence, dangling retire_claim — 4 quarantined, 2 valid claims applied), replay_strict_fails_and_lenient_quarantines (CLI end-to-end).

One necessary idempotency adjustment

apply_add_claim's duplicate check now compares only write-time immutable fields (subject, predicate, object, agent_id, agent_kind, scope) instead of also provenance and confidence. Lifecycle records legitimately mutate provenance/confidence after the add_claim line, and per-agent log duplicates of that line replay after those mutations — comparing mutable fields would produce false ReplayDuplicateId failures on healthy logs. Genuine id-reuse collisions (the finding-5 race) still diverge in the compared fields and are caught.

ADR updates needed (separate PR per repo policy — doc/adr/*.md intentionally not touched)

  • ADR-0019 (operational policy): §4 — document the new record kinds, the candidate-records-in-events.jsonl routing (FK phase ordering), lenient replay + ReplayQuarantine, the immutable-fields idempotency rule, and the vector-chunk derived-projection boundary; §5/§2 — note BEGIN IMMEDIATE write transactions + busy_timeout(5s).
  • ADR-0021 (scope): scope is now present in add_claim/record_event/record_observation log records; old scope-less lines replay as 'global'.
  • ADR-0022 (connection scope resolution): no functional change, but the replay guarantees section should note scope now round-trips through the log.
  • ADR-0023 (typed recall filters & claim retirement): retirement is now a logged retire_claim record (reason privacy-filtered before logging, since it enters the append-only log); replay reproduces the retired:<reason> source_refs marker.

Test evidence

  • cargo fmt --check — clean
  • cargo clippy -p aver-core --all-targets -- -D warnings — clean (also aver-cli)
  • cargo test -p aver-core478 passed, 0 failed (30 test binaries), including 12 new tests in tests/log_replay.rs
  • cargo test -p aver-cli -p aver-server — all green (34 binaries), including new CLI test tests/replay.rs
  • cargo check --workspace — clean

Public API changes: ReplayMode, ReplayQuarantine, replay_with_mode added; ReplayReport gains lifecycle + quarantined fields; replay() signature unchanged. No MCP tool changes (mcp.rs instructions untouched).

Known residual divergence (pre-existing, not in scope)

Replay sets last_verified_at = ts for all claims; live promote_candidate_claim leaves it NULL (insert_claim sets it). Flagging here so it can be decided in the ADR follow-up whether promote should set it or the log should carry it.

Summary by CodeRabbit

  • New Features

    • Added strict and lenient modes to aver replay.
    • Lenient replay quarantines invalid log lines, continues processing, and reports diagnostics.
    • Replay now restores lifecycle states, scopes, provenance, and candidate outcomes more completely.
    • Added lifecycle and quarantine metrics to replay output.
  • Bug Fixes

    • Improved concurrent write safety and replay compatibility with older log entries.
  • Documentation

    • Updated feature, CLI, storage, and replay behavior documentation.

…ity typing)

Replay now reconstructs the full write-time state instead of a subset:

- scope: add_claim/record_event/record_observation log records carry the
  ADR-0021 scope; replay persists it (old scope-less lines default to
  'global', matching migration 0085).
- lifecycle: retire_claim, add_contradiction, propose/promote/reject
  candidate, and consolidation supersede/decay/merge outcomes are appended
  to the log before the projection mutation, with apply_* handlers so
  replay no longer resurrects retired claims as ACTIVE or drops candidate
  and contradiction state. Consolidation logs concrete outcomes (ids,
  post-decay confidences, merged refs), not formulas.
- provenance: add_claim lines log provenance explicitly; replay uses it
  (fallback: agent_kind derivation for old lines), so promoted INFERRED
  candidates no longer replay as USER_ASSERTED.
- entity typing: replay shares the Store ensure_entity implementation
  (prefix: inference, Thing fallback, requires_review) instead of
  hard-coding Thing.
- promote: ontology_check runs before any log append, so an unknown
  extractor predicate fails without poisoning the log.
- id allocation: claims, hyperedges, events, candidates, and contradictions
  allocate inside BEGIN IMMEDIATE covering alloc + log append + insert;
  busy_timeout(5s) serializes cross-process writers. Closes the duplicate
  claim-id race that made replay abort with ReplayDuplicateId.
- lenient replay: replay_with_mode(ReplayMode::Lenient) quarantines bad
  lines with path/line/error diagnostics and continues; strict stays the
  default. CLI: aver replay --lenient.
- vector chunks are documented as a rebuildable derived projection
  (re-embeddable from claim text) and are intentionally not logged.

The add_claim idempotency check now compares only write-time immutable
fields (subject/predicate/object/agent/scope): lifecycle records
legitimately mutate provenance/confidence after the add line, and per-agent
log duplicates replay after those mutations.
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Replay now logs lifecycle outcomes transactionally, rebuilds richer state from JSONL, supports strict or lenient recovery, and exposes quarantine diagnostics through the CLI. Tests cover lifecycle fidelity, concurrency, compatibility, entity typing, and derived projections.

Changes

Replay integrity

Layer / File(s) Summary
Transactional logging and lifecycle records
crates/aver-core/src/lib.rs
Write operations serialize ID allocation, JSONL append, and SQLite projection updates; lifecycle outcomes, scope, provenance, and retirement records are logged.
Replay modes and state reconstruction
crates/aver-core/src/lib.rs, crates/aver-core/tests/log_replay.rs
Replay reconstructs lifecycle state, entity typing, scopes, provenance, and candidate transitions; strict failures, lenient quarantine, concurrency, legacy records, and vector projection behavior are tested.
CLI exposure and replay contract documentation
crates/aver-cli/..., README.md, doc/how-it-works.md
replay --lenient selects quarantine mode and reports diagnostics; documentation describes transactional logging, replay defaults, and derived projections.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant ReplayCLI
  participant ReplayCore
  participant JSONL
  participant SQLite
  Operator->>ReplayCLI: run replay or replay --lenient
  ReplayCLI->>ReplayCore: call replay_with_mode
  ReplayCore->>JSONL: read records
  ReplayCore->>SQLite: apply valid claims and lifecycle state
  ReplayCore-->>ReplayCLI: report applied and quarantined records
  ReplayCLI-->>Operator: print metrics and diagnostics
Loading

Poem

Logs hold the line,
Claims rise from ordered records,
Bad lines stand aside.
SQLite follows the trail,
Replay remembers all.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: fuller log-first replay coverage for scope, lifecycle, and entity typing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/core-log-replay

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/aver-core/src/lib.rs (1)

5663-5708: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Include scope in duplicate-ID validation.

Both handlers parse and insert scope but omit it from the existing-row comparison. Conflicting duplicate records are silently accepted, leaving replay order to decide the scope.

Also applies to: 5732-5771

🤖 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/aver-core/src/lib.rs` around lines 5663 - 5708, The duplicate-ID
validation in both replay handlers must compare the existing row’s scope as well
as the other event fields. Update the existing-row SELECT, tuple extraction, and
matches condition around the replay insertion logic (including the corresponding
handler near the second range) to retrieve and validate scope, returning
ReplayDuplicateId on any mismatch while preserving idempotent success for
identical records.
🤖 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 `@crates/aver-core/src/lib.rs`:
- Around line 5039-5055: The replay loop around apply_log_line must execute each
line within a savepoint and roll it back whenever handling fails in lenient mode
before adding ReplayQuarantine; preserve strict-mode error propagation and
remove or adapt any nested replay transactions that conflict with the savepoint.
In crates/aver-core/tests/log_replay.rs lines 480-516, extend the invalid-claim
assertions to verify that neither entity nor ontology side effects remain after
quarantine.
- Around line 2347-2364: Update the rejection flow around
RejectCandidateLogEntry to re-read the candidate status after BEGIN IMMEDIATE
and allow only a PENDING → REJECTED transition. Treat retries for an already
REJECTED candidate with the same reason as successful no-ops, and reject or
otherwise avoid emitting a record for conflicting or already-applied rejections
so the event log remains replayable.
- Around line 5989-6162: Update apply_promote_candidate_claim and
apply_reject_candidate_claim to allow only valid source-status transitions,
reject conflicting terminal states, and verify the UPDATE changes exactly one
row before incrementing lifecycle. In apply_supersede_claims,
apply_decay_confidence, and apply_merge_source_refs, validate every referenced
claim exists and require the expected rows_changed for each update; return a
replay error for missing rows or impossible transitions, while preserving valid
idempotent behavior where supported.
- Around line 1014-1019: Refactor the claim-processing flow around
ontology_check so validation is completed without mutating ontology state, then
append the JSONL ontology-extension record before any ontology or projection
mutation. Apply the validated ontology extension, predicate_types/closure
updates, and related projection changes only afterward within the existing BEGIN
IMMEDIATE transaction, preserving privacy filtering and the log-first ordering;
update the corresponding flow near the additional ontology mutation site as
well.

In `@doc/how-it-works.md`:
- Around line 86-92: Update the documentation around
Store::backfill_vector_embeddings to clarify that it is not a standalone restore
path: replay must first regenerate the vector chunk rows, after which backfill
fills embeddings only for existing chunks that are missing them. Preserve the
distinction between rebuilding chunks and populating their embeddings.

---

Outside diff comments:
In `@crates/aver-core/src/lib.rs`:
- Around line 5663-5708: The duplicate-ID validation in both replay handlers
must compare the existing row’s scope as well as the other event fields. Update
the existing-row SELECT, tuple extraction, and matches condition around the
replay insertion logic (including the corresponding handler near the second
range) to retrieve and validate scope, returning ReplayDuplicateId on any
mismatch while preserving idempotent success for identical records.
🪄 Autofix (Beta)

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

Run ID: 6ead5f37-9cca-4629-a35c-e8004ef3a37d

📥 Commits

Reviewing files that changed from the base of the PR and between f84fd7e and 2a2973b.

📒 Files selected for processing (6)
  • README.md
  • crates/aver-cli/src/main.rs
  • crates/aver-cli/tests/replay.rs
  • crates/aver-core/src/lib.rs
  • crates/aver-core/tests/log_replay.rs
  • doc/how-it-works.md

Comment on lines +1014 to +1019
// Pre-allocate the claim id inside a write transaction. The
// BEGIN IMMEDIATE serializes cross-process writers (WAL single
// writer), so two processes can no longer compute the same
// MAX(id)+1 and poison the log with duplicate claim ids. The log
// append stays strictly before the SQLite INSERT (ADR-0005).
self.conn.execute_batch("BEGIN IMMEDIATE")?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Move ontology mutation behind the append boundary.

ontology_check can modify predicate_types, closure rows, and ontology_extension_log before these transactions and before JSONL. If append fails, unlogged projection state remains. Split validation from mutation, then append before applying the ontology extension and projection within the write transaction.

As per coding guidelines, preserve privacy filtering and log-first write ordering when changing code.

Also applies to: 1135-1138

🤖 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/aver-core/src/lib.rs` around lines 1014 - 1019, Refactor the
claim-processing flow around ontology_check so validation is completed without
mutating ontology state, then append the JSONL ontology-extension record before
any ontology or projection mutation. Apply the validated ontology extension,
predicate_types/closure updates, and related projection changes only afterward
within the existing BEGIN IMMEDIATE transaction, preserving privacy filtering
and the log-first ordering; update the corresponding flow near the additional
ontology mutation site as well.

Source: Coding guidelines

Comment on lines +2347 to +2364
let now = time::OffsetDateTime::now_utc().unix_timestamp();
self.conn.execute_batch("BEGIN IMMEDIATE")?;
let result = (|| -> Result<(), Error> {
// Candidate lifecycle records live in the episodic log so replay
// applies them in the same phase as the candidate proposal.
let entry = RejectCandidateLogEntry {
kind: "reject_candidate_claim",
ts: now,
candidate_id,
reason,
};
append_jsonl(&self.event_log_path, &entry)?;
let rows_changed = self.conn.execute(
"UPDATE candidate_claims
SET status = 'REJECTED', rejection_reason = ?1
WHERE id = ?2",
params![reason, candidate_id],
)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not emit multiple rejection records for one candidate.

A REJECTED candidate passes the precheck, so sequential or racing calls can append different reasons and overwrite the projection. Replay rejects the second marker, making a live-generated log unreplayable. Re-read status under BEGIN IMMEDIATE and permit only PENDING → REJECTED; treat identical retries as idempotent.

🤖 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/aver-core/src/lib.rs` around lines 2347 - 2364, Update the rejection
flow around RejectCandidateLogEntry to re-read the candidate status after BEGIN
IMMEDIATE and allow only a PENDING → REJECTED transition. Treat retries for an
already REJECTED candidate with the same reason as successful no-ops, and reject
or otherwise avoid emitting a record for conflicting or already-applied
rejections so the event log remains replayable.

Comment on lines +5039 to +5055
let result = apply_log_line(
&conn,
&line,
&input.to_string_lossy(),
lineno + 1,
&mut report,
)?;
);
if let Err(err) = result {
match mode {
ReplayMode::Strict => return Err(err),
ReplayMode::Lenient => report.quarantined.push(ReplayQuarantine {
path: input.to_string_lossy().into_owned(),
line: lineno + 1,
error: err.to_string(),
}),
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Quarantine must leave no projection debris.

A handler can mutate entities or ontology before failing; lenient mode then quarantines the line but preserves those partial writes in the rebuilt database.

  • crates/aver-core/src/lib.rs#L5039-L5055: apply each line inside a savepoint and roll it back before quarantining; remove or adapt nested replay transactions.
  • crates/aver-core/tests/log_replay.rs#L480-L516: assert the invalid claim leaves no entity or ontology side effects.
📍 Affects 2 files
  • crates/aver-core/src/lib.rs#L5039-L5055 (this comment)
  • crates/aver-core/tests/log_replay.rs#L480-L516
🤖 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/aver-core/src/lib.rs` around lines 5039 - 5055, The replay loop around
apply_log_line must execute each line within a savepoint and roll it back
whenever handling fails in lenient mode before adding ReplayQuarantine; preserve
strict-mode error propagation and remove or adapt any nested replay transactions
that conflict with the savepoint. In crates/aver-core/tests/log_replay.rs lines
480-516, extend the invalid-claim assertions to verify that neither entity nor
ontology side effects remain after quarantine.

Comment on lines +5989 to +6162
fn apply_promote_candidate_claim(
conn: &Connection,
value: &serde_json::Value,
path: &str,
lineno: usize,
report: &mut ReplayReport,
) -> Result<(), Error> {
let candidate_id = replay_i64(value, "candidate_id", path, lineno)?;
let _ts = replay_i64(value, "ts", path, lineno)?;
let claim_id = replay_i64(value, "claim_id", path, lineno)?;

let existing: Option<(String, Option<i64>)> = conn
.query_row(
"SELECT status, promoted_claim_id FROM candidate_claims WHERE id = ?1",
[candidate_id],
|row| Ok((row.get(0)?, row.get(1)?)),
)
.optional()?;
let Some((status, promoted_claim_id)) = existing else {
return Err(Error::ReplayMalformed {
path: path.to_string(),
line: lineno,
detail: format!("promote_candidate_claim references missing candidate {candidate_id}"),
});
};
if status == "PROMOTED" {
if promoted_claim_id == Some(claim_id) {
return Ok(());
}
return Err(Error::ReplayDuplicateId {
detail: format!(
"candidate_id={candidate_id} already PROMOTED to a different claim at {path}:{lineno}"
),
});
}
conn.execute(
"UPDATE candidate_claims
SET status = 'PROMOTED', promoted_claim_id = ?1
WHERE id = ?2",
params![claim_id, candidate_id],
)?;
report.lifecycle += 1;
Ok(())
}

fn apply_reject_candidate_claim(
conn: &Connection,
value: &serde_json::Value,
path: &str,
lineno: usize,
report: &mut ReplayReport,
) -> Result<(), Error> {
let candidate_id = replay_i64(value, "candidate_id", path, lineno)?;
let _ts = replay_i64(value, "ts", path, lineno)?;
let reason = replay_str(value, "reason", path, lineno)?;

let existing: Option<(String, Option<String>)> = conn
.query_row(
"SELECT status, rejection_reason FROM candidate_claims WHERE id = ?1",
[candidate_id],
|row| Ok((row.get(0)?, row.get(1)?)),
)
.optional()?;
let Some((status, rejection_reason)) = existing else {
return Err(Error::ReplayMalformed {
path: path.to_string(),
line: lineno,
detail: format!("reject_candidate_claim references missing candidate {candidate_id}"),
});
};
if status == "REJECTED" {
if rejection_reason.as_deref() == Some(reason) {
return Ok(());
}
return Err(Error::ReplayDuplicateId {
detail: format!(
"candidate_id={candidate_id} already REJECTED with a different reason at {path}:{lineno}"
),
});
}
conn.execute(
"UPDATE candidate_claims
SET status = 'REJECTED', rejection_reason = ?1
WHERE id = ?2",
params![reason, candidate_id],
)?;
report.lifecycle += 1;
Ok(())
}

/// Replay a `supersede_claims` record: assert the logged lifecycle
/// transitions. Naturally idempotent (the UPDATE is a no-op once the claim
/// is SUPERSEDED) and tolerant of claims quarantined in lenient mode.
fn apply_supersede_claims(
conn: &Connection,
value: &serde_json::Value,
path: &str,
lineno: usize,
report: &mut ReplayReport,
) -> Result<(), Error> {
let _ts = replay_i64(value, "ts", path, lineno)?;
let claim_ids = replay_i64_list(value, "claim_ids", path, lineno)?;
for claim_id in &claim_ids {
conn.execute(
"UPDATE claims SET status = 'SUPERSEDED' WHERE id = ?1",
[claim_id],
)?;
}
report.lifecycle += 1;
Ok(())
}

/// Replay a `decay_confidence` record: apply the logged post-decay values
/// verbatim — the log records outcomes, not formulas.
fn apply_decay_confidence(
conn: &Connection,
value: &serde_json::Value,
path: &str,
lineno: usize,
report: &mut ReplayReport,
) -> Result<(), Error> {
let _ts = replay_i64(value, "ts", path, lineno)?;
let changes: Vec<ConfidenceChange> = serde_json::from_value(
replay_field(value, "changes", path, lineno)?.clone(),
)
.map_err(|err| Error::ReplayMalformed {
path: path.to_string(),
line: lineno,
detail: format!("'changes' is not a confidence-change array: {err}"),
})?;
for change in &changes {
conn.execute(
"UPDATE claims SET confidence = ?1 WHERE id = ?2",
params![change.confidence, change.claim_id],
)?;
}
report.lifecycle += 1;
Ok(())
}

fn apply_merge_source_refs(
conn: &Connection,
value: &serde_json::Value,
path: &str,
lineno: usize,
report: &mut ReplayReport,
) -> Result<(), Error> {
let _ts = replay_i64(value, "ts", path, lineno)?;
let claim_id = replay_i64(value, "claim_id", path, lineno)?;
let source_refs = replay_string_list(value, "source_refs", path, lineno)?;
let promote = replay_field(value, "promote", path, lineno)?
.as_bool()
.ok_or_else(|| Error::ReplayMalformed {
path: path.to_string(),
line: lineno,
detail: "'promote' is not a boolean".to_string(),
})?;
let merged = serde_json::to_string(&source_refs)?;
if promote {
conn.execute(
"UPDATE claims
SET source_refs = ?1, provenance = 'EXTRACTED', confidence = MAX(confidence, 0.75)
WHERE id = ?2",
params![merged, claim_id],
)?;
} else {
conn.execute(
"UPDATE claims SET source_refs = ?1 WHERE id = ?2",
params![merged, claim_id],
)?;
}
report.lifecycle += 1;
Ok(())
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Strict replay must reject impossible lifecycle transitions.

Promotion can overwrite REJECTED, rejection can overwrite PROMOTED, and supersede/decay/merge count missing-claim updates as successful. Validate source status and referenced rows, requiring the expected rows_changed before incrementing lifecycle.

🤖 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/aver-core/src/lib.rs` around lines 5989 - 6162, Update
apply_promote_candidate_claim and apply_reject_candidate_claim to allow only
valid source-status transitions, reject conflicting terminal states, and verify
the UPDATE changes exactly one row before incrementing lifecycle. In
apply_supersede_claims, apply_decay_confidence, and apply_merge_source_refs,
validate every referenced claim exists and require the expected rows_changed for
each update; return a replay error for missing rows or impossible transitions,
while preserving valid idempotent behavior where supported.

Comment thread doc/how-it-works.md
Comment on lines +86 to +92
**Derived projections are not logged.** Vector chunks (embedding vectors and
the `vec0` ANN index) are re-derivable from claim text with the same
embedding model, so they are deliberately absent from the log: replay
rebuilds claims and leaves `vector_chunks` empty rather than bloating the
append-only log with large derived arrays. Rebuild them after a restore with
`Store::add_embedded_vector_chunk_for_claim` /
`Store::backfill_vector_embeddings`.

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

Do not present backfill as a standalone restore path.

Replay leaves vector_chunks empty, while backfill_vector_embeddings only fills embeddings on existing chunk rows. State that chunks must first be regenerated; backfill only handles existing rows missing embeddings.

🤖 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 `@doc/how-it-works.md` around lines 86 - 92, Update the documentation around
Store::backfill_vector_embeddings to clarify that it is not a standalone restore
path: replay must first regenerate the vector chunk rows, after which backfill
fills embeddings only for existing chunks that are missing them. Preserve the
distinction between rebuilding chunks and populating their embeddings.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant