fix(vault): slug_hint, ghost-page guard, dedup proposals, warn on claim stub edits#220
fix(vault): slug_hint, ghost-page guard, dedup proposals, warn on claim stub edits#220Tet-9 wants to merge 2 commits into
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:
✨ 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 |
Tet-9
force-pushed
the
fix/219-vault-to-kb-slug-and-guards
branch
from
226e919 to
d5a7075
Compare
|
@plind-junior , this is ready to merge |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5a7075ed1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| source_ids=list({*edited.sources, source.id}), | ||
| proposed_by=actor, | ||
| tags=list(edited.tags), | ||
| slug_hint=page_id, |
There was a problem hiding this comment.
when a vault edit is proposed now, this slug_hint=page_id makes the page proposal payload id equal an already-existing page. the approve path in src/vouch/proposals.py calls _ensure_no_existing_artifact() before handling ProposalKind.PAGE, so vouch approve/kb.approve returns cannot approve: page <id> already exists for every normal vault edit instead of updating the page. either approval needs an edit/update path or the proposal must not use an existing id until that exists.
Useful? React with 👍 / 👎.
| # proposal is approved files duplicate proposals for the same edit, | ||
| # cluttering the review queue and causing the second approve to fail | ||
| # with "page already exists". | ||
| if _has_pending_page_proposal(store, page_id): |
There was a problem hiding this comment.
when a user makes a second different edit to the same mirrored page while the first proposal is still pending, this condition only checks the page id and then treats the changed file as skipped/unchanged. in the default sync_vault direction (both), the following backward pass rewrites the vault from the current KB and drops that second edit without ever filing a proposal; in forward-only mode it remains an unreported changed file. compare the current bytes/pending payload or preserve the changed file instead of treating all same-page pending proposals as duplicates.
Useful? React with 👍 / 👎.
| "are read-only mirrors — edit the citing page instead", | ||
| rel, | ||
| ) | ||
| result.pages_skipped_unknown_id.append(rel) |
There was a problem hiding this comment.
when a claim stub edit reaches this branch, it is added to pages_skipped_unknown_id; the CLI renders every entry in that bucket as skipped — could not parse page id in sync_cmd, so a non-watch vouch sync reports a misleading parse-id error instead of the read-only-claim-stub guidance. clients consuming VaultSyncResult also cannot distinguish this case, so please surface a dedicated skip reason/result field or special-case claims/ paths before returning.
Useful? React with 👍 / 👎.
That is one nice update you've made on the repo, kudos to you man💪. |
|
To use Codex here, create a Codex account and connect to github. |
Tet-9
force-pushed
the
fix/219-vault-to-kb-slug-and-guards
branch
from
d5a7075 to
5822f58
Compare
|
@plind-junior , this is all done now. P1: Added P2: P3: Claim stub edits now land in a dedicated Added a regression test for P1 confirming vault-edit proposals approve without error. Thus making it all set to merge |
3 participants
Fixes #219
What
vault_to_kbhad four related bugs in the forward sync path:1. Wrong proposal id (slug instead of page id)
propose_pagewas called withoutslug_hint, so the proposal id wasderived from
_slugify(edited.title)instead of the page id fromfrontmatter. A page with id
auth-decision-001and title"Auth Decision"would produce a proposal for a new page
auth-decision, silentlyduplicating the KB entry on approve instead of updating it.
Fix: pass
slug_hint=page_idtopropose_page.2. Ghost page proposals
If a page was deleted from the KB after the last backward sync, the mirror
file still exists in the vault.
vault_to_kbwould file a proposal for thedeleted page, which would fail on approve with an unresolvable artifact id.
Fix: call
store.get_page(page_id)before filing; skip with a warning ifthe page no longer exists.
3. Duplicate proposals on repeated runs
Running
vault_to_kbtwice before the first proposal is approved filed twoproposals for the same page edit. The second approve would then fail with
"page already exists".
Fix: added
_has_pending_page_proposalhelper that checks pending proposalsbefore filing; skips if one already targets the same page id.
4. Silent drop on claim stub edits
kb_to_vaultwrites claim stubs under<vault>/vouch/claims/. If the useredits a stub in Obsidian,
vault_to_kbsilently dropped the edit with nofeedback — the stub path was not in the state file under
pages/so thefile was skipped. Fix: detect claim stub paths in the skip branch and emit
a warning directing the user to edit the citing page instead.
Changes
src/vouch/vault_sync.py: four fixes + new_has_pending_page_proposalhelper function
Checklist
make checkpasses (ruff + mypy + pytest)testbranch