fix(storage): create evidence/ dir on demand in put_evidence#498
fix(storage): create evidence/ dir on demand in put_evidence#498plind-junior wants to merge 3 commits into
Conversation
receipt-backed capture wrote the source but silently filed 0 claims on any KB created before receipts existed: put_evidence opened evidence/<id>.yaml with mode "x" but never made the evidence/ dir, so the write threw FileNotFoundError and capture_answer's cli wrapper swallowed it (source written, no claims, no error). mkdir(parents=True, exist_ok=True) first, the way put_source already does. fresh `vouch init` created evidence/ so this only bit pre-existing kbs. regression test covers put_evidence on a kb whose evidence/ dir is absent.
|
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:
📝 WalkthroughWalkthrough
ChangesEvidence storage
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
storage
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/test_storage.py`:
- Around line 644-647: Update the evidence-directory setup in the test around
store.put_evidence to ensure shutil.rmtree removes the directory without
ignore_errors=True, or explicitly assert the directory is absent before invoking
put_evidence. Preserve the existing assertion that the evidence YAML file is
recreated successfully.
🪄 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: f52a4320-3da2-40a0-96fe-1fd3ae06f26b
📒 Files selected for processing (2)
src/vouch/storage.pytests/test_storage.py
| shutil.rmtree(store.kb_dir / "evidence", ignore_errors=True) | ||
| ev = store.put_evidence(Evidence(id="e1", source_id=src.id, | ||
| locator="L1-L5", quote="snippet")) | ||
| assert (store.kb_dir / "evidence" / f"{ev.id}.yaml").exists() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not ignore failure to remove the evidence directory.
ignore_errors=True can leave evidence/ present, allowing this regression test to pass without exercising the missing-directory path. Remove the directory without suppressing errors, or assert that it no longer exists before calling put_evidence.
🤖 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_storage.py` around lines 644 - 647, Update the evidence-directory
setup in the test around store.put_evidence to ensure shutil.rmtree removes the
directory without ignore_errors=True, or explicitly assert the directory is
absent before invoking put_evidence. Preserve the existing assertion that the
evidence YAML file is recreated successfully.
capture_answer used to file a pending claim per answer when no auto-approve gate was set, which floods the review queue on every substantive turn once the Stop hook is installed. now it returns `gate-closed` and writes nothing unless review.auto_approve_on_receipt or approver_role: trusted-agent is set — the gate is the on-switch, so `install-mcp` is safe for anyone who has not opted into passive memory. the gated session-rollup path is unaffected.
add a "Passive memory" section: the two-step enable (install-mcp wires the Stop answer-capture + per-prompt recall hooks; set review.auto_approve_on_receipt, or approver_role: trusted-agent, to open the gate), how it behaves (saves per answer not on tab-close, recalled next session), and honest caveats — it captures every substantive answer, recall is injected not enforced (use `vouch synthesize` / `/vouch-ask` for a guaranteed grounded answer), short and duplicate answers are skipped. also correct the install-mcp hook list, which had drifted to three hooks.
github-actions
Bot
added
docs
1 participant
receipt-backed capture wrote the source but silently filed 0 claims on any KB created before receipts existed: put_evidence opened evidence/.yaml with mode "x" but never made the evidence/ dir, so the write threw FileNotFoundError and capture_answer's cli wrapper swallowed it (source written, no claims, no error). mkdir(parents=True, exist_ok=True) first, the way put_source already does. fresh
vouch initcreated evidence/ so this only bit pre-existing kbs.regression test covers put_evidence on a kb whose evidence/ dir is absent.
What changed
Why
What might break
VEP
Tests
make checkpasses locally (lint + mypy + pytest)CHANGELOG.mdupdated under## [Unreleased]Summary by CodeRabbit
Bug Fixes
Tests