fix: add LanceDB row-count validation after extraction to prevent poison state

[jlin53882]

Problem Statement

現狀：萃取完成後不驗證寫入結果

萃取 pipeline 的最終階段：

// src/smart-extractor.ts — extractAndPersist()
if (createEntries.length > 0) {
  await this.store.bulkStore(createEntries);  // ← 只管寫入，不管寫入是否真的成功
}

return stats;  // ← 返回的 stats.created 是「預期寫入數」，不是「實際寫入數」

// src/store.ts — bulkStore() 底層
async bulkStore(entries: MemoryEntry[]): Promise<void> {
  await this.ensureInitialized();
  await this.table.add(entries.map(toRow));
  // ← 沒有比對回傳值，沒有驗證實際寫入數量
}

store.ts 有 count() 方法可以查出真實的 LanceDB row count：

// src/store.ts:512
async count(): Promise<number> {
  await this.ensureInitialized();
  return await this.table.countRows();  // ← 可查出真實寫入數
}

但萃取完成後從來沒有調用過。

何時會出問題

情境 A：OpenClaw 被強制關機

萃取 pipeline 啟動：
1. LLM extract → 5 個候選記憶
2. 個別處理 → 3 個 createEntries
3. bulkStore(3 個 entries) → 寫入 2 個後...
4. OpenClaw 被 kill（使用者關機 / 系統重啟 / 網路斷線）
5. 實際只寫入 2 個，但 stats 返回 { created: 3, ... }

下次啟動：
→ 沒有任何機制知道「應該有 3 個但實際只有 2 個」
→ 使用者以為 3 個都記住了，但第 3 個已經永遠消失
→ 沒有錯誤訊息、沒有重試、沒有修復

情境 B：LanceDB write failure 但沒有拋出

bulkStore() 內部呼叫 this.table.add(entries)
LanceDB 在特定情況下（磁碟滿、許可權、檔案鎖）可能 partial write
add() 本身不回傳成功寫入的數量
No error thrown → 萃取 pipeline 假設全部成功
→ stats.created = 3，但實際 DB 只有 1 個
→ phantom state：「我記住了」但實際上沒有

情境 C：compactor 同時運作（race condition）

萃取正在寫入 10 個 entries
memory-compactor 同時執行刪除（基於 decay / 閾值）
compactor 刪了 3 個舊記憶
萃取最後 bulkStore 只成功寫入 7 個新記憶
stats.created = 10，但實際凈增加 = 7 - 3 = 4
→ 差異 6 個，這 6 個去了哪？不知道

規模感

情境	發生機率	影響
每次正常萃取完成	極低	不受影響
OpenClaw 被 kill（長萃取 session）	中等（系統問題/網路）	1-3 個記憶消失
磁碟滿	低	全部記憶消失，無錯誤提示
Compactor race condition	低但累積	記憶數量慢性流失

這不是一個會每次都發生的問題，而是一個累積性資料缺口：每次發生一點點，使用者過了很久才發現「奇怪，某個記憶怎麼不見了」。

現有對比：claude-context 的做法

Zilliz Cloud 的 MCP 實現遇到過同樣的問題（Issue #295）。他們的 snapshot 和 Milvus collection count 不同步時，client 會把 { indexedFiles: 0, totalChunks: 0, status: 'completed' } 當成「已索引完成」，實際上 collection 是空的。觸發 force reindex 就把真實資料刪掉重寫 0，形成無限迴圈。

他們的修復邏輯：

// claude-context handlers.ts — validateLegacyZeroEntries()
const realRowCount = await vdb.getCollectionRowCount(collectionName);

if (realRowCount === -1) {
    // 無法確定（網路錯誤等），跳過不做任何事
    return;
}
if (realRowCount > 0) {
    // 真實有資料，但 snapshot 可能寫了 0
    // → 用真實 count 覆蓋 snapshot（heal）
    snapshotManager.setCodebaseIndexed(codebasePath, { indexedFiles: realRowCount, ... });
}
if (realRowCount === 0 && snapshot.status === 'completed') {
    // 危險：snapshot 說 completed，但真實資料庫是空的
    // → 這是 phantom entry，刪除讓他下次強制重來
    snapshotManager.removeCodebaseCompletely(codebasePath);
}

---

Proposed Solution

核心邏輯：萃取後驗證

在 extractAndPersist() 完成後，加入 validation check：

// src/smart-extractor.ts — extractAndPersist() 修改

async extractAndPersist(...): Promise<ExtractionStats> {
  const countBefore = await this.store.count();  // ← 萃取前計數
  
  // ... 現有萃取邏輯 ...
  
  let validation: ExtractionValidation | null = null;
  
  if (createEntries.length > 0) {
    await this.store.bulkStore(createEntries);
    
    // 新增 validation
    const countAfter = await this.store.count();
    const actualCreated = countAfter - countBefore;
    
    if (actualCreated !== stats.created) {
      validation = {
        expected: stats.created,
        actual: actualCreated,
        discrepancy: stats.created - actualCreated,
      };
      
      this.log(
        `memory-pro: extraction validation mismatch: ` +
        `expected=${validation.expected} actual=${validation.actual} ` +
        `lost=${validation.discrepancy}`
      );
      
      if (this.config.onExtractionValidationFailed) {
        await this.config.onExtractionValidationFailed(validation, {
          entries: createEntries,
          sessionKey,
          targetScope,
        });
      }
      
      stats.validationMismatch = validation.discrepancy;
    }
  }
  
  return stats;
}

Config 延伸

// SmartExtractorConfig 新增可選欄位
export interface SmartExtractorConfig {
  // ... 現有欄位 ...
  
  /** Called when extraction write count doesn't match expected count. */
  onExtractionValidationFailed?: (
    validation: ExtractionValidation,
    context: { entries: StoreEntry[]; sessionKey: string; targetScope: string }
  ) => Promise<void> | void;
}

錯誤分類與處理策略

情況	可能性	處理
實際寫入 > 預期	極低（並發萃取或 compactor）	log warning，視為 warning 而非 error
實際寫入 < 預期	中等（kill / partial write）	log error + callback，外部可 trigger retry
實際寫入 = 預期	正常	無操作

為什麼不直接 retry

不建議在 validation 失敗後直接 retry，原因是：
1. 失敗原因可能是「重複 ID」而非「寫不入」
2. 重新執行一次 LLM 萃取代價昂貴（API call + time）
3. 外部 caller（如 auto-capture）可以自己決定要不要 retry
4. validation 的目的只是「確認資料確實寫入了」，不是「自動修復」

---

Impact

維度	改善
資料完整性	及時發現寫入缺口，不再有「靜默資料消失」
可除錯性	validation mismatch 有 callback + log，問題可追蹤
系統信心	使用者知道「萃取 N 個記憶」是真的寫入了 N 個
Compactor race	差異出現時，log 會顯示，累積問題可被發現

---

Questions for Maintainers

1. onExtractionValidationFailed callback 的設計方向是否合理？還是有更好的 API 設計？
2. 這個 validation 是否應該預設開啟？還是需要 config 開關？
3. 目前 bulkStore 的回傳值是 void，是否值得改為回傳 number（實際寫入數）？

---

References

• Zilliz Cloud Issue feat(integrations): add Claude and Codex host bridges #295 fix（snapshot validation concept）
• src/store.ts:512 — count() 方法現已完成，可直接使用

[AliceLJY]

Valid integrity gap, but I would avoid countBefore/countAfter as the main validator.

A global row-count delta becomes ambiguous as soon as compaction or another writer is active, so it can produce false mismatch noise even when the current extraction wrote correctly. Cleaner direction is store-level acknowledgement: either have bulkStore() return the inserted count or IDs, or re-read the exact records that were just written.

So I agree this should be tracked as a bug, but I would scope the fix around write acknowledgements rather than total table row deltas.

[jlin53882]

三種設計方向分析

經過對 memory-lancedb-pro 原始碼的完整審查，發現維護者的 Proposed Solution 與實際程式碼存在一個關鍵落差：bulkStore() 在 codebase 中並不存在。萃取是透過 this.store.store()（單筆）逐次寫入，共 4 個 branch（行 1159, 1290, 1354, 1413），而非批量寫入。以下是三個方向的完整評估。

---

「 方向 A： count() 差值驗證（維護者提案，需調整）

做法：菇取前、後各取一次 count()，以差值驗證 stats.created 是否正確。

問題：

1. count() 包含 inactive 歷史記錄。compactor 同時執行時，菇取寫入 10 個但 compactor 削了 3 個舊記憶，count 差值 = 7，但 stats.created = 10，驗證會誤判為失敗。
2. 依賴「菇取期間無其他寫入」的假設。auto-capture 在同 scope 下可能並發執行，差值會被污染。
3. 驗證的是「有決經發現問題」，不是「問題有沒發生」。記憶已經消失，只是多了一行 log。

---

「 方向 B：廁除 in-memory 計數，stats 以實際回傳為準

做法：移除 12 處 stats.created++，讓 stats.created 直接等於 stored.length。

問題：

1. Breaking change：stats.created 語意從「菇取意圖數」變成「成功寫入數」。現有監控 dashboard 和 auto-capture caller 依賴舊語意會斷言失敗。
2. compactor race 的問題依然沒解決。即使 stats 改用 stored.length，comptactor 刪除的 entry 不在統計範圍內。
3. 方向 C 完全覆蓋方向 B 的效益，且風險更低。

---

「 方向 C：以 store() 回傳值驱動計數 + count() 差值驗證（推薦）

核心發現：store.ts 的 store() 方法本身就回傳 Promise<MemoryEntry>（含 UUID），但 storeCandidate() 只 await 後就昭掉回傳值。這個回傳值正是我們需要的「實際寫入確認」。

Phase 1 — 讓 storeCandidate() 回傳 MemoryEntry | null（約 15 行）

• 接企 this.store.store() 的回傳值並往下傳
• processCandidate() 改為 const result = await this.storeCandidate(...); if (result) stats.created++;
• 這樣 stats 的基礎從一開始就是「實際成功寫入」，不是樂见估計

Phase 2 — count() 差值用於偵測外部並發問題（約 10 行）

• 不是用來「確認 stats 對不錯」（Phase 1 已解決）
• 而是用來偵測 compactor race（菇取期間有外部刪除）
• Callback 的 validation 多了 externalDeletions 欄位，區分「菇取寫入失敗」和「外部刪除導致淨差」

Phase 3 — Config 延伸

• verifyWrite?: boolean（預設 true）
• onExtractionValidationFailed?: (validation, context) => Promise<void> | void
• Callback 的 context.entries 改為 MemoryEntry[]（實際產出），而非 StoreEntry[]（輸入）

為什麼方向 C 最儦：

問題	A	B	C
stats 樂见計數	保留	✔ 廁除	✔ 廁除
count race（compactor）	誤報	無法處理	✔ 區分了海
單筆 kill（寫入後未遒增）	驗證層補敗	無法處理	✔ 回傳值直接檩疑
向後相容	✔ 維持	⚠ 破壞	✔ 維持
bulkStore 不存在的問題	⚠ 需先实作	⚠ 需先实作	✔ 自然整合

---

實作規模：Phase 1 為核心修改，約 30 行，影韏 3 個函式（storeCandidate、processCandidate、Callback 接收端）。Phase 2/3 為疊加，不需要改根心菇取邏輯。

---

請維護者確認：

1. 方向 C 的設計方向是否可以接受？
2. verifyWrite 是否預設 true（主動保護）？
3. Phase 2 的 externalDeletions 區分邏輯是否必要？（若不需要，可簡化為單一 discrepancy 欄位）

[AliceLJY]

Direction C is acceptable, but keep Phase 1 narrower:

• Count stats.created from the actual store acknowledgement path: storeCandidate() / processCandidate() should increment only when the store write returns a concrete entry/ID.
• Do not use global countBefore / countAfter as the main correctness gate in this PR. It is noisy under compaction or concurrent writers and can report false mismatches even when the current extraction wrote correctly.
• If the current branch uses bulkStore(), apply the same principle there: exact returned IDs / exact re-read of written IDs, not total table delta.
• Keep externalDeletions / compactor-race accounting as Phase 2 unless the Phase 1 implementation already has exact evidence for it.
• verifyWrite is fine only if it validates exact written IDs; otherwise omit the option for now.

So the first fix should be: make stats reflect actual acknowledged writes, add focused regression coverage, and avoid table-wide delta validation as the source of truth.

[jlin53882]

Phase 1 Implementation Plan + Clarifying Questions

Hi Alice, following your Direction C guidance, I have analyzed the codebase and mapped out the dependency chain. Before starting implementation, I need to confirm a few points.

---

Dependency Chain

PR #622/#623/#626 (merged ✅)
  └─ runWithFileLock: correct lock semantics with stale cleanup
       ↓
PR #669 (merged ✅)
  └─ bulkStore(): single lock acquisition, returns MemoryEntry[]
       ↓
PR #678 (open, not yet merged ⚠️)
  ├─ handleSupersede → createEntries[] batch (reduces N×lock → 1)
  ├─ handleContradict/contextualize → dual path (batch / direct fallback)
  └─ invalidateEntries loop (after bulkStore, per-entry update)
       ↓
