diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a7c8868..05bb0ee9 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/vouch/vault_sync.py b/src/vouch/vault_sync.py index ba86db7f..e75e6307 100644 --- a/src/vouch/vault_sync.py +++ b/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", diff --git a/tests/test_vault_sync.py b/tests/test_vault_sync.py index 84d3f92c..484a2d68 100644 --- a/tests/test_vault_sync.py +++ b/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: