Skip to content

fix(proposals): validate claim/relation/entity payloads at propose time#509

Open
tryeverything24 wants to merge 1 commit into
vouchdev:testfrom
tryeverything24:fix/confidence-bounds-propose-time
Open

fix(proposals): validate claim/relation/entity payloads at propose time#509
tryeverything24 wants to merge 1 commit into
vouchdev:testfrom
tryeverything24:fix/confidence-bounds-propose-time

Conversation

@tryeverything24

@tryeverything24 tryeverything24 commented Jul 16, 2026

Copy link
Copy Markdown

what changed

propose_claim, propose_relation, and propose_entity built their payload dict and handed it straight to _file_proposal with no check against the Claim/Relation/Entity model's own constraints (confidence: float = Field(ge=0.0, le=1.0), the RelationType/EntityType enums). approve() was the only place that ever caught it, via its own Claim(**payload) construction — too late to give the proposer a usable error, and too late to keep the pending queue clean, since a proposal that fails that check can never be approved and just sits stuck there.

why

Found this by exploring the CLI directly: vouch propose-claim --confidence 1.5 ... files successfully with no error. Approving it later fails with a raw pydantic ValidationError, and the proposal can never be fixed or approved — it's permanently stuck in the pending queue.

fix

Validates with the same Model(**payload) construction _payload_block_reason and approve() already use, so any model-level constraint is caught generically, not just confidence bounds (this also now catches an invalid RelationType/EntityType string at propose time, not just approve). Mirrors the propose-time validation propose_page and propose_relation's existing endpoint/evidence-existence checks already do for other fields in these same functions — same established pattern, closing the one field class that slipped through it.

validation commands run

python -m pytest tests/ -q --ignore=tests/embeddings   # 1431 passed
python -m mypy src                                      # clean
python -m ruff check src tests                           # clean

Confirmed the 4 new regression tests in tests/test_proposals.py fail on the pre-fix code (verified via stash/restore) and pass after.

Summary by CodeRabbit

  • Bug Fixes

    • Invalid claim, relation, and entity proposal data is now rejected immediately.
    • Invalid confidence values and entity or relation types no longer create proposals that remain stuck in the pending queue.
    • Proposal errors now provide clearer messages identifying the invalid payload type.
  • Documentation

    • Updated the unreleased changelog to document immediate proposal validation.

propose_claim, propose_relation, and propose_entity built their payload
dict and handed it straight to _file_proposal with no check against the
Claim/Relation/Entity model's own constraints (confidence Field(ge=0.0,
le=1.0), the RelationType/EntityType enums). approve() was the only
place that ever caught it, via Claim(**payload) etc — too late to give
the proposer a usable error, and too late to keep the pending queue
clean, since a proposal that fails that check can never be approved and
just sits there.

validates with the same Model(**payload) construction _payload_block_reason
and approve() already use, so any model-level constraint is caught, not
just confidence. mirrors the propose-time validation propose_page and
propose_relation's existing endpoint/evidence checks already do for
other fields — same pattern, closing the one field class that slipped
through it.

validation commands run: pytest tests/ -q --ignore=tests/embeddings
(1431 passed), mypy src (clean), ruff check src tests (clean).
@github-actions github-actions Bot added docs documentation, specs, examples, and repo guidance storage kb storage, migrations, schemas, and proposals tests tests and fixtures size: S 50-199 changed non-doc lines labels Jul 16, 2026
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Proposal functions now validate claim, entity, and relation payloads against their models before filing proposals. Invalid confidence values and entity types raise ProposalError immediately, with regression tests confirming no pending proposals are created.

Changes

Proposal validation

Layer / File(s) Summary
Validate proposal payloads
src/vouch/proposals.py
propose_claim, propose_entity, and propose_relation now construct their respective models and convert ValidationError or TypeError into specific ProposalError messages.
Verify rejection behavior
tests/test_proposals.py, CHANGELOG.md
Regression tests cover invalid confidence values and entity types, and the changelog documents proposal-time validation.

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

Possibly related PRs

  • vouchdev/vouch#300: Moves proposal validation to model construction and adds related model validators.

Suggested reviewers: plind-junior

🚥 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 summarizes the main change: validating claim, relation, and entity payloads when proposals are created.
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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/vouch/proposals.py (1)

149-152: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract duplicate validation logic into a helper function.

The payload validation and error conversion block is repeated identically across three proposal functions. Consider extracting this into a shared helper to keep the functions focused and ensure error formatting remains consistent.

  • src/vouch/proposals.py#L149-L152: replace with a call to the helper, e.g., _validate_payload(Claim, payload, "claim").
  • src/vouch/proposals.py#L317-L320: replace with a call to the helper, e.g., _validate_payload(Entity, payload, "entity").
  • src/vouch/proposals.py#L383-L386: replace with a call to the helper, e.g., _validate_payload(Relation, payload, "relation").
♻️ Proposed helper function
from typing import Any

def _validate_payload(model_cls: Any, payload: dict[str, Any], name: str) -> None:
    try:
        model_cls(**payload)
    except (ValidationError, TypeError) as e:
        raise ProposalError(f"invalid {name} payload: {e}") from e
🤖 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/proposals.py` around lines 149 - 152, Extract the repeated Claim,
Entity, and Relation payload validation into a shared _validate_payload helper
in src/vouch/proposals.py, preserving the existing ValidationError/TypeError
conversion to ProposalError and message format. Replace the blocks at
src/vouch/proposals.py lines 149-152, 317-320, and 383-386 with calls using the
corresponding model and name arguments: Claim/"claim", Entity/"entity", and
Relation/"relation".
🤖 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.

Nitpick comments:
In `@src/vouch/proposals.py`:
- Around line 149-152: Extract the repeated Claim, Entity, and Relation payload
validation into a shared _validate_payload helper in src/vouch/proposals.py,
preserving the existing ValidationError/TypeError conversion to ProposalError
and message format. Replace the blocks at src/vouch/proposals.py lines 149-152,
317-320, and 383-386 with calls using the corresponding model and name
arguments: Claim/"claim", Entity/"entity", and Relation/"relation".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ee96314d-6e12-40f0-8478-75dd8c1f101a

📥 Commits

Reviewing files that changed from the base of the PR and between bfb6b0a and b3dd73b.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/vouch/proposals.py
  • tests/test_proposals.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs documentation, specs, examples, and repo guidance 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