Phase 1 (pending ⏳ — should start AFTER PR #678 merges)
  ├─ Remove stats.created++ from processCandidate() body
  ├─ After bulkStore(): stats.created = result.length
  └─ Handle filter delta (see Q1 below)
       ↓
Phase 2 (out of scope for now)
  ├─ externalDeletions / compactor-race accounting
  └─ supersede invalidate atomic window

Key note: PR #678 is complementary to Phase 1 — it fixes the N×lock problem, Phase 1 fixes the stats accuracy problem. They are orthogonal and stack cleanly. Phase 1 implementation should begin after PR #678 merges to avoid rebasing conflicts.

---

Clarifying Questions

Q1 — bulkStore filter delta: bulkStore() filters entries with missing text/vector internally, so result.length may be less than createEntries.length. Should stats.created reflect the actual acknowledged count (bulkStoreResult.length) or the original create-decision count (createEntries.length)? Also: should mismatches be logged, and if so, at what threshold?

Q2 — handleSupersede stats: supersede writes 1 new entry (store()) + 1 update (update()). stats.created currently increments 1 for the new entry. Should this stay unchanged in Phase 1?

Q3 — validationMismatch field: Issue #693 originally proposed an ExtractionStats.validationMismatch field. Is this in scope for Phase 1, or should it wait for Phase 2?

Q4 — handleMerge fallback: when merge LLM fails and we fall back to storeCandidate(), if store() throws, stats.created is not incremented (current behavior). Should Phase 1 add retry logic here?

---

Looking forward to your guidance before proceeding with implementation.

[jlin53882]

Q1-Q4 Code Research — Our Findings

Before your reply, here is our deep-dive analysis of each question. This should help calibrate expectations for what Phase 1 can and cannot fix.

---

Q1 — bulkStore filter delta

Code: store.ts line 494:

const validEntries = entries.filter(
  (entry) => entry && entry.text && entry.text.length > 0
              && entry.vector && entry.vector.length > 0
);

Root cause of invalid entries: In smart-extractor.ts line 692, when embedding fails:

createEntries?.push(this.buildStoreEntry(candidate, vector || [], ...)); // empty vector!

Empty-vector entries get filtered out by bulkStore, inflating stats.created.

Our recommendation: Use bulkStoreResult.length as the source of truth. The only entries that should inflate the count are ones that bulkStore actually persisted. Logging the delta is useful for debugging but not critical — it adds noise to logs if embedded failures are frequent.

---

Q2 — handleSupersede stats (with PR #678 applied)

Finding: After PR #678, handleSupersede has two paths:

• existing found → store() (direct, no createEntries) → caller increments stats.created
• existing NOT found → pushes to createEntries → caller increments stats.created

Both paths go through the same stats.created++ in the caller — no inconsistency introduced by PR #678. However, there is a separate bug: entries stored via handleSupersede's direct store() path bypass bulkStore entirely. They are counted in stats but not reflected in bulkStoreResult.length.

Our recommendation: Phase 1 should track them separately: stats.created = bulkStoreResult.length + handleSupersedeDirectCount.

---

Q3 — validationMismatch field

The validationMismatch concept is useful for debugging but is not the same as the stats inaccuracy bug. The inaccuracy bug exists even when no filtering happens — it comes from counting decisions, not acknowledgments.

validationMismatch is a diagnostic enhancement that belongs in Phase 2 or as a follow-up issue.

---

Q4 — handleMerge fallback vs handleContradict bug

handleMerge fallback: Does NOT throw — pushes to createEntries and returns cleanly. stats.created is incremented by caller. No bug here.

handleContradict: Has a real bug — when !matchId:

if (!dedupResult.matchId) {
  stats.created++;              // ← incremented BEFORE bulkStore
  createEntries?.push(entry_d);
} else {
  createEntries?.push(entry_d); // ← no stats++ here
}

When !matchId, stats.created++ happens BEFORE bulkStore() runs. If bulkStore() fails (throw), the stat is already inflated.

Our recommendation: Move stats.created++ for the no-match case inside the caller, so it only increments if bulkStore actually succeeds (after refactor).

---

Summary of Phase 1 Scope (revised after research)

Issue	Fixable in Phase 1?	Notes
Q1: filter delta	Yes	Use bulkStoreResult.length + track direct writes
Q2: handleSupersede direct writes bypass batch	Yes	Add handleSupersedeDirectCount
Q3: validationMismatch	No	Phase 2 / follow-up issue
Q4: handleContradict no-match stats inflation	Yes	Move stat increment to caller

Key structural change needed: After bulkStore(result), set stats.created = result.length + directWriteCount where directWriteCount tracks entries written outside the batch (via handleSupersede's direct store() path).