fix: dual-SQLite WAL consistency — stale gate, safe refresh, EPA retry - #449
fix: dual-SQLite WAL consistency — stale gate, safe refresh, EPA retry#449longhz wants to merge 1 commit into
Conversation
Three fixes for the same root cause: when better-sqlite3 (JS main) and rusqlite (Rust readonly) are compiled independently, the Rust readonly connection reads stale WAL data because the wal-index salt only syncs at checkpoint. This causes silent correctness failures across three subsystems: pairwise similarity, VCP Refresh RAG parsing, and EPA basis cache publish.
|
你的pr内容和你的源码实现货不对板啊，而且这进一步加深了我的怀疑，也就是你的fix对应的实现方法真的是官方源码吗？很奇怪啊，你有在官方技术群吗？ |
|
Request changes. The PR description claims that Rust detects stale EPA publishes by comparing Tag counts and throws tag count stale, but the submitted Rust code does not implement that check. publish_epa_basis_cache only validates database identity. Therefore the new EPA retry loop has no corresponding producer. For pairwise, the JS generation is read before wal_checkpoint(PASSIVE) and then passed as an override to Rust. If a mutation commits between those operations, Rust may scan a newer database snapshot while using the older generation for the artifact gate. The Rust side does not validate that the override matches the snapshot it scans, and it does not return a stale result. This can still incorrectly skip recomputation. The added test checkpoints before reading the generation and does not cover the claimed dual-SQLite concurrent WAL scenario. Please add a Linux reproducer with two independent SQLite builds, assert checkpoint return values, and implement an explicit snapshot/generation consistency check before relying on the override. Also please reconcile the environment/version mismatch: the submitted repository uses rusqlite 0.29, while the PR description claims rusqlite 0.31. |
2 participants
Summary
Follow-up to Issue #435. While investigating the pairwise stale gate, I found the same dual-SQLite WAL consistency root cause silently corrupting two other subsystems. This PR bundles all three fixes.
Root Cause: Dual-SQLite WAL Consistency
VCPToolBox has two independent SQLite connections to the same database file:
On Linux x64 with WAL mode, these two connections are compiled independently -- they are separate SQLite library instances in separate process memory spaces. The Rust readonly connection opens the database with mmap-ed -shm (wal-index) file. However, the wal-index header contains a salt that is only synchronized at checkpoint time. Until wal_checkpoint() runs, the Rust side mmap'd view of the wal-index is a snapshot from the last checkpoint -- it cannot see any WAL frames written by the JS side since then.
This is NOT a crash (like the SIGBUS series). It is a silent correctness failure: Rust reads stale data, makes decisions based on it, and produces wrong results with no error logged.
The fix pattern across all three subsystems:
Changes
1. Pairwise Similarity -- Stale Gate (TagMemoEngine.js + lib.rs)
Problem
The upstream PairwiseSimTask::compute() reads tagmemo_pairwise_fact_generation from kv_store via the Rust readonly connection. This value is a monotonic generation counter advanced by SQLite triggers on tags/file_tags mutations. It serves as a gate: if the generation hasn't changed since the last artifact, skip the full scan (tens of thousands of BLOBs, hundreds of thousands of pair computations).
The bug: after JS writes new tags and triggers advance the generation, the Rust readonly connection still reads the OLD generation (pre-checkpoint). The gate sees "no change" and skips the recomputation entirely -- computedCount=0 when it should be >0. The new tags are silently ignored in pairwise similarity.
Reproduction (Linux x64, Node v24, better-sqlite3 v12):
Fix
lib.rs: Add fact_generation: Option parameter to compute_pairwise_similarities. The PairwiseSimTask struct gains fact_generation_override: Option. In compute(), prefer the override value; fall back to the existing readonly read only when the caller doesn't provide it (backward compatible).
TagMemoEngine.js: Before calling computePairwiseSimilarities, read fact_generation via the JS main connection (better-sqlite3), execute wal_checkpoint(PASSIVE) to sync the wal-index, and pass the value as the 5th argument. A stale retry loop (max 1 retry) handles the window where external mutations race between the read and the compute call.
tests: Add getFactGeneration(db) helper with wal_checkpoint(PASSIVE) to simulate the production path. All 4 computePairwiseSimilarities calls pass the 5th argument.
2. VCP Refresh -- RAG Block Parsing (chatCompletionHandler.js)
Problem
The RAG refresh function _refreshRagBlocksIfNeeded scans conversation history for VCP_RAG_BLOCK markers and re-executes the RAG query with fresh context. Three issues:
Regex too greedy: The metadata capture group matches ANY text, including non-JSON content. If a conversation contains Markdown code blocks with RAG-like syntax examples, the regex matches those too -- triggering spurious RAG refreshes on non-RAG content.
Unsafe JSON parsing: JSON.parse(metadataJson) is called directly on the captured text. If the regex matched a non-JSON string, JSON.parse throws an uncaught exception -- crashing the chat completion handler and returning a 500 to the user.
No code block filtering: The scan runs on the raw message content, including fenced code blocks. Code examples demonstrating RAG usage are indistinguishable from actual RAG blocks.
Fix
stripMarkdownCodeFencesForRagRefresh: Strip all fenced code blocks before scanning. This prevents code examples from being mistaken for RAG blocks.
safeParseRagBlockMetadata: Validate that the captured text is a JSON object (starts with {, ends with }) before calling JSON.parse. Non-JSON matches are silently skipped instead of throwing.
Tighter regex: Change metadata capture to only match JSON objects {\u2026}. This prevents matching unrendered placeholders, regex templates, and other non-JSON content.
3. EPA Basis Cache -- Stale Publish (EPAModule.js)
Problem
The EPA (Eigen-Pair Analysis) module has a compute-then-publish pipeline:
Between step 1 and step 2, the JS main connection might INSERT/DELETE tags (from a concurrent knowledge base update). The Rust publish_epa_basis_cache detects this by comparing the current tag count against the count from step 1. If they differ, it throws a "tag count stale" error.
Before this fix, the error was caught and the function returned false -- the EPA basis was silently not updated, and the caller had no way to retry.
Fix
_runEpaComputePublishOnce: Extract the compute-to-publish sequence into a single attempt function. The return value distinguishes three outcomes: success, failed, stale.
Retry loop: _recomputeWithRust wraps the attempt in a loop (max 1 retry). On stale, it re-runs the entire compute-to-publish sequence. This matches the pattern used in TagMemoEngine for pairwise, artifact rebuild, and intrinsic residual paths.
Backward Compatibility
All changes are backward-compatible:
lib.rs: fact_generation parameter is Option with default None. When not provided, behavior is identical to the current code (Rust readonly read). No existing callers break.
chatCompletionHandler.js: Regex is narrowed, which can only match FEWER strings -- existing valid RAG blocks are unaffected. The safeParseRagBlockMetadata function replaces JSON.parse with a validated wrapper; invalid input is silently skipped instead of throwing.
EPAModule.js: The retry loop only activates when a stale error is detected. On the first successful attempt, behavior is identical. The internal _runEpaComputePublishOnce is a refactoring of the existing inline code, not a new code path.
Environment
The author noted in Issue #435 that the bug "should be fixed" in their environment. This is expected: the issue is specific to dual-SQLite compilation on Linux, where the two libraries are truly independent. On macOS, the system SQLite or unified compilation may mask the problem.
Files Changed
Changes
1. Pairwise Similarity -- Stale Gate (TagMemoEngine.js + lib.rs)
Problem
The upstream PairwiseSimTask::compute() reads tagmemo_pairwise_fact_generation from kv_store via the Rust readonly connection. This value is a monotonic generation counter advanced by SQLite triggers on tags/file_tags mutations. It serves as a gate: if the generation hasn't changed since the last artifact, skip the full scan (tens of thousands of BLOBs, hundreds of thousands of pair computations).
The bug: after JS writes new tags and triggers advance the generation, the Rust readonly connection still reads the OLD generation (pre-checkpoint). The gate sees "no change" and skips the recomputation entirely -- computedCount=0 when it should be >0. The new tags are silently ignored in pairwise similarity.
Reproduction (Linux x64, Node v24, better-sqlite3 v12):
Fix
lib.rs: Add fact_generation: Option parameter to compute_pairwise_similarities. The PairwiseSimTask struct gains fact_generation_override: Option. In compute(), prefer the override value; fall back to the existing readonly read only when the caller doesn't provide it (backward compatible).
TagMemoEngine.js: Before calling computePairwiseSimilarities, read fact_generation via the JS main connection (better-sqlite3), execute wal_checkpoint(PASSIVE) to sync the wal-index, and pass the value as the 5th argument. A stale retry loop (max 1 retry) handles the window where external mutations race between the read and the compute call.
tests: Add getFactGeneration(db) helper with wal_checkpoint(PASSIVE) to simulate the production path. All 4 computePairwiseSimilarities calls pass the 5th argument.
2. VCP Refresh -- RAG Block Parsing (chatCompletionHandler.js)
Problem
The RAG refresh function scans conversation history for VCP_RAG_BLOCK markers and re-executes the RAG query with fresh context. Three issues:
Regex too greedy: The metadata capture group matches ANY text, including non-JSON content. If a conversation contains Markdown code blocks with RAG-like syntax examples, the regex matches those too -- triggering spurious RAG refreshes on non-RAG content.
Unsafe JSON parsing: JSON.parse(metadataJson) is called directly on the captured text. If the regex matched a non-JSON string, JSON.parse throws an uncaught exception -- crashing the chat completion handler and returning a 500 to the user.
No code block filtering: The scan runs on the raw message content, including fenced code blocks. Code examples demonstrating RAG usage are indistinguishable from actual RAG blocks.
Fix
stripMarkdownCodeFencesForRagRefresh: Strip all fenced code blocks before scanning. This prevents code examples from being mistaken for RAG blocks.
safeParseRagBlockMetadata: Validate that the captured text is a JSON object (starts with {, ends with }) before calling JSON.parse. Non-JSON matches are silently skipped instead of throwing.
Tighter regex: Change metadata capture to only match JSON objects. This prevents matching unrendered placeholders, regex templates, and other non-JSON content.
3. EPA Basis Cache -- Stale Publish (EPAModule.js)
Problem
The EPA (Eigen-Pair Analysis) module has a compute-then-publish pipeline:
Between step 1 and step 2, the JS main connection might INSERT/DELETE tags (from a concurrent knowledge base update). The Rust publish_epa_basis_cache detects this by comparing the current tag count against the count from step 1. If they differ, it throws a "tag count stale" error.
Before this fix, the error was caught and the function returned false -- the EPA basis was silently not updated, and the caller had no way to retry.
Fix
_runEpaComputePublishOnce: Extract the compute-to-publish sequence into a single attempt function. The return value distinguishes three outcomes: success, failed, stale.
Retry loop: _recomputeWithRust wraps the attempt in a loop (max 1 retry). On stale, it re-runs the entire compute-to-publish sequence. This matches the pattern used in TagMemoEngine for pairwise, artifact rebuild, and intrinsic residual paths.
Backward Compatibility
All changes are backward-compatible:
lib.rs: fact_generation parameter is Option with default None. When not provided, behavior is identical to the current code (Rust readonly read). No existing callers break.
chatCompletionHandler.js: Regex is narrowed, which can only match FEWER strings -- existing valid RAG blocks are unaffected. The safeParseRagBlockMetadata function replaces JSON.parse with a validated wrapper; invalid input is silently skipped instead of throwing.
EPAModule.js: The retry loop only activates when a stale error is detected. On the first successful attempt, behavior is identical. The internal _runEpaComputePublishOnce is a refactoring of the existing inline code, not a new code path.
Environment
The author noted in Issue #435 that the bug "should be fixed" in their environment. This is expected: the issue is specific to dual-SQLite compilation on Linux, where the two libraries are truly independent. On macOS, the system SQLite or unified compilation may mask the problem.
Files Changed