Fix/required field validators#156
Closed
philluiz2323 wants to merge 83 commits into
Closed
Conversation
…ouchdev#76) crystallize wrote a durable Page directly through store.put_page, embedding sess.task, sess.note, and sess.agent verbatim into the rendered markdown body. The body was never gated by propose_page + approve, so an agent calling kb.session_start(task=<payload>) and getting any one claim approved via crystallize could land arbitrary content into pages/. The page surfaces in kb.read_page, kb.list_pages, kb.context, and (once vouchdev#60 is fixed) kb.search. Restrict the summary body to fields the proposing agent cannot influence: session id (server-generated), timestamps (server clock), and the list of approved artifact ids. The agent-controlled fields remain on the Session model itself and are still queryable, but no longer promoted into a durable Page. Also include summary_page_id in the session.crystallize audit event's object_ids when a page is written, so vouch audit truthfully attributes the write. Adds two regression tests: - test_crystallize_summary_page_does_not_leak_agent_controlled_fields - test_crystallize_audit_event_records_summary_page_id
Leftover from the merge of main (which brought PR vouchdev#62's test_sessions.py changes); ruff F401 was failing CI on PR vouchdev#77.
…index FTS5 on update (vouchdev#78) Two compounding bugs let archived / superseded / redacted claims keep flowing back to agents through kb.context: 1. build_context_pack had no claim.status filter, so any retrieval hit was appended to the pack regardless of subsequent lifecycle mutations. 2. store.update_claim refreshed the embedding cache but never re-indexed the FTS5 row, so claims_fts.status stayed frozen at first-index time. Every lifecycle op (archive, supersede, contradict, confirm) routes through update_claim and was therefore invisible to FTS5 / semantic search. This made ClaimStatus.{ARCHIVED, SUPERSEDED, REDACTED} purely decorative on the read side — agents kept quoting retracted knowledge as if it were live. Fix: - storage.update_claim now also calls index_db.index_claim under a short-lived sqlite connection (mirroring proposals.approve's first-index pattern). FTS5 errors are logged-and-skipped so a lifecycle op never fails because the embedding/FTS5 layer is misconfigured. - context.build_context_pack resolves each claim hit, drops it if the on-disk status is in {ARCHIVED, SUPERSEDED, REDACTED}, and drops it if the claim YAML has been deleted between index time and now. CONTESTED claims keep surfacing so contradictions remain visible. Tests: - test_context_pack_excludes_archived_claims - test_context_pack_excludes_superseded_claims - test_update_claim_refreshes_fts5_status (asserts the FTS5 status column via direct SQL against state.db) No on-disk-layout, schema, or bundle-format change.
Resolves conflicts in CHANGELOG.md, tests/test_sessions.py, and auto-merges src/vouch/sessions.py with the recently-merged: - vouchdev#61 (FTS5 indexing of crystallize summary page) — adds index_db.index_page(...) right after store.put_page(...) inside crystallize. Composes cleanly with this PR's strict-derivation _build_summary_body and the summary_page_id audit fix. - vouchdev#62 (test_crystallize_collects_approval_failures) — preserved as a sibling test. - vouchdev#75 (bundle import sha256 verification) — CHANGELOG entry only. - vouchdev#73 (bundle POSIX separators) — CHANGELOG entry only. Both regression tests added by this PR (test_crystallize_summary_page_does_not_leak_agent_controlled_fields and test_crystallize_audit_event_records_summary_page_id) still pass against the merged sessions.crystallize.
vouchdev#81) The 'claims must cite sources' guarantee (README §'Why this exists' point 3; CONTRIBUTING §'Things we won't merge') used to live only in proposals.propose_claim, so every other write path silently accepted Claim(evidence=[]) and landed an uncited claim: - store.put_claim direct: existence-check loop iterates zero times. - store.update_claim: writes the YAML without re-validating. - bundle.import_apply via _validate_content: defers to Claim.model_validate, which accepted evidence=[] because the model had no min-length constraint. Add @field_validator('evidence') on Claim — raises ValueError when the list is empty. Closes all three bypass paths in one place. store.update_claim additionally re-validates via Claim.model_validate(claim.model_dump()) before persisting, so in-place mutation (c.evidence = []; store.update_claim(c)) raises before the YAML hits disk — the field validator only fires at construction time, not on attribute assignment. Four regression tests: - test_claim_model_rejects_empty_evidence (tests/test_storage.py) — Claim(evidence=[]) raises pydantic.ValidationError. - test_put_claim_rejects_empty_evidence — store.put_claim raises; no claims/<id>.yaml is written. - test_update_claim_rejects_empty_evidence — in-place mutation + update_claim raises; the on-disk YAML is unchanged. - test_import_rejects_uncited_claim (tests/test_bundle.py) — a schema-valid bundle whose claim YAML has evidence: [] is rejected by import_check (schema validation issue) and import_apply raises before writing. The existing guard in proposals.propose_claim becomes a redundant user-facing error message and is left in place for the friendlier CLI/JSONL error string. No on-disk-layout, schema, or bundle-format change; data the model never should have accepted now raises.
…ing from tarball (vouchdev#80) import_check previously only verified that tar members listed in the manifest had matching hashes, but never checked the reverse: whether every manifest entry had a corresponding tar member. A bundle whose manifest.json referenced claims/c1.yaml but whose tarball contained only manifest.json would pass import_check with ok=True, and import_apply would silently write nothing — no exception, no audit event indicating data loss. Add the missing-member pass (mirroring the existing check in export_check) so that manifest entries without a matching tar member produce a "manifest lists missing file" issue. import_apply then refuses to import because check.issues is non-empty.
read-only `vouch diff <old> <new>` that shows what changed between two claim or two page revisions: field-level changes plus a line-diff of the long text/body. auto-detects kind, hides churning metadata.
new read-only `vouch diff <old> <new>` compares two claims or two pages by id and shows what changed: scalar/list fields as old → new, plus a line-diff of the long text/body. auto-detects kind, hides churning metadata (timestamps, approved_by), supports --json. closes a roadmap 0.1 item.
mypy inferred old/new as Claim from the first branch, so the page branch's reassignment tripped an incompatible-assignment error in CI. annotate the union up front.
feat(diff): add `vouch diff` for claim/page revisions
…hdev#82 review) The new Claim.evidence min-citation validator (vouchdev#81) also fires when claims are read back from disk. A KB that has a pre-existing uncited claims/<id>.yaml from before the fix would otherwise crash vouch lint / vouch doctor with a bare pydantic.ValidationError deep in store.list_claims(). Add _load_claims_for_lint(), a per-file iteration that catches pydantic.ValidationError (and any other load error) and surfaces each bad file as a Finding with code='invalid_claim' and an explicit repair hint: 'edit the YAML to add a citation, or delete the file'. lint() also stops calling status() to populate counts — status() calls the strict store.list_claims() which would re-raise on the same files — and builds the counts dict inline from the safely-loaded valid claims. Regression test in tests/test_health.py: - test_lint_surfaces_legacy_uncited_claim_yaml_without_crashing hand-crafts a claims/legacy.yaml with evidence: [] (matches the on-disk shape an older buggy write path would have left), asserts vouch lint runs to completion, surfaces invalid_claim in findings with the repair-hint message, and that the well-formed sibling claim is still discovered. CHANGELOG migration note expanded to describe the repair hint.
The new inline-built counts in lint() (from 5c881de) is a literal dict with mixed value types (str/int/bool), which mypy correctly inferred as dict[str, object] and rejected against the HealthReport.counts: dict[str, int] annotation. The original status() returned the same mixed dict via an untyped 'dict' return, which masked the mismatch — the narrow type was effectively never checked at the call site. Widen counts to dict[str, Any] to match runtime reality. Also tighten status()'s return annotation from 'dict' to 'dict[str, Any]' for consistency. No caller does arithmetic on counts values; they just echo or pass through, so the widening is risk-free.
feat: add guided proposal review CLI
Feat/pending json
…ch-sync Feat/deterministic vouch sync
…ummary-page-bypass fix(sessions): close crystallize review-gate bypass via summary page
…s-citation fix(models): require Claim.evidence to be non-empty at the model layer
…nd-config fix(context): honor retrieval.backend config instead of hardcoding embeddings
feat(cli): vouch approve accepts multiple ids for scriptable bulk approval
generalize init seeding into a small in-code template registry. default 'starter' is unchanged; new 'gittensor' template seeds a cited, approved pack (1 source, 1 entity, 4 claims) about SN74 contribution scoring so a fresh kb in a gittensor repo has retrievable context immediately. unknown --template gives a clean error listing available packs. complements gittensory's live-data signals rather than cloning them.
# Conflicts: # src/vouch/health.py # tests/test_cli.py
# Conflicts: # src/vouch/cli.py
feat(init): add --template with a Gittensor (SN74) starter pack
…late feat(init): add --template with a gittensor seed pack
…chdev#97) Adds a `vouch.logging_config` module with a `JsonFormatter` and an idempotent `configure_logging()` that the CLI, MCP server, and JSONL server entry points call at startup. When `VOUCH_LOG_FORMAT=json`, the `vouch` logger emits one JSON object per line with `level`, `logger`, `event`, and any structured extras (`actor`, `object_ids`, ...) passed via the stdlib `extra=` argument. Any other value (including unset) is a no-op - vouch's loggers keep stdlib default behaviour, so callers who don't opt in see no change. Scoped to the `vouch` logger namespace with propagate=False in JSON mode, so this never reformats logs emitted by libraries vouch happens to depend on.
# Conflicts: # CHANGELOG.md
…tion-docs docs(gittensor): adoption guide (init, MCP wiring, decision workflow)
…le-proposals Feat/vouch expire stale proposals
the var was already documented in adapters/generic-mcp/README ('force a
specific KB root, skip discovery') but never wired into the code. now
`discover_root()` reads VOUCH_KB_PATH first and short-circuits the upward
walk, with a clear error if the value isn't an existing .vouch directory.
closes the doc-vs-code drift and removes the per-host 'cwd: ...' ceremony
needed by Claude Desktop (and any other host that launches the stdio
server from a default working directory).
fix(storage): honour VOUCH_KB_PATH env var in discover_root
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.
Replace the _vouch_managed attribute (and its type: ignore[attr-defined]) with a _VouchManagedHandler(StreamHandler) subclass. configure_logging() identifies its handler by isinstance for install/reuse/remove, which reads cleaner and drops the attribute hack. Tests updated to match.
# Conflicts: # CHANGELOG.md
feat(health): vouch fsck — deep consistency checks beyond doctor
…t-json feat: structured JSON logging via VOUCH_LOG_FORMAT=json
…ent-addressing fix: enforce Source content-addressing on bundle/sync import
Co-authored-by: Cursor <cursoragent@cursor.com>
feat: add vouch migrate for safe on-disk KB upgrades
tiered adoption story under adapters/claude-code/: .mcp.json (T1), fenced CLAUDE.md snippet (T2), four slash commands (T3), settings.json with SessionStart hook + read-only auto-allow (T4). new vouch install-mcp claude-code [--tier T1..T4] writes them idempotently into a project.
docs: plan for Claude Code adapter (4 tiers + install-mcp CLI)
📝 WalkthroughWalkthroughThis PR implements the comprehensive 0.1.0 release surface: model validation enforcement (non-empty evidence/text/name/title), on-disk KB format versioning with forward-only migrations, proposal lifecycle management (approval gating with self-approval rules, batch approval, pending expiration), bidirectional KB synchronization via directories or bundles, deep health checks (fsck), artifact diffing, template-based init (gittensor), structured JSON logging, and integrated CLI/operational improvements (colors, JSON modes, interactive review queue). Includes 100+ line CHANGELOG update and extensive test coverage across all features. ChangesModel & Schema Validation
Infrastructure & Lifecycle
Proposal & Review Lifecycle
Data Transfer & Synchronization
Operational & UX
Documentation & Specs
🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/vouch/storage.py (1)
181-197:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Windows TOCTOU escape in
read_under_root()(fail-closed without atomic no-follow)
- On Windows,
os.O_NOFOLLOWis not available, so the current fallback (flags |= getattr(os, "O_NOFOLLOW", 0)) is a no-op; a symlink/junction swapped in afterPath(path).resolve()/is_relative_to(self.root)can redirectos.open()to a regular file outsideself.root.- The later
fstat()/stat.S_ISREG(...)check only verifies “regular file”, not that the opened target still resides withinself.root, soread_under_root()can still become an arbitrary-file-read primitive viakb.register_source_from_path.🔒 Possible direction
- flags = os.O_RDONLY - # POSIX can reject a symlink swapped in after resolve(); Windows has - # no O_NOFOLLOW, so it falls back to the regular-file check below. - flags |= getattr(os, "O_NOFOLLOW", 0) + flags = os.O_RDONLY + nofollow = getattr(os, "O_NOFOLLOW", None) + if nofollow is None: + raise ValueError( + "read_under_root() requires an atomic no-follow open on this platform" + ) + flags |= nofollow🤖 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 `@src/vouch/storage.py` around lines 181 - 197, The read_under_root() code currently tries to use O_NOFOLLOW but silently no-ops on Windows, leaving a TOCTOU/symlink escape; modify read_under_root() to fail-closed on platforms that do not support atomic no-follow: before calling os.open(resolved, flags) check whether getattr(os, "O_NOFOLLOW", 0) is non-zero (or use hasattr/os.supported flag) and if it is zero/absent raise a ValueError indicating the platform cannot safely open with no-follow; keep the existing POSIX path that adds O_NOFOLLOW, opens with os.open(resolved, flags), fstat-checks with stat.S_ISREG and returns the contents, and do not attempt a non-atomic post-open path check on platforms without O_NOFOLLOW (i.e., refuse to operate instead of falling back to unsafe behavior).src/vouch/bundle.py (1)
416-420:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRe-run the source content-address check during
import_apply.The new
sources/<sha>invariant is only enforced inimport_check(). If the bundle changes between that pass and this re-open, the write-time path will still accept asources/<sha>/contentbody whose bytes no longer hash to<sha>, because it only reruns_validate_content()here.Suggested fix
val_issues: list[str] = [] _validate_content(member.name, body, val_issues) - if val_issues: - skipped.append(member.name) - continue + _check_source_content_address(member.name, body, val_issues) + if val_issues: + raise RuntimeError(f"refusing to import: {val_issues[0]}") dest.write_bytes(body)🤖 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 `@src/vouch/bundle.py` around lines 416 - 420, During import_apply, re-run the content-address verification for source blobs instead of only calling _validate_content() which may miss byte-hash mismatches: update import_apply to compute the expected SHA from the member.name (the sources/<sha> segment) and verify the actual body bytes hash equals that SHA (same check performed in import_check), and reject or skip the member when the computed hash does not match; ensure this verification occurs in the same block that currently calls _validate_content() and affects skipped/apply behavior so write-time accepts only content whose bytes hash to the declared <sha>.
🧹 Nitpick comments (3)
docs/PROJECT-INDEX.md (2)
32-32: ⚡ Quick winAdd language identifiers to fenced code blocks.
These fenced blocks are missing language tags (MD040). Please annotate them (e.g.,
text,bash) to satisfy markdownlint and improve readability.Also applies to: 128-128, 174-174, 202-202, 219-219, 239-239, 412-412
🤖 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 `@docs/PROJECT-INDEX.md` at line 32, Several fenced code blocks use bare triple backticks (```) with no language tag; update each such block (the ones using ``` in the document) to include an appropriate language identifier (e.g., ```text, ```bash, ```json) so markdownlint MD040 is satisfied and syntax highlighting improves—search for occurrences of the plain ``` fences and replace them with the correct fenced form based on the block content.
292-292: ⚡ Quick winSurround tables with blank lines for markdownlint compliance.
These table starts are flagged by MD058. Add blank lines before/after each table block to keep docs lint-clean.
Also applies to: 299-299, 306-306, 314-314, 322-322, 328-328, 337-337, 344-344, 353-353, 359-359, 364-364, 372-372, 516-516, 524-524
🤖 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 `@docs/PROJECT-INDEX.md` at line 292, Multiple markdown tables (e.g., the table starting with the header "| Command | Description |") are not surrounded by blank lines causing MD058 failures; fix by adding a single blank line immediately before and after each table block so every table is separated from surrounding text/paragraphs (apply the same change to the other table occurrences flagged in the comment). Ensure you preserve the exact table content and only add the blank lines to satisfy markdownlint compliance.tests/test_retrieval_backend.py (1)
48-101: ⚡ Quick winAdd one regression for the legacy
retrieval.backendsfallback.
_configured_backend()now has a compatibility branch for the old list form, but this file only exercises the new singularretrieval.backendkey. A small test forretrieval.backends: [...]would lock in that backwards-compatibility path.🤖 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_retrieval_backend.py` around lines 48 - 101, Add a regression test that exercises the legacy retrieval.backends list fallback by writing a config with retrieval.backends: ["embedding"] (or a list containing the legacy value) into the store config, then call context.build_context_pack(store, query="JWT") with _force_semantic_hit and assert the returned pack contains items and that _backends(pack) shows the expected backend (e.g., {"embedding"}) or that any item["backend"] == "embedding"; reuse helpers used in this file (_force_semantic_hit, _set_backend style of config mutation, context.build_context_pack, and _backends) to mirror the existing tests for coverage of the compatibility branch.
🤖 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 `@docs/superpowers/plans/2026-05-27-claude-code-adapter.md`:
- Line 20: Three fenced code blocks in the document are missing language tags
(the file-tree block beginning with "adapters/claude-code/ README.md", the
frontmatter block beginning with "--- description: Show vouch KB health —
counts, pending proposals, audit/index state.", and the block that starts with
"# Claude Code adapter"); fix by changing their opening backticks to include a
language identifier: use ```text (or ```bash) for the file-tree/listing block
and ```markdown for the frontmatter and the markdown content block, ensuring
each opening ``` is replaced with the appropriate ```language tag so MD040 is
satisfied.
- Line 823: The expected-output code span contains leading/trailing internal
padding triggering MD038; locate the line containing the text "Expected: ` M
src/vouch/storage.py`" and remove the internal space(s) inside the code span so
the token becomes "M src/vouch/storage.py" (no internal padding), ensuring the
lint rule MD038 is satisfied.
In `@docs/superpowers/specs/2026-05-25-vouch-diff-design.md`:
- Around line 63-73: The fenced code block that begins with the sample diff (the
block containing "diff claim <old> → <new>") is missing a language tag; update
the opening triple-backticks for that block from ``` to ```text so the block is
explicitly tagged as text (i.e., replace the existing opening ``` with ```text
and keep the closing ``` unchanged).
In `@docs/superpowers/specs/2026-05-27-init-templates-design.md`:
- Around line 15-17: The fenced code block containing the CLI usage line "vouch
init [--path P] [--template starter|gittensor]" is missing a language tag;
update that fenced block to include the bash language tag (i.e., change the
opening triple-backticks to "```bash") so the CLI example is properly marked as
shell code for Markdown linting and consistent formatting.
In `@README.md`:
- Line 265: Update the "What ships today" CLI summary row so it exactly matches
the documented CLI surface in the detailed section: add the missing commands
(e.g., migrate, review, expire, sync-check, sync-apply, diff, and any others
present in the detailed CLI list) and ensure the ordering/formatting matches the
rest of the README; edit the table cell that currently lists CLI commands (the
single-line pipe-delimited row) to include these additional commands and remove
or rename any stale entries so both summary and detailed sections are
consistent.
In `@ROADMAP.md`:
- Line 15: Update the ROADMAP line that currently reads `vouch approve --batch`
to the implemented bulk-approve syntax `vouch approve <id> <id> ...` and mention
the optional `--keep-going` flag; replace the old token `vouch approve --batch`
with `vouch approve <id> <id> ... (use --keep-going to continue on errors)` so
the roadmap matches the actual CLI shape.
In `@src/vouch/cli.py`:
- Around line 535-562: The precheck allows duplicate proposal ids to slip
through (e.g., `vouch approve id id`) causing a partial commit; before the
"all-or-nothing" branch that uses check_approvable, detect duplicates in
proposal_ids (when keep_going is False) and reject the command: compute
duplicated ids from proposal_ids, print an error for each duplicate and raise
click.ClickException refusing to approve, so nothing is approved; update the CLI
flow around the existing keep_going/check_approvable block (referencing
proposal_ids, keep_going, check_approvable, do_approve) and add a regression
test that calls the approve command with the same id twice to assert it fails
preemptively.
In `@src/vouch/context.py`:
- Around line 148-154: When building result["explain"] you must apply the same
retracted-claim filter used for items so retracted/archived/superseded claims do
not appear when explain=True; specifically, after retrieving claim via claim =
store.get_claim(hid) (and the existing except ArtifactNotFoundError handling),
check claim.status against _RETRACTED_CLAIM_STATUSES and skip adding that hit to
result["explain"] if it matches. Apply this same check in both places mentioned
(the block around the claim = store.get_claim(hid) at lines shown and the
similar block at 219-223) so hits are filtered consistently before populating
result["explain"].
In `@src/vouch/health.py`:
- Around line 84-114: Add per-file safe loaders for Page and Entity mirroring
_load_claims_for_lint: implement _load_pages_for_lint(store: KBStore) and
_load_entities_for_lint(store: KBStore) that iterate pages/*.yaml and
entities/*.yaml, call Page.model_validate(...) / Entity.model_validate(...),
accumulate valid objects and Finding entries for ValidationError and other
exceptions (use similar finding types/messages as in _load_claims_for_lint).
Then update callers lint(), fsck(), and _safe_counts() to use these new loaders
instead of directly calling KBStore.list_pages() / KBStore.list_entities() so
legacy empty/invalid YAML files are reported as findings rather than raising
ValidationError.
- Around line 101-106: The ValidationError handling in the except block
currently grabs the last line of str(e) which is the docs URL; instead call
e.errors(include_url=False) and extract the first error message
(e.errors(include_url=False)[0]["msg"]) to use as tail, and update the findings
message in the findings.append(Finding(...)) call to end with "fix the YAML or
delete the file" (replacing the "add a citation" hint) so it covers empty
text/name/title failures as well.
In `@src/vouch/sync.py`:
- Around line 295-317: The current loop in sync_apply reads and writes files
interleaved which can leave a partially-applied KB on disk if a later validation
fails; change it to a two-phase commit: first iterate src.files and for each
path use bundle._safe_member_path and _read_source_file to stage (path, dest,
data) entries while checking existing dest conflicts (using on_conflict and
sha256_hex) and running bundle._validate_content to collect any validation
issues, raising or recording conflicts as before, and only if all staged entries
pass validation proceed to a second loop that creates dest.parent, writes
dest.write_bytes(data), and appends to written/skipped_conflicts. Ensure sha256
checks, conflict handling, and val_issues logic are preserved but moved to the
staging phase so no writes occur until all checks succeed.
---
Outside diff comments:
In `@src/vouch/bundle.py`:
- Around line 416-420: During import_apply, re-run the content-address
verification for source blobs instead of only calling _validate_content() which
may miss byte-hash mismatches: update import_apply to compute the expected SHA
from the member.name (the sources/<sha> segment) and verify the actual body
bytes hash equals that SHA (same check performed in import_check), and reject or
skip the member when the computed hash does not match; ensure this verification
occurs in the same block that currently calls _validate_content() and affects
skipped/apply behavior so write-time accepts only content whose bytes hash to
the declared <sha>.
In `@src/vouch/storage.py`:
- Around line 181-197: The read_under_root() code currently tries to use
O_NOFOLLOW but silently no-ops on Windows, leaving a TOCTOU/symlink escape;
modify read_under_root() to fail-closed on platforms that do not support atomic
no-follow: before calling os.open(resolved, flags) check whether getattr(os,
"O_NOFOLLOW", 0) is non-zero (or use hasattr/os.supported flag) and if it is
zero/absent raise a ValueError indicating the platform cannot safely open with
no-follow; keep the existing POSIX path that adds O_NOFOLLOW, opens with
os.open(resolved, flags), fstat-checks with stat.S_ISREG and returns the
contents, and do not attempt a non-atomic post-open path check on platforms
without O_NOFOLLOW (i.e., refuse to operate instead of falling back to unsafe
behavior).
---
Nitpick comments:
In `@docs/PROJECT-INDEX.md`:
- Line 32: Several fenced code blocks use bare triple backticks (```) with no
language tag; update each such block (the ones using ``` in the document) to
include an appropriate language identifier (e.g., ```text, ```bash, ```json) so
markdownlint MD040 is satisfied and syntax highlighting improves—search for
occurrences of the plain ``` fences and replace them with the correct fenced
form based on the block content.
- Line 292: Multiple markdown tables (e.g., the table starting with the header
"| Command | Description |") are not surrounded by blank lines causing MD058
failures; fix by adding a single blank line immediately before and after each
table block so every table is separated from surrounding text/paragraphs (apply
the same change to the other table occurrences flagged in the comment). Ensure
you preserve the exact table content and only add the blank lines to satisfy
markdownlint compliance.
In `@tests/test_retrieval_backend.py`:
- Around line 48-101: Add a regression test that exercises the legacy
retrieval.backends list fallback by writing a config with retrieval.backends:
["embedding"] (or a list containing the legacy value) into the store config,
then call context.build_context_pack(store, query="JWT") with
_force_semantic_hit and assert the returned pack contains items and that
_backends(pack) shows the expected backend (e.g., {"embedding"}) or that any
item["backend"] == "embedding"; reuse helpers used in this file
(_force_semantic_hit, _set_backend style of config mutation,
context.build_context_pack, and _backends) to mirror the existing tests for
coverage of the compatibility branch.
🪄 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: 158e3b0f-1a91-4e52-bb6c-0a7293e8aa60
📒 Files selected for processing (41)
CHANGELOG.mdREADME.mdROADMAP.mddocs/PROJECT-INDEX.mddocs/README.mddocs/gittensor.mddocs/multi-agent.mddocs/review-gate.mddocs/superpowers/plans/2026-05-27-claude-code-adapter.mddocs/superpowers/specs/2026-05-25-vouch-diff-design.mddocs/superpowers/specs/2026-05-27-init-templates-design.mdsrc/vouch/bundle.pysrc/vouch/capabilities.pysrc/vouch/cli.pysrc/vouch/context.pysrc/vouch/diff.pysrc/vouch/health.pysrc/vouch/jsonl_server.pysrc/vouch/logging_config.pysrc/vouch/migrations.pysrc/vouch/models.pysrc/vouch/onboarding.pysrc/vouch/proposals.pysrc/vouch/server.pysrc/vouch/sessions.pysrc/vouch/storage.pysrc/vouch/sync.pytests/test_bundle.pytests/test_cli.pytests/test_cli_output.pytests/test_context.pytests/test_diff.pytests/test_expire.pytests/test_health.pytests/test_logging.pytests/test_migrations.pytests/test_retrieval_backend.pytests/test_sessions.pytests/test_storage.pytests/test_sync.pytests/test_templates.py
|
|
||
| Files created or modified: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add missing language identifiers to fenced code blocks.
Line 20, Line 245, and Line 458 use fenced blocks without language tags, which triggers MD040 and can break markdown lint gates.
Suggested patch
-```
+```text
adapters/claude-code/
README.md ← MODIFY: tiered adoption guide, vouch-kb install
@@
-```
+```markdown
---
description: Show vouch KB health — counts, pending proposals, audit/index state.
---
@@
-```
+```markdown
# Claude Code adapterAlso applies to: 245-245, 458-458
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/superpowers/plans/2026-05-27-claude-code-adapter.md` at line 20, Three
fenced code blocks in the document are missing language tags (the file-tree
block beginning with "adapters/claude-code/ README.md", the frontmatter block
beginning with "--- description: Show vouch KB health — counts, pending
proposals, audit/index state.", and the block that starts with "# Claude Code
adapter"); fix by changing their opening backticks to include a language
identifier: use ```text (or ```bash) for the file-tree/listing block and
```markdown for the frontmatter and the markdown content block, ensuring each
opening ``` is replaced with the appropriate ```language tag so MD040 is
satisfied.
| git status --porcelain src/vouch/storage.py | ||
| ``` | ||
|
|
||
| Expected: ` M src/vouch/storage.py` (the pre-existing edit is back). |
There was a problem hiding this comment.
Fix code-span spacing in expected output line.
Line 823 uses a code span with leading/trailing spaces (MD038). Remove internal padding to satisfy lint.
Suggested patch
-Expected: ` M src/vouch/storage.py` (the pre-existing edit is back).
+Expected: `M src/vouch/storage.py` (the pre-existing edit is back).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Expected: ` M src/vouch/storage.py` (the pre-existing edit is back). | |
| Expected: `M src/vouch/storage.py` (the pre-existing edit is back). |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 823-823: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 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 `@docs/superpowers/plans/2026-05-27-claude-code-adapter.md` at line 823, The
expected-output code span contains leading/trailing internal padding triggering
MD038; locate the line containing the text "Expected: ` M src/vouch/storage.py`"
and remove the internal space(s) inside the code span so the token becomes "M
src/vouch/storage.py" (no internal padding), ensuring the lint rule MD038 is
satisfied.
Comment on lines
+63
to
+73
| ``` | ||
| diff claim <old> → <new> | ||
| status: working → stable | ||
| confidence: 0.7 → 0.9 | ||
| evidence: ['s1'] → ['s1', 's2'] | ||
| text: | ||
| --- a | ||
| +++ b | ||
| -old wording | ||
| +new wording | ||
| ``` |
There was a problem hiding this comment.
Specify the language for the sample output fenced block.
Line 63 starts a fenced block without a language tag (MD040). Tag it as text to keep lint clean.
Suggested patch
-```
+```text
diff claim <old> → <new>
status: working → stable
@@
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 63-63: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/superpowers/specs/2026-05-25-vouch-diff-design.md` around lines 63 - 73,
The fenced code block that begins with the sample diff (the block containing
"diff claim <old> → <new>") is missing a language tag; update the opening
triple-backticks for that block from ``` to ```text so the block is explicitly
tagged as text (i.e., replace the existing opening ``` with ```text and keep the
closing ``` unchanged).
Comment on lines
+15
to
+17
| ``` | ||
| vouch init [--path P] [--template starter|gittensor] | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the CLI usage block.
Line 15 uses an unlabeled fenced block (MD040). Use bash for consistency with other command examples.
Suggested patch
-```
+```bash
vouch init [--path P] [--template starter|gittensor]</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/superpowers/specs/2026-05-27-init-templates-design.md` around lines 15 -
17, The fenced code block containing the CLI usage line "vouch init [--path P]
[--template starter|gittensor]" is missing a language tag; update that fenced
block to include the bash language tag (i.e., change the opening
triple-backticks to "```bash") so the CLI example is properly marked as shell
code for Markdown linting and consistent formatting.
| |------|-----------------| | ||
| | Knowledge base | `.vouch/` folder, YAML claims/entities/relations/evidence/sessions, markdown pages with frontmatter, JSONL audit log, content-addressed sources | | ||
| | CLI | `init`, `discover`, `capabilities`, `status`, `lint`, `doctor`, `pending`, `show`, `approve`, `reject`, `propose-{claim,page,entity,relation}`, `source add`, `source verify`, `supersede`, `contradict`, `archive`, `confirm`, `cite`, `session {start,end}`, `crystallize`, `search`, `context`, `index`, `audit`, `export`, `export-check`, `import-check`, `import-apply`, `serve` | | ||
| | CLI | `init`, `discover`, `capabilities`, `status`, `lint`, `doctor`, `fsck`, `pending`, `show`, `approve`, `reject`, `propose-{claim,page,entity,relation}`, `source add`, `source verify`, `supersede`, `contradict`, `archive`, `confirm`, `cite`, `session {start,end}`, `crystallize`, `search`, `context`, `index`, `audit`, `export`, `export-check`, `import-check`, `import-apply`, `serve` | |
There was a problem hiding this comment.
Update the CLI summary row to match the documented surface.
Line 265’s “What ships today” CLI list is stale versus the detailed CLI section (for example, migrate, review, expire, sync-check, sync-apply, diff are missing). Please sync this row to avoid contradictory docs in the same file.
🤖 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 `@README.md` at line 265, Update the "What ships today" CLI summary row so it
exactly matches the documented CLI surface in the detailed section: add the
missing commands (e.g., migrate, review, expire, sync-check, sync-apply, diff,
and any others present in the detailed CLI list) and ensure the
ordering/formatting matches the rest of the README; edit the table cell that
currently lists CLI commands (the single-line pipe-delimited row) to include
these additional commands and remove or rename any stale entries so both summary
and detailed sections are consistent.
Comment on lines
+535
to
+562
| if not keep_going: | ||
| blocked = [ | ||
| (pid, reason_blocked) | ||
| for pid in proposal_ids | ||
| if (reason_blocked := check_approvable(store, pid, approved_by=approver)) | ||
| ] | ||
| if blocked: | ||
| for pid, why in blocked: | ||
| click.echo(f"✗ {pid}: {why}", err=True) | ||
| raise click.ClickException( | ||
| f"refusing to approve: {len(blocked)} of {len(proposal_ids)} not " | ||
| "approvable — nothing was approved (use --keep-going for best-effort)" | ||
| ) | ||
|
|
||
| failures = 0 | ||
| for pid in proposal_ids: | ||
| try: | ||
| artifact = do_approve(store, pid, approved_by=approver, reason=reason) | ||
| except (ArtifactNotFoundError, ValueError, ProposalError, LifecycleError) as e: | ||
| failures += 1 | ||
| click.echo(f"✗ {pid}: {e}", err=True) | ||
| continue | ||
| click.echo(f"Approved → {type(artifact).__name__.lower()}/{artifact.id}") | ||
|
|
||
| if failures: | ||
| raise click.ClickException( | ||
| f"{failures} of {len(proposal_ids)} proposal(s) failed to approve" | ||
| ) |
There was a problem hiding this comment.
Reject duplicate proposal ids before the “all-or-nothing” batch path runs.
check_approvable() is evaluated against the original pending state for each tuple element, so vouch approve <id> <id> passes the precheck, approves the first copy, then fails on the second with “not pending”. That leaves a partial batch even though this mode is documented as “nothing was approved” on invalid input.
🩹 Suggested guard
+ seen: set[str] = set()
+ dupes = sorted({pid for pid in proposal_ids if pid in seen or seen.add(pid)})
+ if dupes:
+ raise click.ClickException(
+ f"duplicate proposal id(s): {', '.join(dupes)}"
+ )
+
if not keep_going:
blocked = [
(pid, reason_blocked)
for pid in proposal_ids
if (reason_blocked := check_approvable(store, pid, approved_by=approver))Please add a regression for approve id1 id1 as well, since the current batch tests only cover distinct ids.
🤖 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 `@src/vouch/cli.py` around lines 535 - 562, The precheck allows duplicate
proposal ids to slip through (e.g., `vouch approve id id`) causing a partial
commit; before the "all-or-nothing" branch that uses check_approvable, detect
duplicates in proposal_ids (when keep_going is False) and reject the command:
compute duplicated ids from proposal_ids, print an error for each duplicate and
raise click.ClickException refusing to approve, so nothing is approved; update
the CLI flow around the existing keep_going/check_approvable block (referencing
proposal_ids, keep_going, check_approvable, do_approve) and add a regression
test that calls the approve command with the same id twice to assert it fails
preemptively.
Comment on lines
+148
to
+154
| try: | ||
| claim = store.get_claim(hid) | ||
| except ArtifactNotFoundError: | ||
| continue | ||
| if claim.status in _RETRACTED_CLAIM_STATUSES: | ||
| continue | ||
| cites = list(claim.evidence) |
There was a problem hiding this comment.
explain=True still exposes retracted claim hits.
The filter above removes archived/superseded/redacted claims from items, but result["explain"] is built from raw hits, so those same claims still leak back to callers when explain=True.
Suggested fix
hits = _retrieve(store, query, limit)
items: list[ContextItem] = []
+ explain_rows: list[dict[str, Any]] = []
for kind, hid, summary, score, backend in hits:
cites: list[str] = []
if kind == "claim":
# Exclude retracted claims even if the underlying index still
# matches them (the FTS5 row's status column can lag — see `#78`
@@
items.append(
ContextItem(
id=hid, type=cast(ContextItemKind, kind), summary=summary, score=score,
backend=backend, citations=cites,
freshness="unknown",
)
)
+ if explain:
+ explain_rows.append(
+ {"kind": kind, "id": hid, "score": score, "backend": backend}
+ )
@@
result["backend"] = hits[0][4] if hits else "none"
if explain:
- result["explain"] = [
- {"kind": k, "id": i, "score": sc, "backend": hits[0][4] if hits else "none"}
- for k, i, _sn, sc, _be in hits
- ]
+ result["explain"] = explain_rows
return resultAlso applies to: 219-223
🤖 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 `@src/vouch/context.py` around lines 148 - 154, When building result["explain"]
you must apply the same retracted-claim filter used for items so
retracted/archived/superseded claims do not appear when explain=True;
specifically, after retrieving claim via claim = store.get_claim(hid) (and the
existing except ArtifactNotFoundError handling), check claim.status against
_RETRACTED_CLAIM_STATUSES and skip adding that hit to result["explain"] if it
matches. Apply this same check in both places mentioned (the block around the
claim = store.get_claim(hid) at lines shown and the similar block at 219-223) so
hits are filtered consistently before populating result["explain"].
Comment on lines
+84
to
+114
| def _load_claims_for_lint(store: KBStore) -> tuple[list[Claim], list[Finding]]: | ||
| """Iterate `claims/*.yaml` one file at a time so a single invalid | ||
| YAML can't crash the whole lint sweep — surface it as a finding | ||
| and keep going. This is the repair hint for KBs that have legacy | ||
| uncited claims from before the Claim.evidence min-citation | ||
| validator landed (#81): `vouch lint` lists them as | ||
| `invalid_claim` findings so the user can fix or delete the file | ||
| rather than seeing a bare `pydantic.ValidationError` traceback.""" | ||
| valid: list[Claim] = [] | ||
| findings: list[Finding] = [] | ||
| claims = store.list_claims() | ||
| cdir = store.kb_dir / "claims" | ||
| if not cdir.is_dir(): | ||
| return valid, findings | ||
| for p in sorted(cdir.glob("*.yaml")): | ||
| cid = p.stem | ||
| try: | ||
| valid.append(Claim.model_validate(_yaml_load(p.read_text()))) | ||
| except ValidationError as e: | ||
| tail = str(e).splitlines()[-1].strip() if str(e) else "validation failed" | ||
| findings.append(Finding( | ||
| "error", "invalid_claim", | ||
| f"claim {cid} ({p}) fails model validation: {tail} — " | ||
| "edit the YAML to add a citation, or delete the file", | ||
| [cid], | ||
| )) | ||
| except Exception as e: | ||
| findings.append(Finding( | ||
| "error", "unreadable_claim", | ||
| f"claim {cid} ({p}) could not be loaded: {e}", [cid], | ||
| )) | ||
| return valid, findings |
There was a problem hiding this comment.
Extend the safe-load path beyond claims.
This helper only covers Claim, but the same PR also adds required-field validators for Page.title and Entity.name. lint(), fsck(), and _safe_counts() still call store.list_pages() / store.list_entities() directly, so a legacy empty page/entity YAML will still raise and abort the run instead of being surfaced as a finding. Please add the same per-file validation handling for pages and entities before using them in counts or deep checks.
🤖 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 `@src/vouch/health.py` around lines 84 - 114, Add per-file safe loaders for
Page and Entity mirroring _load_claims_for_lint: implement
_load_pages_for_lint(store: KBStore) and _load_entities_for_lint(store: KBStore)
that iterate pages/*.yaml and entities/*.yaml, call Page.model_validate(...) /
Entity.model_validate(...), accumulate valid objects and Finding entries for
ValidationError and other exceptions (use similar finding types/messages as in
_load_claims_for_lint). Then update callers lint(), fsck(), and _safe_counts()
to use these new loaders instead of directly calling KBStore.list_pages() /
KBStore.list_entities() so legacy empty/invalid YAML files are reported as
findings rather than raising ValidationError.
Comment on lines
+101
to
+106
| except ValidationError as e: | ||
| tail = str(e).splitlines()[-1].strip() if str(e) else "validation failed" | ||
| findings.append(Finding( | ||
| "error", "invalid_claim", | ||
| f"claim {cid} ({p}) fails model validation: {tail} — " | ||
| "edit the YAML to add a citation, or delete the file", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Pydantic 2.13.4, what is the recommended way to extract the first human-readable validation message from ValidationError, and does ValidationError.errors(include_url=False) omit the trailing "For further information" URL line?
💡 Result:
In Pydantic 2.13.4, the recommended way to extract human-readable error messages is to access the 'msg' field within the dictionaries returned by the ValidationError.errors method [1][2]. For example, to extract the message of the first error: try: # Your validation code... except ValidationError as e: first_error_msg = e.errors['msg'] print(first_error_msg) Regarding your second question, yes, ValidationError.errors(include_url=False) successfully omits the 'url' field from the dictionaries in the returned list [3][4][5]. However, it is important to note that this method returns structured dictionary data, not a formatted string [1][2]. The trailing "For further information" URL line you see is part of the string representation (str(e)) of the ValidationError object, not the dictionary objects returned by errors [1][3][5]. By using.errors(include_url=False), you are removing the URL from the structured data; if you subsequently need to present this as a clean human-readable message, you must construct that string yourself using the extracted 'msg' (and potentially other fields like 'loc' or 'input') from the error dictionary [3][5].
Citations:
- 1: https://pydantic.dev/docs/validation/2.12/errors/errors
- 2: https://pydantic.dev/docs/validation/latest/errors/errors/
- 3: An option not to display URLs as part of str(ex) pydantic/pydantic#6441
- 4: How to remove "url" & "input" fields from validation error response? pydantic/pydantic#7485
- 5: More concise validation error in API responses pydantic/pydantic#7938
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import sys, textwrap, json, subprocess
# Ensure pydantic 2.13.4 is installed
subprocess.check_call([sys.executable, "-m", "pip", "install", "-q", "pydantic==2.13.4"])
from pydantic import BaseModel, ValidationError
class M(BaseModel):
x: int
try:
M(x="abc")
except ValidationError as e:
s = str(e)
lines = s.splitlines()
print("=== str(e) last 3 lines ===")
for ln in lines[-3:]:
print(ln)
# Check whether the last line is the "For further information" line
last = lines[-1].strip() if lines else ""
print("\nlast_line_contains_for_further_info:", "For further information" in last)
errs = e.errors(include_url=False)
print("\n=== e.errors(include_url=False) ===")
print(json.dumps(errs, indent=2, ensure_ascii=False))
print("\nfirst_error_msg:", errs[0].get("msg"))
print("first_error_has_url:", any("url" in d for d in errs))
PYRepository: vouchdev/vouch
Length of output: 662
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section of src/vouch/health.py
sed -n '1,200p' src/vouch/health.py | nl -ba | sed -n '70,140p'
# Find where this "add a citation" hint is used
rg -n "add a citation|citation, or delete" -S src/vouch/health.py src/vouch
# Look for validators related to empty text/name/title (best-effort search)
rg -n "empty[_ -]?(text|name|title)|name[_ -]?(title|text)|title|text" -S src/vouchRepository: vouchdev/vouch
Length of output: 101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show file with line numbers around the area of interest (avoid relying on nl)
awk 'NR>=70 && NR<=140 {printf "%4d:%s\n", NR, $0}' src/vouch/health.py
echo "---- Search for the existing hint text ----"
rg -n "add a citation|citation, or delete the file" -S src/vouch/health.py
echo "---- Search for the “validation empty text/name/title” related validators ----"
rg -n "empty[_ -]?(text|name|title)|name[_ -]?(title|text)|title|text|ValidationError" -S src/vouch/health.pyRepository: vouchdev/vouch
Length of output: 4493
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the Claim model definition
rg -n "class Claim\\b" src/vouch -S
# Show Claim-related model/validators code (best-effort: inspect a few likely files)
rg -n "model_validator|field_validator|ValidationError|empty.*text|empty.*name|empty.*title|name.*empty|title.*empty|text.*empty" src/vouch -S
# Check where the legacy "min-citation" validator (`#81`) is implemented / referenced
rg -n "min-citation|min_citation|citation validator|`#81`" src/vouch -SRepository: vouchdev/vouch
Length of output: 2667
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant validator implementations in src/vouch/models.py
awk 'NR>=150 && NR<=340 {printf "%4d:%s\n", NR, $0}' src/vouch/models.pyRepository: vouchdev/vouch
Length of output: 8257
Fix lint error reporting to show the actual ValidationError message (not the docs URL)
In src/vouch/health.py, str(e).splitlines()[-1] captures the trailing “For further information …” URL, so lint reports the docs tail instead of the real validation reason. Use e.errors(include_url=False) and surface the first error’s msg (e.g. e.errors(...)[0]["msg"]), and replace the “add a citation” hint with a generic “fix the YAML or delete the file” so it matches empty text/name/title failures too.
🤖 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 `@src/vouch/health.py` around lines 101 - 106, The ValidationError handling in
the except block currently grabs the last line of str(e) which is the docs URL;
instead call e.errors(include_url=False) and extract the first error message
(e.errors(include_url=False)[0]["msg"]) to use as tail, and update the findings
message in the findings.append(Finding(...)) call to end with "fix the YAML or
delete the file" (replacing the "add a citation" hint) so it covers empty
text/name/title failures as well.
Comment on lines
+295
to
+317
| written: list[str] = [] | ||
| skipped_conflicts: list[str] = [] | ||
| for path, incoming in sorted(src.files.items()): | ||
| dest = bundle._safe_member_path(kb_dir, path) | ||
| if dest.exists(): | ||
| local_sha = sha256_hex(dest.read_bytes()) | ||
| if local_sha == incoming.sha256: | ||
| continue | ||
| if on_conflict == "fail": | ||
| raise RuntimeError(f"refusing to sync conflicting path: {path}") | ||
| skipped_conflicts.append(path) | ||
| continue | ||
|
|
||
| data = _read_source_file(src, path) | ||
| if sha256_hex(data) != incoming.sha256: | ||
| raise RuntimeError(f"refusing to sync: hash mismatch at write time: {path}") | ||
| val_issues: list[str] = [] | ||
| bundle._validate_content(path, data, val_issues) | ||
| if val_issues: | ||
| raise RuntimeError(f"refusing to sync: {val_issues[0]}") | ||
| dest.parent.mkdir(parents=True, exist_ok=True) | ||
| dest.write_bytes(data) | ||
| written.append(path) |
There was a problem hiding this comment.
Stage the write set before mutating the KB.
sync_check() validates one snapshot, but this loop writes files while re-reading the source. If a later hash mismatch at write time or schema/content-address failure happens after earlier writes succeeded, sync_apply() raises with a partially imported KB already on disk. Validate every pending write first, then commit in a second pass.
Suggested two-phase write flow
- written: list[str] = []
+ planned_writes: list[tuple[Path, bytes, str]] = []
skipped_conflicts: list[str] = []
for path, incoming in sorted(src.files.items()):
dest = bundle._safe_member_path(kb_dir, path)
if dest.exists():
local_sha = sha256_hex(dest.read_bytes())
@@
data = _read_source_file(src, path)
if sha256_hex(data) != incoming.sha256:
raise RuntimeError(f"refusing to sync: hash mismatch at write time: {path}")
val_issues: list[str] = []
bundle._validate_content(path, data, val_issues)
+ bundle._check_source_content_address(path, data, val_issues)
if val_issues:
raise RuntimeError(f"refusing to sync: {val_issues[0]}")
- dest.parent.mkdir(parents=True, exist_ok=True)
- dest.write_bytes(data)
- written.append(path)
+ planned_writes.append((dest, data, path))
+
+ written: list[str] = []
+ for dest, data, path in planned_writes:
+ dest.parent.mkdir(parents=True, exist_ok=True)
+ dest.write_bytes(data)
+ written.append(path)🤖 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 `@src/vouch/sync.py` around lines 295 - 317, The current loop in sync_apply
reads and writes files interleaved which can leave a partially-applied KB on
disk if a later validation fails; change it to a two-phase commit: first iterate
src.files and for each path use bundle._safe_member_path and _read_source_file
to stage (path, dest, data) entries while checking existing dest conflicts
(using on_conflict and sha256_hex) and running bundle._validate_content to
collect any validation issues, raising or recording conflicts as before, and
only if all staged entries pass validation proceed to a second loop that creates
dest.parent, writes dest.write_bytes(data), and appends to
written/skipped_conflicts. Ensure sha256 checks, conflict handling, and
val_issues logic are preserved but moved to the staging phase so no writes occur
until all checks succeed.
Collaborator
ReviewSummary: The core fix for #155 is correct and well-tested — three What works
Suggestions
Verdictrequest changes — The #155 fix itself is correct, minimal, and well-tested; approve that subset immediately. The blocking issue is scope: 40-file, ~7 500-line diff attached to a single-issue validator fix makes the PR very hard to reason about for correctness on the unrelated features (fsck, migrate, sync, crystallize hardening, etc.) and creates audit-log noise linking unrelated changes to #155. Please either (a) confirm this is a deliberate batch release PR and update the title/body to reflect that, or (b) split the unrelated work into separate PRs so each can be reviewed against its own issue. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Adds three
@field_validators insrc/vouch/models.pythat reject empty / whitespace-only values on the required text fields the rest of the codebase already treats as non-empty:Claim.text(_text_non_empty),Entity.name(_name_non_empty), andPage.title(_title_non_empty). Each raisesValueError("<kind> must be a non-empty, non-whitespace string"). The validators sit alongside the existingClaim._at_least_one_citation(#82) and follow the same classmethod shape — no other model fields, no other modules, no storage-layer change.Fixes #155
Why
Fixes #155. The non-empty contract for these three fields already lived in the
propose_*helpers —proposals.propose_claim(src/vouch/proposals.py:91-92),proposals.propose_entity(src/vouch/proposals.py:175-176),proposals.propose_page(src/vouch/proposals.py:140-141) all raiseProposalError(...)on empty input. But the model layer was silent, so every other write path slipped through:Claim(id="c1", text="", evidence=[src.id]),Entity(id="e1", name="", type=…),Page(id="p1", title="")were all accepted by Pydantic;store.put_claim/put_entity/put_pagechecked evidence-existence and claim-refs but not text content; the YAML/markdown landed on disk.bundle._validate_contentruns<Model>.model_validate(...). Same diagnosis as bug: Claim model has no min-evidence validator — uncited claims land via bundle import, put_claim, update_claim #81 / fix(models): require Claim.evidence to be non-empty at the model layer #82 forClaim.evidence: with no text-content validator on the model, a manifest-consistent bundle whose claim YAML hastext: ""(or page with empty title, entity with empty name) imported cleanly.c.text = ""; store.update_claim(c)round-trips throughClaim.model_validate(...)(the fix(models): require Claim.evidence to be non-empty at the model layer #82 review fix), but that re-validation only runs whatever validators are currently on the model. Mutation persisted because no text validator existed to fire.Same structural shape as the validation-gap fixes the project has already absorbed (#81 / #82 for claim citations, #123 for graph integrity, #125 for source content-addressing, #149 for artifact ids): an invariant articulated in one helper, enforced by zero writers anywhere else. This is the same fix pattern, applied to the three other required text fields. Downstream effects of an empty-text record landing are exactly what you'd expect —
kb.searchreturns hits with empty snippets,kb.contextpacks items withsummary="",_enrich_summaryfalls back to the (also empty) claim text — so retrieval / rendering surfaces degrade silently.What might break
Nothing on-disk, schema, bundle-format, MCP/JSONL surface, or audit-log shape — strictly additive validation. Honest writes are unaffected; only records whose text/name/title is empty or whitespace-only start being rejected at write time.
Claim/Entity/Pageconstruction with emptytext/name/titlenow raisespydantic.ValidationError(previously silent).store.put_<X>(<model>(...))paths inherit the model-layer rejection.bundle.import_check/import_applyandsync.sync_check/sync_applyreject any in-YAML record whose text-field is empty via_validate_content's existing pydantic path — surfaced asschema validation failed: …text must be a non-empty…exactly the way bug: Claim model has no min-evidence validator — uncited claims land via bundle import, put_claim, update_claim #81 / fix(models): require Claim.evidence to be non-empty at the model layer #82 surface uncited claims.invalid_claimfindings viavouch lint(the per-file resilience added in the fix(models): require Claim.evidence to be non-empty at the model layer #82 review) rather than crashing — mirrors that migration path exactly.propose_*helpers keep their explicitProposalErrorraises; they fire first (before the model is even constructed in some cases) and produce friendlier CLI/JSONL error messages than a barepydantic.ValidationErrorat the storage seam.VEP
Not required. No change to the object model fields,
kb.*method surface, on-disk layout, bundle format, or audit-log shape — this only tightens write-time validation to match a contract thepropose_*helpers already document.Tests
make checkpasses locally —ruff check src testsclean,mypy srcreportsSuccess: no issues found in 33 source files, fullpytestgreen on Windows / Python 3.13 except the pre-existingtests/test_jsonl_server.py::test_read_under_root_rejects_symlink_at_resolved_pathwhich needs symlink-create privilege (unrelated; same env-only skip noted in fix(models): require Claim.evidence to be non-empty at the model layer #82 / fix: reject dangling Relation/Page references at every write path #124 / fix: enforce Source content-addressing on bundle/sync import #126).New / changed behaviour has tests — 14 new regressions:
tests/test_storage.py(10):test_claim_model_rejects_empty_text,test_claim_model_rejects_whitespace_only_text,test_put_claim_rejects_empty_text,test_update_claim_rejects_in_place_mutation_to_empty_text,test_entity_model_rejects_empty_name,test_entity_model_rejects_whitespace_only_name,test_put_entity_rejects_empty_name,test_page_model_rejects_empty_title,test_page_model_rejects_whitespace_only_title,test_put_page_rejects_empty_title.tests/test_bundle.py(3):test_import_rejects_claim_with_empty_text,test_import_rejects_entity_with_empty_name,test_import_rejects_page_with_empty_title— each builds a manifest-consistent single-member bundle and assertsimport_check.ok is Falsewithschema validation failed: …non-empty…plusimport_applyraising before any file lands. Shares a small_write_single_member_bundlehelper next to the existing bundle-test helpers.tests/test_health.py(1):test_lint_surfaces_legacy_empty_text_yaml_without_crashing— hand-writes a claim YAML withtext: ""to disk (predates the validator), assertsvouch lintsurfaces it as aninvalid_claimfinding rather than crashing the sweep, and that the rest of the sweep still ran (mirrorstest_lint_surfaces_legacy_uncited_claim_yaml_without_crashingfor the bug: Claim model has no min-evidence validator — uncited claims land via bundle import, put_claim, update_claim #81 / fix(models): require Claim.evidence to be non-empty at the model layer #82 migration).CHANGELOG.mdupdated under## [Unreleased]— add before merge (suggested entry):Summary by CodeRabbit
Release Notes
New Features
fsck(deep consistency checks),migrate(KB format upgrades),expire(pending proposal cleanup),review(guided review queue), andsync-check/sync-apply(KB synchronization).vouch init --templatewith newgittensorstarter pack.vouch approve <id> <id> ...with--keep-goingflag.VOUCH_LOG_FORMAT=jsonand--jsonoutput forlint,search, andpendingcommands.auto,embedding,fts5, orsubstringmode viaconfig.yaml.Bug Fixes
VOUCH_KB_PATHenvironment variable now honored for KB discovery.Documentation