fix(vault-sync): mirror post-approve WORKING/DRAFT artifacts (#583) - #586
Conversation
|
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:
Walkthrough
ChangesApproved artifact vault mirroring
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant sync_vault
participant kb_to_vault
participant KBStore
participant Vault
sync_vault->>kb_to_vault: run backward synchronization
kb_to_vault->>KBStore: collect live WORKING claims and DRAFT pages
kb_to_vault->>Vault: write live page and claim mirrors
kb_to_vault->>Vault: remove stale tracked mirrors
kb_to_vault-->>sync_vault: return mirrored and removed ids
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
04fb8f6 to
04b1fcb
Compare
|
rewrote commit history to remove the |
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 `@src/vouch/proposals.py`:
- Around line 786-788: Lowercase ordinary prose in the comments at
src/vouch/proposals.py lines 786-788 and 816-817, preserving enum identifiers
such as WORKING, DRAFT, ACTIONABLE, and ACTIVE. At src/vouch/vault_sync.py lines
162-165, lowercase “Gate-approved” and replace “Obsidian” with lowercase prose
such as “vault”; make no code changes.
In `@src/vouch/vault_sync.py`:
- Around line 162-165: Update kb_to_vault() to remove existing vault mirror
files whose artifacts are absent from the current live set, including retracted
claims and archived pages, while preserving current writes for live artifacts.
Add a sync-after-retraction regression test covering removal of the stale
markdown file, and revise the nearby lifecycle comment so it does not imply
deletion already occurs.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 861d0ed0-5815-4aac-967a-b8241765a65c
📒 Files selected for processing (4)
CHANGELOG.mdsrc/vouch/proposals.pysrc/vouch/vault_sync.pytests/test_vault_sync.py
address coderabbit on vouchdev#586: lowercase comment prose, exclude archived pages from the live vault set, and remove leftover claim/page markdown when artifacts leave that set.
|
addressed coderabbit:
ready for re-review. |
address coderabbit on vouchdev#586: lowercase comment prose, exclude archived pages from the live vault set, and remove leftover claim/page markdown when artifacts leave that set.
1b8fc91 to
9dfad06
Compare
address coderabbit on vouchdev#586: lowercase comment prose, exclude archived pages from the live vault set, and remove leftover claim/page markdown when artifacts leave that set.
a2cc000 to
acd7b96
Compare
…v#583) kb_to_vault skipped durable WORKING claims and DRAFT pages (the normal propose+approve defaults), so Obsidian looked empty until statuses were hand-edited. include those statuses in the live vault set, exclude ARCHIVED/retracted, and delete stale mirrors. vault-sync only - avoids core proposals.py so trust-gate can pass. Fixes vouchdev#583
acd7b96 to
ce9a1d8
Compare
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_vault_sync.py`:
- Around line 154-157: Complete the VaultSyncResult partition assertions in
tests/test_vault_sync.py: at lines 154-157, assert that "obsidian-status" is in
result.claims_mirrored; at lines 178-182, assert that "alpha-claim" is absent
from result.claims_mirrored and "alpha-page" is absent from
result.pages_mirrored.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7fc15ff8-2516-441c-af3c-53c3f43793c2
📒 Files selected for processing (3)
CHANGELOG.mdsrc/vouch/vault_sync.pytests/test_vault_sync.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/vouch/vault_sync.py
address coderabbit on vouchdev#586: assert claims_mirrored for the approve path and assert retracted artifacts are absent from mirrored partitions. also lowercase vault_sync comment prose.
|
addressed coderabbit:
ready for re-review. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/vouch/vault_sync.py (1)
269-280: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick windo not delete untracked markdown files
this loop deletes every
*.mdfile whose stem is outside the live-id set, including files that were never created by the mirror. however,vault_to_kb()explicitly treats files absent from sync state as arbitrary user files and skips them (src/vouch/vault_sync.pylines 335-344). a user-createdpages/*.mdorclaims/*.mdfile can therefore be silently deleted on the next backward sync. restrict deletion to files known to be mirror-owned, such as paths recorded in sync state, and preserve untracked files.🤖 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/vault_sync.py` around lines 269 - 280, Update the mirror cleanup loops in the backward-sync flow to delete only markdown paths recorded as mirror-owned in sync state, rather than every file whose stem is absent from the live ID sets. Preserve untracked user files in both mirror and claims_out, while continuing to append removed owned page and claim IDs to the sync result.
🤖 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.
Outside diff comments:
In `@src/vouch/vault_sync.py`:
- Around line 269-280: Update the mirror cleanup loops in the backward-sync flow
to delete only markdown paths recorded as mirror-owned in sync state, rather
than every file whose stem is absent from the live ID sets. Preserve untracked
user files in both mirror and claims_out, while continuing to append removed
owned page and claim IDs to the sync result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ba767a24-a675-48d7-b62d-f3a98f70196f
📒 Files selected for processing (2)
src/vouch/vault_sync.pytests/test_vault_sync.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_vault_sync.py
address coderabbit on vouchdev#586: kb_to_vault cleanup must not unlink untracked user markdown under vouch/pages or vouch/claims. delete only paths recorded in sync state that left the live set, and keep those user files out of the refreshed state.
|
addressed coderabbit follow-up:
ready for re-review. |
Summary
kb_to_vaultskipped durableWORKINGclaims andDRAFTpages (normal propose+approve defaults), so Obsidian looked empty (fix(vault-sync): kb_to_vault skips default post-approve WORKING/DRAFT statuses #583).ARCHIVEDout; delete stale mirrors.proposals.py) so trust-gate / code-owner core review is not required.Test plan
pytest tests/test_vault_sync.pyFixes #583
Summary by CodeRabbit
WORKINGclaims andDRAFTpages.ARCHIVEDpages are no longer mirrored, and retracted claims remain excluded.