feat(review): persist immutable review and revision policies#195
feat(review): persist immutable review and revision policies#195Abiorh001 wants to merge 27 commits into
Conversation
|
Warning Review limit reached
Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds canonical immutable ChangesPolicy persistence lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant APIClient
participant GuideService
participant ProjectRepository
participant PostgreSQL
APIClient->>GuideService: create or update draft guide
GuideService->>ProjectRepository: upsert policies with actor context
ProjectRepository->>PostgreSQL: write canonical policy fields and provenance
PostgreSQL->>PostgreSQL: enforce constraints and publication guards
PostgreSQL-->>ProjectRepository: accept or reject policy write
ProjectRepository-->>GuideService: return persisted policy state
GuideService-->>APIClient: return guide and non-archival policy response
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
@.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md:
- Around line 5-22: Update the review evidence so the “Final reviewed code
candidate” field consistently uses the actually reviewed commit SHA
ba9c6249c053696dc9f32bfd750a2b9bb787e4f6, matching “Reviewed code SHA” and the
reviewer run IDs; do not alter unrelated evidence fields.
In @.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md:
- Around line 5-11: Align the active implementation candidate with the trust
bundle by identifying b9501b... as the current reviewed post-repair candidate in
STATUS.md and the corresponding WS-REV-001-03P review-policy persistence chunk;
clearly label 35531df... as historical authorization context if it remains, so
both artifacts bind merge evidence to the same exact SHA.
In `@backend/alembic/versions/0034_review_revision_policy_persistence.py`:
- Around line 165-279: Update _create_write_guards to add statement-level BEFORE
TRUNCATE triggers for both review_policies and revision_policies, alongside the
existing row-level guards. Extend each guard function’s TG_OP handling to
explicitly reject TRUNCATE with the immutability error path. Update downgrade()
to drop both truncate triggers.
🪄 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: 9747dead-4c6c-4474-a989-4035a1292ec3
📒 Files selected for processing (20)
.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03P-review-revision-policy-persistence.md.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md.agent-loop/merge-intents/WS-REV-001-03P.jsonbackend/alembic/versions/0034_review_revision_policy_persistence.pybackend/app/modules/projects/models.pybackend/app/modules/projects/repository.pybackend/app/modules/projects/schemas.pybackend/app/modules/projects/service.pybackend/scripts/api_contract_e2e.pybackend/tests/conftest.pybackend/tests/test_alembic.pybackend/tests/test_artifact_admission.pybackend/tests/test_projects.pybackend/tests/test_tasks.pydocs/architecture_data_model.mddocs/template_project_guide.md
1 participant
PR Trust Bundle: WS-REV-001-03P
Chunk
WS-REV-001-03P- Review And Revision Policy Persistence.Goal
Persist the immutable REV-owned policy facts required by later review routing,
leases, decisions, and human revision work, without implementing those later
lifecycle stages.
Human-approved intent
REV starts only after
allow_review, consumes the existing Submission andartifact set, records complete traceable review/revision history in later
chunks, and never absorbs Project Guide, Task, AUTH, ART, or CON ownership.
What changed and why
Migration 0034 replaces ambiguous legacy policy columns with explicit review
preference/lease, finding-evidence, revision-limit/deadline, and provenance
facts. Legacy values remain private and losslessly downgradeable until one
atomic conversion. Models, schemas, repository/service mapping, active docs,
and focused tests now agree with that database contract.
Design chosen
Separate table-specific triggers make policy rows immutable, serialize every
insert/update with the exact guide row, reject runtime legacy claims, and permit
only atomic legacy-to-canonical conversion. Downgrade locks all three involved
tables and refuses canonical facts or unreconstructible archives before DDL.
Alternatives rejected
Scope control and product behavior
The change is limited to the contract's policy persistence, mapping, docs, and
proof-only compatibility exceptions. Canonical decisions remain exactly
accept,needs_revision, andreject. No review lifecycle transition isactivated by this PR.
Acceptance criteria proof
Direct SQL proves canonical shape, provenance, private archives, immutable
identity/context, update/delete refusal, runtime-insert denial, atomic
conversion, downgrade refusal, and lossless legacy round-trip. Eight real
PostgreSQL races prove both policy tables and both write operations serialize
with guide publication in both lock orders.
Tests/checks run
Focused migration, Project, Task, and artifact tests passed. Alembic has one
head, Ruff passed all changed backend paths, stale wording and Markdown links
passed, 100 agent-gate tests passed, and diff integrity passed. Full backend
tests and coverage are delegated to GitHub Actions as instructed by the user.
Test delta and CI integrity
Tests add constraints, downgrade, deterministic waiter, legacy compatibility,
archive privacy, and immutable snapshot proof. Existing compensation and ART
assertions remain intact. No workflow, package script, skip, xfail, coverage
floor, or assertion was weakened; CI must preserve 78 percent repository-wide
coverage and prove at least 90 percent for the materially changed subsystem.
Reviewer results
Senior engineering, QA/test, security/auth, product/ops, architecture, docs,
reuse/dedup, test delta, CI integrity, and circuit breaker all PASS on code
candidate
35531df254c6b25726d666a5e89eda997b97d792after three repair rounds.External review
Pending publication. GitHub Actions and CodeRabbit must pass on the final pushed
head, and every actionable external comment must be resolved before merge.
Remaining risks and follow-up work
Later queue, lease, Review chain, decision, revision, FinalAcceptance, and CON
composition remain deliberately unimplemented. After merge, loop memory may
name same-initiative successor 03A, but 03A requires a separate explicit signed
start and a fresh current-main dependency check.
Human review focus
Review the trigger invariants, downgrade refusal/reconstruction, eight race
cases, legacy API privacy, strict REV boundary, and preservation of Task and
ART compatibility assertions.
Human merge ownership
Only the user may approve and merge this specific PR. Do not merge with a
pending/failed current-head GitHub or CodeRabbit check or an unresolved
actionable comment. Merge does not start 03A.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes