Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
- **`vouch fsck` no longer crashes on an approved delete proposal** (#538
reopened, root-caused): `_check_decided_proposals` indexed a `presence`
dict by every approved proposal's own kind, but the dict has no entry
for `ProposalKind.DELETE` — so any KB that had ever had a delete
approved crashed `fsck()` with an uncaught `KeyError`. Delete proposals
are now checked in a first pass against their `target_kind` (reporting
`decided_delete_invalid_target_kind` for a missing/unrecognized one, or
`decided_delete_artifact_present` if the target wasn't actually
removed), and the artifact ids they legitimately deleted are excluded
from the second pass so the *original* creating proposal doesn't
false-positive as `decided_missing_artifact` once its artifact is
correctly gone.
- **`reset()`/`deindex()` now clear the legacy `embeddings` table too**
(#543 reopened, root-caused): both functions' own docstrings promise to
remove every embedding row for a reindex or a deleted artifact, but
Expand Down
12 changes: 9 additions & 3 deletions src/vouch/capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,15 @@ def load_config(store: KBStore) -> CaptureConfig:
return CaptureConfig(
enabled=coerce_bool(raw.get("enabled", DEFAULT_ENABLED), DEFAULT_ENABLED),
realtime=coerce_bool(raw.get("realtime", DEFAULT_REALTIME), DEFAULT_REALTIME),
min_observations=int(raw.get("min_observations", DEFAULT_MIN_OBSERVATIONS)),
dedup_window_seconds=float(
raw.get("dedup_window_seconds", DEFAULT_DEDUP_WINDOW_SECONDS)
min_observations=coerce_numeric(
raw.get("min_observations", DEFAULT_MIN_OBSERVATIONS),
DEFAULT_MIN_OBSERVATIONS,
int,
),
dedup_window_seconds=coerce_numeric(
raw.get("dedup_window_seconds", DEFAULT_DEDUP_WINDOW_SECONDS),
DEFAULT_DEDUP_WINDOW_SECONDS,
float,
),
answer_mode=answer_mode,
)
Expand Down
64 changes: 62 additions & 2 deletions src/vouch/health.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,10 @@ def _check_decided_proposals(

A crash between `put_<kind>()` and `move_proposal_to_decided()` would
leave a `decided/` entry without a matching artifact (or vice versa);
surface the artifact-missing case so an operator can investigate.
surface the artifact-missing case so an operator can investigate. A
``DELETE`` proposal is the inverse: approving it removes the artifact,
so its target must be *absent*, checked against ``target_kind`` (the
kind of what was deleted) rather than ``pr.kind`` (always ``DELETE``).
"""
relations = {r.id for r in store.list_relations()}
presence: dict[ProposalKind, set[str]] = {
Expand All @@ -482,7 +485,62 @@ def _check_decided_proposals(
ProposalKind.ENTITY: set(entities),
ProposalKind.RELATION: relations,
}
for pr in store.list_proposals(ProposalStatus.APPROVED):
approved = list(store.list_proposals(ProposalStatus.APPROVED))

# First pass: an approved DELETE proposal's target is expected to be
# absent. Collect what it legitimately removed so the second pass
# (which checks that a create/edit proposal's artifact still exists)
# doesn't flag the artifact its own delete proposal correctly removed.
deleted: dict[ProposalKind, set[str]] = {k: set() for k in presence}
for pr in approved:
if pr.kind is not ProposalKind.DELETE:
continue
artifact_id = pr.payload.get("id") if isinstance(pr.payload, dict) else None
if not artifact_id:
findings.append(
Finding(
"error",
"decided_no_artifact_id",
f"approved proposal {pr.id} has no payload id",
[pr.id],
)
)
continue
target_kind_str = (
pr.payload.get("target_kind") if isinstance(pr.payload, dict) else None
)
try:
target_kind = ProposalKind(target_kind_str) if target_kind_str else None
except ValueError:
target_kind = None
if target_kind is None or target_kind not in presence:
findings.append(
Finding(
"error",
"decided_delete_invalid_target_kind",
f"approved delete proposal {pr.id} has an invalid or "
f"missing target_kind {target_kind_str!r}",
[pr.id],
)
)
continue
if artifact_id in presence[target_kind]:
findings.append(
Finding(
"error",
"decided_delete_artifact_present",
f"approved delete proposal {pr.id} targeted "
f"{target_kind.value} {artifact_id}, but the artifact "
f"still exists on disk",
[pr.id, artifact_id],
)
)
else:
deleted[target_kind].add(artifact_id)

for pr in approved:
if pr.kind is ProposalKind.DELETE:
continue
artifact_id = pr.payload.get("id") if isinstance(pr.payload, dict) else None
if not artifact_id:
findings.append(
Expand All @@ -494,6 +552,8 @@ def _check_decided_proposals(
)
)
continue
if artifact_id in deleted[pr.kind]:
continue # removed by a later, separately-verified delete proposal
if artifact_id not in presence[pr.kind]:
findings.append(
Finding(
Expand Down
66 changes: 66 additions & 0 deletions tests/test_health.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from vouch import health, index_db
from vouch.models import Claim, ClaimStatus, Proposal, ProposalKind, ProposalStatus
from vouch.proposals import approve, propose_claim, propose_delete
from vouch.storage import KBStore, _yaml_dump


Expand Down Expand Up @@ -315,6 +316,71 @@ def test_fsck_decided_missing_artifact(store: KBStore) -> None:
assert "decided_missing_artifact" in codes


def test_fsck_survives_approved_delete_proposal(store: KBStore) -> None:
"""An approved delete proposal must not crash fsck, and the artifact it
correctly removed must not be flagged as a missing create-time artifact."""
src = store.put_source(b"evidence")
pr = propose_claim(store, text="a fact", evidence=[src.id], proposed_by="agent-a")
claim = approve(store, pr.id, approved_by="human-b")

del_pr = propose_delete(
store, target_kind="claim", target_id=claim.id, proposed_by="agent-a",
)
approve(store, del_pr.id, approved_by="human-c")

report = health.fsck(store)
assert report.findings == []


def test_fsck_flags_delete_whose_artifact_still_exists(store: KBStore) -> None:
"""A delete proposal is approved, but the artifact it claims to have
removed is still on disk — the delete never actually took effect."""
src = store.put_source(b"evidence")
claim = Claim(id="still-here", text="t", evidence=[src.id])
store.put_claim(claim)
store.put_proposal(Proposal(
id="del-1",
kind=ProposalKind.DELETE,
proposed_by="agent",
payload={"target_kind": "claim", "id": "still-here", "snapshot": {}},
status=ProposalStatus.APPROVED,
))

report = health.fsck(store)
codes = {f.code for f in report.findings}
assert "decided_delete_artifact_present" in codes


def test_fsck_flags_delete_with_invalid_target_kind(store: KBStore) -> None:
store.put_proposal(Proposal(
id="del-2",
kind=ProposalKind.DELETE,
proposed_by="agent",
payload={"target_kind": "not-a-real-kind", "id": "whatever", "snapshot": {}},
status=ProposalStatus.APPROVED,
))

report = health.fsck(store)
codes = {f.code for f in report.findings}
assert "decided_delete_invalid_target_kind" in codes


def test_fsck_flags_delete_proposal_with_no_artifact_id(store: KBStore) -> None:
"""A malformed DELETE proposal with no payload id is reported the same
way a malformed create/edit proposal already is, not silently skipped."""
store.put_proposal(Proposal(
id="del-3",
kind=ProposalKind.DELETE,
proposed_by="agent",
payload={"target_kind": "claim", "snapshot": {}},
status=ProposalStatus.APPROVED,
))

report = health.fsck(store)
codes = {f.code for f in report.findings}
assert "decided_no_artifact_id" in codes


def test_fsck_index_orphan_row(store: KBStore) -> None:
"""An FTS5 row with no on-disk claim is reported as an index orphan."""
src = store.put_source(b"e")
Expand Down
Loading