Add permanent lifecycle history for submission reviews - #918
Merged
Conversation
Submission review state was overwritten in place, and re-opening a submission (resubmit, appeal, new evidence) erased who decided it and when. A new append-only state-transition log now records every lifecycle event with its actor: submission, steward review, bulk reject, AI gate reject, submitter edits and cancellations, appeals, review-field clearing on added evidence, and admin edits (including direct changes to review fields without a state change). Each state change and its log row commit atomically, and fixture loads cannot synthesize log rows. Bulk rejections now leave the same per-submission decision note that single rejections do, so they appear in steward note history and in note-based metrics, and the bulk operation runs in a locked transaction instead of racing concurrent reviews. The resubmitted_more_info filter is removed: re-open paths have cleared review timestamps since June, so it could never match current data. The steward search autocomplete now suggests is:ai-reviewed in its place. ## Implementation Notes - backend/contributions/models.py: SubmissionStateTransition model with 9 event types, record() helper, and a raw-guarded post_save signal that logs creation - backend/contributions/migrations/0079_submissionstatetransition.py: schema for the new table with (submission, created_at) and (event, created_at) indexes - backend/contributions/views.py: transition writes in review, edit, cancel, appeal, and add-evidence paths (each atomic with its state change); bulk_reject rewritten with transaction.atomic + select_for_update, per-submission decision notes (data.bulk=true) and bulk_reject transitions; resubmitted_more_info filter removed - backend/contributions/ai_review/views.py: resubmitted_more_info filter removed, unused F import dropped - backend/contributions/management/commands/review_submissions.py: _apply_reject is now atomic and writes a gate_reject transition attributed to the AI steward - backend/contributions/admin.py: read-only SubmissionStateTransition admin; save_model logs an admin transition when state, reviewed_by, or reviewed_at changes - backend/contributions/tests/test_state_transitions.py: 9 tests, one per lifecycle path incl. bulk-reject notes and admin review-field edits - frontend/src/components/StewardSearchBar.svelte, frontend/src/lib/searchParser.js, frontend/src/lib/searchToParams.js, frontend/src/tests/searchParser.test.js: drop the dead resubmitted token, suggest ai-reviewed in autocomplete/help - .claude/skills/ai-review.md: remove the deleted filter from the AI review agent instructions (unknown params are silently ignored) - backend/CLAUDE.md: document the new model and the behavior changes
📝 WalkthroughWalkthroughAdds an append-only ChangesSubmission lifecycle auditing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Steward
participant ReviewAPI
participant SubmittedContribution
participant SubmissionStateTransition
participant SubmissionNote
Steward->>ReviewAPI: review or bulk-reject submissions
ReviewAPI->>SubmittedContribution: persist rejection state
ReviewAPI->>SubmissionStateTransition: record transition events
ReviewAPI->>SubmissionNote: persist bulk decision notes
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 @.claude/skills/ai-review.md:
- Line 192: Correct the “Pending submissions resubmitted after more info was
requested” row in the More-Info Filters table by placing the endpoint retrieval
instructions in the Endpoint column and the `internal_notes` inspection criteria
in the Filter column, leaving no content misaligned or unnecessarily empty.
In `@backend/contributions/views.py`:
- Around line 3163-3168: Update the bulk reject operation in the
SubmittedContribution.objects.filter(...).update call to explicitly set
updated_at=timezone.now(), matching the timestamp behavior of the single-review
path.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5bf0b2f7-8468-4f1a-8c91-c2f11f600888
📒 Files selected for processing (14)
.claude/skills/ai-review.mdCHANGELOG.mdbackend/CLAUDE.mdbackend/contributions/admin.pybackend/contributions/ai_review/views.pybackend/contributions/management/commands/review_submissions.pybackend/contributions/migrations/0079_submissionstatetransition.pybackend/contributions/models.pybackend/contributions/tests/test_state_transitions.pybackend/contributions/views.pyfrontend/src/components/StewardSearchBar.sveltefrontend/src/lib/searchParser.jsfrontend/src/lib/searchToParams.jsfrontend/src/tests/searchParser.test.js
💤 Files with no reviewable changes (1)
- frontend/src/lib/searchToParams.js
…iews Bulk rejection updates rows through a queryset update, which skips the automatic modified-timestamp maintenance that single reviews get, so bulk-rejected submissions kept a stale updated_at. The bulk path now stamps updated_at alongside reviewed_at with the same clock reading. Also realigns a misformatted row in the AI review agent's more-info filter table so endpoint and filter guidance sit in their own columns. ## Implementation Notes - backend/contributions/views.py: bulk_reject update() now sets updated_at=now using one shared timezone.now() value for both timestamps - .claude/skills/ai-review.md: More-Info Filters row for resubmitted submissions puts the endpoint in the Endpoint column and the internal_notes inspection guidance in the Filter column
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 (3)
.claude/skills/ai-review.md (2)
181-183: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClarify the obsolete-filter warning.
An ignored
resubmitted_more_infoparameter does not produce the entire unfiltered dataset:/api/v1/ai-review/still applies its documented default pending/unproposed/non-appealed scope. Change “ENTIRE unfiltered queue” to “the endpoint’s default queue without this filter” to avoid misleading agents.🤖 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 @.claude/skills/ai-review.md around lines 181 - 183, The warning about the removed resubmitted_more_info filter incorrectly says ignored parameters return the “ENTIRE unfiltered queue.” Update the corresponding guidance text in the AI review documentation to state that the endpoint returns its default pending/unproposed/non-appealed queue without this filter, preserving the instruction not to send the obsolete parameter.
179-179: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEnforce the documented integer constraint.
.claude/skills/ai-review.mdsaysmin_accepted_contributionsis a positive integer, butNumberFilteraccepts decimal input and this filter only checksvalue > 0, so1.5will pass. Add integer validation server-side or update the docs to match the actual type.🤖 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 @.claude/skills/ai-review.md at line 179, Ensure the documented positive-integer constraint for min_accepted_contributions matches implementation: update the associated NumberFilter validation to require a positive integer, or revise the documentation if decimals are intentionally supported.backend/contributions/views.py (1)
1238-1259: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winWrap evidence creation and the review reset in one transaction.
Evidence.objects.create(...)runs before thetransaction.atomic()block, so a later failure can persist the new evidence while the review-field reset and transition roll back. Move the create into the same atomic section.🤖 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 `@backend/contributions/views.py` around lines 1238 - 1259, Move Evidence.objects.create(...) into the existing transaction.atomic() block that resets the submission review fields and records SubmissionStateTransition.EVENT_EVIDENCE_ADDED, ensuring evidence creation and all related updates commit or roll back together.
🤖 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 @.claude/skills/ai-review.md:
- Around line 181-183: The warning about the removed resubmitted_more_info
filter incorrectly says ignored parameters return the “ENTIRE unfiltered queue.”
Update the corresponding guidance text in the AI review documentation to state
that the endpoint returns its default pending/unproposed/non-appealed queue
without this filter, preserving the instruction not to send the obsolete
parameter.
- Line 179: Ensure the documented positive-integer constraint for
min_accepted_contributions matches implementation: update the associated
NumberFilter validation to require a positive integer, or revise the
documentation if decimals are intentionally supported.
In `@backend/contributions/views.py`:
- Around line 1238-1259: Move Evidence.objects.create(...) into the existing
transaction.atomic() block that resets the submission review fields and records
SubmissionStateTransition.EVENT_EVIDENCE_ADDED, ensuring evidence creation and
all related updates commit or roll back together.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ea85ca6e-f2bb-4cec-973a-f332fb7e07f8
📒 Files selected for processing (2)
.claude/skills/ai-review.mdbackend/contributions/views.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Submission review state is overwritten in place, and re-opening a submission (resubmit, appeal, new evidence) erases who decided it and when. Bulk rejections additionally left no per-decision record at all. This PR makes the review lifecycle fully auditable without changing the review workflow itself.
SubmissionStateTransitionappend-only log (migration 0079): every lifecycle event is recorded with its actor and from/to states. Covered paths: submission (via a raw-guarded post_save signal), steward review, bulk reject, AI gate reject, submitter edit/resubmit, cancel, appeal, review-field clearing on added evidence, and admin edits (including direct changes toreviewed_by/reviewed_atwithout a state change). Each state change and its log row commit atomically. Read-only in the Django admin.data.action='reject',data.bulk=true), so bulk decisions appear in the steward note timeline and note-based metrics. The operation now runs in a locked transaction (select_for_update), closing a pre-existing race with concurrent single reviews; the response shape is unchanged andrejected_idsnow exactly matches the updated rows.resubmitted_more_infofilter from the steward and AI review filtersets and the steward search grammar: re-open paths have clearedreviewed_atsince 2026-06-22, so the filter could never match current data. The AI review agent instructions are updated accordingly (unknown params are silently ignored, so a stale caller would otherwise receive the entire unfiltered queue).is:ai-reviewedis now suggested in the steward search autocomplete and help panel (the mapping tohas_ai_analysisalready existed).No behavior changes for reviewers beyond the visible bulk-reject notes: states, permissions, points, rewards, proposals, notifications, and response shapes are untouched, and nothing reads the new table yet.
Test plan
contributions/tests/test_state_transitions.py, one per lifecycle path, including bulk-reject notes and admin review-field editsmanage.py checkandmakemigrations --checkcleanSummary by CodeRabbit
New Features
is:ai-reviewed(including related exclusions) and suggests the new filter.has_ai_analysis).Bug Fixes
Documentation
Tests