feat(health): vouch fsck — deep consistency checks beyond doctor#112
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughA new Changesvouch fsck feature
Sequence DiagramsequenceDiagram
participant CLI as vouch CLI
participant Health as health.fsck
participant IndexDB as state.db (FTS)
participant FS as on-disk artifacts
participant Report as HealthReport
CLI->>Health: invoke fsck(store)
Health->>FS: scan claims/pages/entities on disk
Health->>IndexDB: query FTS tables and embedding tables
Health->>Health: run lifecycle, decided, index-drift, embedding checks
Health->>Report: emit findings and ok flag
Health-->>CLI: return report
CLI->>Report: print findings / "clean" and exit(code)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 146: The README added the new CLI command "vouch fsck" but the "What
ships today" CLI inventory list still omits it; update that second CLI list to
include "vouch fsck" (with the same short description "deep consistency:
indexes, lifecycle, decided" used on Line 146) so both lists match—edit the
README section titled "What ships today" and add the "vouch fsck" entry to the
CLI inventory.
In `@src/vouch/cli.py`:
- Around line 195-197: The CLI output loop over report.findings currently prints
only the severity marker, f.code and f.message; update it to also include the
finding's affected object IDs (Finding.object_ids) so users see which objects
are impacted. In the block iterating report.findings (where f.severity, f.code
and f.message are used), append a formatted representation of f.object_ids (e.g.
join the list or fallback to a compact repr when empty) to the click.echo string
only when object_ids is non-empty, ensuring the printed line becomes something
like "<marker> [code] message (objects: id1, id2)".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f009561f-c95e-4661-a74c-4f5ff4e8272e
📒 Files selected for processing (6)
CHANGELOG.mdREADME.mdsrc/vouch/cli.pysrc/vouch/health.pytests/test_cli.pytests/test_health.py
Addresses review comments on vouchdev#112: - CLI fsck output now appends `(objects: id1, id2)` when a finding has structured object_ids, so users can grep / pipe straight to inspection. - The 'What ships today' CLI inventory in README.md now lists `fsck` alongside `doctor`, matching the cheatsheet earlier in the file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_health.py (2)
91-99: ⚡ Quick winMissing
report.ok is Falseassertion for consistency.Unlike
test_fsck_flags_dangling_supersedes(line 88), this test doesn't verifyreport.ok is False. Sincedangling_superseded_byis an error-severity finding, the assertion would improve test coverage consistency.Suggested fix
def test_fsck_flags_dangling_superseded_by(store: KBStore) -> None: """`claim.superseded_by` pointing at a missing claim is an error.""" src = store.put_source(b"e") store.put_claim(Claim(id="c1", text="t", evidence=[src.id], superseded_by="ghost")) report = health.fsck(store) codes = {f.code for f in report.findings} assert "dangling_superseded_by" in codes + assert report.ok is False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_health.py` around lines 91 - 99, Add an assertion in test_fsck_flags_dangling_superseded_by to check that report.ok is False (mirroring test_fsck_flags_dangling_supersedes); locate the test function test_fsck_flags_dangling_superseded_by and after computing report = health.fsck(store) add assert report.ok is False so error-severity finding "dangling_superseded_by" fails the overall report status.
101-109: ⚡ Quick winMissing
report.ok is Falseassertion.Same as above —
dangling_contradictsis error-severity so the test should verify the report fails.Suggested fix
def test_fsck_flags_dangling_contradicts(store: KBStore) -> None: """`claim.contradicts` pointing at a missing claim is an error.""" src = store.put_source(b"e") store.put_claim(Claim(id="c1", text="t", evidence=[src.id], contradicts=["ghost"])) report = health.fsck(store) codes = {f.code for f in report.findings} assert "dangling_contradicts" in codes + assert report.ok is False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_health.py` around lines 101 - 109, The test test_fsck_flags_dangling_contradicts is missing an assertion that the health.fsck report is failing; add an assertion that report.ok is False (e.g., assert report.ok is False) in that test after calling report = health.fsck(store) so the test verifies that the dangling_contradicts finding is treated as an error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_health.py`:
- Around line 91-99: Add an assertion in test_fsck_flags_dangling_superseded_by
to check that report.ok is False (mirroring
test_fsck_flags_dangling_supersedes); locate the test function
test_fsck_flags_dangling_superseded_by and after computing report =
health.fsck(store) add assert report.ok is False so error-severity finding
"dangling_superseded_by" fails the overall report status.
- Around line 101-109: The test test_fsck_flags_dangling_contradicts is missing
an assertion that the health.fsck report is failing; add an assertion that
report.ok is False (e.g., assert report.ok is False) in that test after calling
report = health.fsck(store) so the test verifies that the dangling_contradicts
finding is treated as an error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0badeed7-079e-4d2e-ae07-41bf6ed65f59
📒 Files selected for processing (3)
src/vouch/health.pytests/test_cli.pytests/test_health.py
|
PR should target test branch |
Adds a `vouch fsck` command that runs deeper integrity checks than `vouch doctor`: - orphaned embeddings (rows in `embeddings` / `embedding_index` for artifacts that no longer exist on disk) - dangling supersede / contradict chains (claim.supersedes, superseded_by, contradicts pointing at missing claims; asymmetric contradicts edges) - decided proposals whose approved artifact is missing on disk - FTS5 index drift: orphan rows, missing rows, and the `claims_fts.status` drift shape from vouchdev#78 Read-only; reports findings with object ids. `--fix` is intentionally out of scope for v1 per the acceptance criteria. Closes vouchdev#96 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses review comments on vouchdev#112: - CLI fsck output now appends `(objects: id1, id2)` when a finding has structured object_ids, so users can grep / pipe straight to inspection. - The 'What ships today' CLI inventory in README.md now lists `fsck` alongside `doctor`, matching the cheatsheet earlier in the file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit's pre-merge docstring-coverage check was at 23% for this change set. Add concise docstrings to: - the four `_check_*` helpers in `health.py` — each one names *why* the check exists (the failure mode it catches), not just what it does - the `_index_claim` test helper and the eleven new `test_fsck_*` tests — one-line summaries of what each test asserts No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…radicts} Both tests checked the finding code was emitted but didn't verify the error-severity finding actually flipped the report status. Mirrors the sibling `test_fsck_flags_dangling_supersedes`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Khaostica
force-pushed
the
feat/vouch-fsck
branch
from
5e16199 to
d12e7dd
Compare
# Conflicts: # src/vouch/health.py # tests/test_cli.py
# Conflicts: # CHANGELOG.md
|
One thing worth tightening: Non-blocking: most of the +422 is the same shape repeated across claims/pages/entities. A little helper that takes |
Address review on vouchdev#112. - fsck() now loads claims via _load_claims_for_lint() like lint(), so a single invalid YAML (e.g. a legacy uncited claim from before vouchdev#81) becomes an `invalid_claim` finding instead of aborting the whole check with a traceback — the exact inconsistency a deep checker should surface. Counts are built from the safely-loaded claims via a shared _safe_counts() helper so the report can't re-trip the strict loader. - Add _drift_findings(kind, indexed_ids, on_disk_ids, findings) to emit the orphan + missing-row findings, collapsing the three near-identical blocks in _check_index_drift. Codes and messages are unchanged; adding a kind is now a one-liner. Add test_fsck_surfaces_invalid_claim_yaml_without_crashing.
# Conflicts: # CHANGELOG.md
3 participants
What changed
Adds
vouch fsck, a deep consistency check that goes beyondvouch doctor. Each finding is reported with the affected object ids; the command exits non-zero on anyerror-severity finding.The new checks are:
embeddings(legacy) orembedding_indexforkind/idpairs whose underlying artifact file no longer exists on disk.claim.supersedes,claim.superseded_by, andclaim.contradictsreferences pointing at missing claims; asymmetriccontradictsedges (A says it contradicts B but B does not contradict A).decided/whose target artifact (claim/page/entity/relation) is no longer on disk.index_orphan_*), durable artifacts with no FTS5 row (index_missing_row), andclaims_fts.statusdrift against the on-diskclaim.status(the failure shape from bug: kb.context returns archived/superseded claims; lifecycle mutations never re-index FTS5 status #78).When
state.dbis absent, fsck emits aninfo-severityindex_missingfinding (same convention asdoctor) and the report staysok=True.Closes #96
Why
doctor/lintare intentionally surface-level — they catch broken citations, stale claims, and dangling relations. Recent bugs (#78 archived-claim leakage; #80 missing bundle members) were each symptoms of a missing systematic integrity pass.fsckconsolidates the deeper checks into one command that an operator can run to ask the KB "are your derived caches and lifecycle chains internally consistent?"What might break
Additive, CLI + library only. No
.vouch/files move, no persisted shapes change, nokb.*methods change, no bundle format change, no audit-log shape change. Existing.vouch/directories work unchanged.VEP
Not required. No change to the object model,
kb.*method surface, on-disk layout, bundle format, or audit-log shape — this only adds a read-only CLI command and ahealth.fsck()library function.Tests
python -m ruff check src testspassespython -m pytest tests/test_health.py tests/test_cli.py— 27 passedpython -m pytest— only the pre-existing Windows-onlyO_NOFOLLOWfailures remain (unrelated to this change)tests/test_health.py(10 cases, one per finding code + clean-KB + no-state.db) and a CLI smoke test intests/test_cli.pyCHANGELOG.mdupdated under## [Unreleased]README.mdcheatsheet updatedSummary by CodeRabbit
New Features
Documentation
Tests