Skip to content

fix: resolve dangling refs on vault page updates via update_page#409

Open
RealDiligent wants to merge 1 commit into
vouchdev:mainfrom
RealDiligent:fix/critical-issue-update-page-refs
Open

fix: resolve dangling refs on vault page updates via update_page#409
RealDiligent wants to merge 1 commit into
vouchdev:mainfrom
RealDiligent:fix/critical-issue-update-page-refs

Conversation

@RealDiligent

@RealDiligent RealDiligent commented Jul 7, 2026

Copy link
Copy Markdown

update_page lacked the graph-integrity ref checks that put_page and update_claim already enforce. Vault-edit approvals could land pages citing claims deleted between propose and approve.

What changed

Why

What might break

VEP

Tests

  • make check passes locally (lint + mypy + pytest)
  • New / changed behaviour has a test
  • CHANGELOG.md updated under ## [Unreleased]

Summary by CodeRabbit

  • Bug Fixes
    • Strengthened page update validation so saved changes now reject missing or stale references consistently.
    • Improved approval handling to fail safely when referenced content is removed before an update is finalized.
    • Re-checked page data before saving updates, helping prevent invalid changes from being persisted.

@github-actions github-actions Bot added storage kb storage, migrations, schemas, and proposals tests tests and fixtures size: S 50-199 changed non-doc lines labels Jul 7, 2026
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 868a0ae8-56c8-4242-aefa-ede0c470ed43

📥 Commits

Reviewing files that changed from the base of the PR and between 759d240 and c971870.

📒 Files selected for processing (2)
  • src/vouch/storage.py
  • tests/test_storage.py

📝 Walkthrough

Walkthrough

KBStore introduces a private _validate_page_refs helper that consolidates page graph-reference validation, used in put_page and newly added to update_page, which also re-validates the in-memory Page model. Tests verify rejection of dangling claim references during update and approval flows.

Changes

Page reference validation

Layer / File(s) Summary
Validation helper and put_page wiring
src/vouch/storage.py
Adds _validate_page_refs helper consolidating claim/entity/source existence checks; put_page now calls it instead of inline validation.
update_page revalidation and tests
src/vouch/storage.py, tests/test_storage.py
update_page re-runs Page.model_validate and _validate_page_refs before persisting; tests confirm both direct update and approval-of-proposal paths reject dangling claim references.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant KBStore
    participant Page

    Caller->>KBStore: update_page(page)
    KBStore->>Page: model_validate(page)
    KBStore->>KBStore: _validate_page_refs(page)
    alt Reference invalid
        KBStore-->>Caller: raise ValueError (unknown claim)
    else Reference valid
        KBStore->>KBStore: write page, refresh embeddings
        KBStore-->>Caller: success
    end
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing dangling reference validation during page updates via update_page.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

update_page lacked the graph-integrity ref checks that put_page and
update_claim already enforce. Vault-edit approvals could land pages
citing claims deleted between propose and approve.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: S 50-199 changed non-doc lines storage kb storage, migrations, schemas, and proposals tests tests and fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant