Merge pull request #707 from philluiz2323/fix/vault-sync-second-edit-clobbered

fix(vault-sync): key pending-proposal dedup on edit content, not page id

Author: plind-junior

CHANGELOG.md

@@ -106,6 +106,18 @@ All notable changes to vouch are documented here. Format follows
   artifact the caller could not already retrieve, and it touches no write path.
 
 ### Fixed
+- **vault sync no longer clobbers a second, distinct vault edit made while
+  the first edit's proposal is still pending**: `_has_pending_page_proposal`
+  dedup-checked pending proposals by page id alone, so re-running
+  `vault_to_kb` after a *different* edit to an already-pending page
+  silently skipped filing a new proposal instead of recognizing the edit
+  as distinct. The second edit was never captured in any proposal, and
+  the next backward sync pass then overwrote the vault mirror with the
+  KB's still-unapproved-first-edit content, discarding the second edit
+  with no trace and no error. Now keyed on the content-address (sha256)
+  of the whole edit rather than the page id alone, matching how sources
+  are already fingerprinted elsewhere, so a second distinct edit correctly
+  files its own proposal instead of being coalesced into the first.
 - **`kb.experts` no longer leaks out-of-scope claims into entity rankings**
   (#714): `rank_experts` aggregated evidence density over every claim in
   the KB with no viewer/scope filtering at all, unlike every sibling

src/vouch/vault_sync.py

@@ -314,15 +314,22 @@ def kb_to_vault(store: KBStore, vault_dir: Path) -> VaultSyncResult:
 
 
 def _has_pending_page_proposal(
-    store: KBStore, page_id: str, *, body: str | None = None
+    store: KBStore, page_id: str, *, source_id: str | None = None
 ) -> bool:
     """Return True if a pending proposal already targets ``page_id``.
 
-    When ``body`` is supplied, only returns True if the pending proposal
-    also carries the same body — allowing a second different vault edit
-    to file a new proposal even while the first is still pending.
-    Prevents duplicate proposals when vault_to_kb runs multiple times
-    before the reviewer approves the first proposal for a given page edit.
+    When ``source_id`` is supplied, only returns True if the pending
+    proposal also cites that vault-edit source — allowing a second,
+    *different* vault edit to file its own proposal even while the first
+    is still pending, rather than being silently coalesced into it and
+    then clobbered by the next backward sync pass. The source id is the
+    content-address (sha256) of the whole mirror file `put_source` would
+    assign — the canonical fingerprint of the complete edit (title, type,
+    tags, claims, entities, body), not just the body text. Without this
+    guard, running vault_to_kb twice before the first proposal is
+    approved files duplicate proposals for the same edit, cluttering the
+    review queue and causing the second approve to fail with "page
+    already exists".
     """
     from .models import ProposalKind, ProposalStatus
     for proposal in store.list_proposals(ProposalStatus.PENDING):
@@ -331,7 +338,7 @@ def _has_pending_page_proposal(
         payload = proposal.payload
         if not isinstance(payload, dict) or payload.get("id") != page_id:
             continue
-        if body is None or payload.get("body") == body:
+        if source_id is None or source_id in (payload.get("sources") or []):
             return True
     return False
 
@@ -398,12 +405,15 @@ def vault_to_kb(
             result.pages_skipped_unknown_id.append(rel)
             continue
 
-        # Fix 2 (#219): skip if a pending proposal already targets this page
-        # id. Without this guard, running vault_to_kb twice before the first
-        # proposal is approved files duplicate proposals for the same edit,
-        # cluttering the review queue and causing the second approve to fail
-        # with "page already exists".
-        if _has_pending_page_proposal(store, page_id):
+        # Fix 2 (#219, refined by #706): skip if a pending proposal already
+        # targets this page id *with this exact edit* (source_id=current_hash
+        # — the content address the vault-edit source below will get).
+        # Without the source_id check, a second, different edit made while
+        # the first proposal is still pending would be misclassified as the
+        # same duplicate, skipped without a new proposal, and then silently
+        # clobbered by the next backward sync pass restoring the mirror to
+        # canonical KB content — the second edit vanishing with no trace.
+        if _has_pending_page_proposal(store, page_id, source_id=current_hash):
             log.debug(
                 "vault sync: pending proposal already exists for page %r; "
                 "skipping to avoid duplicate",

tests/test_vault_sync.py

@@ -590,6 +590,49 @@ def test_vault_to_kb_deduplicates_pending_proposals(
     )
 
 
+def test_vault_to_kb_files_new_proposal_for_second_distinct_edit(
+    store: KBStore, vault: Path,
+) -> None:
+    """A second, *different* vault edit made while the first edit's proposal
+    is still pending must get its own proposal, not be silently coalesced
+    into the first (and then lost once a later backward pass reverts the
+    mirror to canonical KB content, since the KB itself never learned about
+    the second edit)."""
+    kb_to_vault(store, vault)
+    mirror = vault / VAULT_DIR / "pages" / "alpha-page.md"
+    base_text = mirror.read_text(encoding="utf-8")
+
+    edit_a = base_text.replace("Original body.", "Edit A.")
+    mirror.write_text(edit_a, encoding="utf-8")
+    r1 = vault_to_kb(store, vault, actor="vault-sync")
+    assert "alpha-page" in r1.pages_proposed
+    proposals_after_first = list((store.kb_dir / "proposed").glob("*.yaml"))
+    assert len(proposals_after_first) == 1
+
+    # A different edit lands before the first proposal is approved (e.g. the
+    # user tweaks the page again on the next sync tick).
+    edit_b = base_text.replace("Original body.", "Edit B, unrelated to A.")
+    mirror.write_text(edit_b, encoding="utf-8")
+    r2 = vault_to_kb(store, vault, actor="vault-sync")
+    assert "alpha-page" in r2.pages_proposed, (
+        "a distinct second edit must file its own proposal instead of "
+        "being skipped as a duplicate of the first"
+    )
+    proposals_after_second = list((store.kb_dir / "proposed").glob("*.yaml"))
+    assert len(proposals_after_second) == 2, (
+        f"expected 2 proposals (one per distinct edit), got "
+        f"{len(proposals_after_second)}"
+    )
+
+    bodies = set()
+    for path in proposals_after_second:
+        import yaml
+        payload = yaml.safe_load(path.read_text(encoding="utf-8"))
+        bodies.add(payload["payload"]["body"])
+    assert any("Edit A." in b for b in bodies)
+    assert any("Edit B, unrelated to A." in b for b in bodies)
+
+
 def test_vault_to_kb_warns_on_claim_stub_edit(
     store: KBStore, vault: Path, caplog: pytest.LogCaptureFixture,
 ) -> None: