fix: catch ArtifactNotFoundError in verify (#30) and validate bundle content on import (#13)

[alpurkan17]

Summary

Two changes in this PR:

1. fix(verify): catch ArtifactNotFoundError (#30)

verify_source() was catching FileNotFoundError but read_source_content() raises ArtifactNotFoundError (a KeyError subclass).

2. Bundle content validation against Pydantic models (#13)

import_apply wrote raw bytes without checking against Pydantic models. Adds per-subdirectory validators (Claim, Source, Entity, Relation, Evidence, Session, Proposal, Page).

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Catch ArtifactNotFoundError in verify_source for missing stored content; add bundle content validators (VALIDATORS, validate_content) and run validation in import_check/import_apply; suppress exception chaining in KBStore put* FileExistsError handlers; add a ContextItem type-ignore.

Changes

Exception Handling Fix

Layer / File(s)	Summary
Fix exception handling for missing stored content src/vouch/verify.py	Import ArtifactNotFoundError and change the exception caught when reading stored source content so missing content yields a VerificationResult with stored_ok=False and external_status="n/a".
Context typing nit src/vouch/context.py	Add inline # type: ignore[arg-type] to the ContextItem(id=hid) constructor call (static typing only).

Bundle Import Validation

Layer / File(s)	Summary
Bundle imports for validation src/vouch/bundle.py	Add imports for bundle model types and the page/object deserializer used by validators.
VALIDATORS mapping src/vouch/bundle.py	Add VALIDATORS dictionary mapping top-level bundle subdirectories to parsing/validation functions (YAML + model_validate, _deserialize_page for pages).
Content validation helper src/vouch/bundle.py	Introduce _validate_content(path, data, issues) to run the appropriate validator and append schema validation failed: ... messages to issues without raising.
Validate during import_check src/vouch/bundle.py	Enhance import_check() to extract manifest-listed members and call _validate_content() so schema issues are returned alongside new/identical/conflict classifications.
Validate during import_apply src/vouch/bundle.py	Call _validate_content() in import_apply() before writing bytes to disk; validation issues are discarded (empty issues list) and do not stop apply.

KBStore handler cleanup

Layer / File(s)	Summary
Suppress exception chaining in put_ handlers* src/vouch/storage.py	Update except FileExistsError handlers in multiple KBStore.put_* methods to raise ValueError(...) from None, suppressing exception chaining consistently.

Sequence Diagram(s)

sequenceDiagram
  participant import_check
  participant TarFile
  participant _validate_content
  participant manifest_issues
  import_check->>TarFile: read manifest-listed member -> bytes
  TarFile->>_validate_content: provide(path, bytes)
  _validate_content-->>manifest_issues: append "schema validation failed: ..." (if errors)
  import_check->>import_check: classify as new/identical/conflict and include issues

Loading

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• vouchdev/vouch#26: Related edits to KBStore put_* FileExistsError handling and collision ValueError logic.

I nibble bytes beneath the patch,
I swap the caught for the right catch.
Bundles hum in YAML tone,
I hop through tests, then bring it home. 🐇

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	bundle.py and storage.py changes (validation and exception chaining) are unrelated to the stated objective of fixing the exception type in verify_source().	Remove changes to bundle.py and storage.py or clearly document their relationship to issue #30 in the PR description.
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	verify.py correctly catches ArtifactNotFoundError instead of FileNotFoundError, implementing the primary requirement from issue #30.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the primary changes: fixing exception handling in verify and adding bundle content validation, matching the multi-faceted nature of the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/vouch/bundle.py (1)

214-220: ⚡ Quick win

Precompute manifest path set once before iterating members.

Line 217 rebuilds the same set on every iteration. Hoisting it out of the loop reduces unnecessary work for larger bundles.

Proposed refactor

         bundle_id = manifest.get("bundle_id", "")
+        manifest_paths = {f["path"] for f in manifest["files"]}
         for f in manifest["files"]:
             dest = kb_dir / f["path"]
             if not dest.exists():
                 new_files.append(f["path"])
@@
         for member in tar.getmembers():
             if member.name == MANIFEST_NAME or not member.isfile():
                 continue
-            if member.name not in {f["path"] for f in manifest["files"]}:
+            if member.name not in manifest_paths:
                 continue
             body = tar.extractfile(member).read()  # type: ignore[union-attr]
             _validate_content(member.name, body, issues)

🤖 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 214 - 220, The loop over tar.getmembers()
repeatedly rebuilds the set {f["path"] for f in manifest["files"]}; hoist that
computation out of the loop by computing a manifest_paths_set (or similarly
named variable) once before iterating, then replace the per-iteration check
member.name not in {f["path"] ...} with member.name not in manifest_paths_set;
keep existing logic around MANIFEST_NAME, member.isfile(),
tar.extractfile(member).read(), and the call to _validate_content(member.name,
body, issues).

🤖 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 `@src/vouch/bundle.py`:
- Around line 31-33: Remove the unused Page import and reformat/sort the import
block to satisfy Ruff: combine and alphabetize imports from .models (include
Claim, Entity, Evidence, Proposal, Relation, Session, Source) and from .storage
(sha256_hex, _deserialize_page), ensuring no unused symbols (drop Page) and
follow standard import grouping/ordering so the linter errors I001/F401 are
resolved.
- Around line 43-51: The VALIDATORS mapping in bundle.py is missing a validator
for the "sources" artifacts so source records aren’t schema-checked; add a
"sources" entry to the VALIDATORS dict following the existing pattern
(decode/yaml load then validate with the Source model) so that "sources" uses
Source.model_validate(yaml.safe_load(data)) (or the equivalent decode step used
for other text artifacts) — update the VALIDATORS dict to include this key
alongside "claims", "entities", "relations", etc., using the same lambda
structure as the other validators.

---

Nitpick comments:
In `@src/vouch/bundle.py`:
- Around line 214-220: The loop over tar.getmembers() repeatedly rebuilds the
set {f["path"] for f in manifest["files"]}; hoist that computation out of the
loop by computing a manifest_paths_set (or similarly named variable) once before
iterating, then replace the per-iteration check member.name not in {f["path"]
...} with member.name not in manifest_paths_set; keep existing logic around
MANIFEST_NAME, member.isfile(), tar.extractfile(member).read(), and the call to
_validate_content(member.name, body, issues).

🪄 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: 2f148455-c2b0-44e1-aa19-f618ce9b8a2e

📥 Commits

Reviewing files that changed from the base of the PR and between ad8cbeb and 27ad8a0.

📒 Files selected for processing (1)

• src/vouch/bundle.py

[plind-junior]

Review

Nice catch on #30 — the exception swap in verify.py is exactly right and the fix is a clean one-liner. A few things I'd want to sort out before merging though.

What's good

• src/vouch/verify.py:28 now catches ArtifactNotFoundError, which is what read_source_content actually raises. Fixes fix(verify): wrong exception type caught — verification crashes on missing content #30.
• The VALIDATORS table in src/vouch/bundle.py:42 is a tidy way to dispatch per-subdirectory schema checks, and import_check correctly bubbles issues up through ImportCheckResult.issues.
• The raise ... from None cleanups in storage.py make the "already exists" errors much friendlier.

Needs changes

• src/vouch/bundle.py:270 — the validation in import_apply doesn't actually block bad writes. The call is _validate_content(member.name, body, []) — a throwaway list. So if a bundle has a malformed claim, validation runs, the issue gets dropped on the floor, and the file is written anyway. That's the exact behaviour Bundle import writes unvalidated content to committed artifact directories #13 is trying to prevent. Something like:

  issues: list[str] = []
  _validate_content(member.name, body, issues)
  if issues:
      skipped.append(member.name)
      continue
  dest.write_bytes(body)

[plind-junior]

The description undersells what's in here. The diff actually does two unrelated things:

1. src/vouch/verify.py:30 — the one-line FileNotFoundError → ArtifactNotFoundError fix for fix(verify): wrong exception type caught — verification crashes on missing content #30. Matches what the title promises.
2. src/vouch/bundle.py:42-280 — a substantially new feature: schema validation of every file in a bundle against the Pydantic models, both in import_check (collected into issues) and in import_apply (skips invalid members). Per commit 27ad8a0 this is meant to close Bundle import writes unvalidated content to committed artifact directories #13, but the PR body doesn't mention Bundle import writes unvalidated content to committed artifact directories #13 at all, and bundle.py isn't part of verify.

Could you update the description (and ideally the title) to call out both?

[alpurkan17]

Updated the title and description to reflect both changes (#30 and #13). Thanks for the review!