Add reviewer rewards and AI review history - #911
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds persisted review proposals, reviewer reward scoring and grant logic, AI/steward review wiring, serializer and frontend AI-analysis display, search filtering for AI-reviewed submissions, tests, and exclusion updates for ChangesReviewer Reward and AI Analysis Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Steward
participant StewardSubmissionViewSet
participant ReviewProposal
participant reviewer_rewards
Steward->>StewardSubmissionViewSet: review(action, points)
StewardSubmissionViewSet->>ReviewProposal: load active proposal
StewardSubmissionViewSet->>reviewer_rewards: compute_reviewer_reward(...)
reviewer_rewards-->>StewardSubmissionViewSet: reward points
StewardSubmissionViewSet->>reviewer_rewards: grant_reviewer_reward(...)
reviewer_rewards-->>StewardSubmissionViewSet: reward contribution
StewardSubmissionViewSet->>ReviewProposal: persist decision fields
StewardSubmissionViewSet-->>Steward: CRM note with review_proposal_id
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/lib/hiddenContributions.js (1)
1-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConstant name no longer matches contents.
HIDDEN_WELCOME_CONTRIBUTION_SLUGSnow hides a reviewer-reward slug unrelated to "welcome" contributions. Behavior is correct, but consider renaming (e.g.,HIDDEN_ONBOARDING_CONTRIBUTION_SLUGS) for clarity given it's now reused for a different purpose.🤖 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 `@frontend/src/lib/hiddenContributions.js` around lines 1 - 7, The constant name is now misleading because HIDDEN_WELCOME_CONTRIBUTION_SLUGS includes a reviewer-reward slug that is not “welcome”-related. Rename the Set to a broader, accurate name such as HIDDEN_ONBOARDING_CONTRIBUTION_SLUGS and update any references to the renamed symbol so the purpose matches its contents.backend/contributions/views.py (1)
2996-3021: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winClose out the matching
ReviewProposalrows here
change_typeclears the submission’s proposal fields, andbulk_rejectrejects the submission outright, but neither path marks the activeReviewProposalas decided. If these actions are meant to terminate a proposal round, setdecided_at/final_actionon the corresponding proposal too; otherwise you leavedecided_at IS NULLrows behind and break the proposal lifecycle invariant.🤖 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 2996 - 3021, The type-change path in change_type clears submission proposal fields but leaves the related ReviewProposal undecided, so the proposal lifecycle stays open. Update the logic in change_type to also find the matching ReviewProposal and set its decided_at and final_action when next_type differs from current_type, keeping the proposal state consistent with the submission reset.
🤖 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 `@backend/contributions/models.py`:
- Around line 1024-1025: The ReviewProposal docstring is inaccurate because the
model is not strictly append-only after creation; fields like questioned_by,
questioned_at, question_feedback, decided_by, decided_at, final_action,
final_points, final_sections, reward_points, and reward_contribution are updated
in place by the question_proposal and review flows. Update the docstring on
ReviewProposal to describe it as a proposal snapshot created once and then
enriched with question/decision/reward metadata over time. Keep the wording
aligned with the behavior in views.py and avoid implying immutable append-only
storage.
In `@backend/contributions/reviewer_rewards.py`:
- Around line 62-75: The `GlobalLeaderboardMultiplier` creation in
`reviewer_rewards.py` is vulnerable to a TOCTOU race because `exists()` and
`create()` are separate steps. Replace the manual check inside
`transaction.atomic()` with an atomic `get_or_create` flow on
`GlobalLeaderboardMultiplier.objects` keyed by `contribution_type`, and keep the
default field values (`multiplier_value`, `valid_from`, `description`, `notes`)
in that call so concurrent reward grants cannot create duplicate rows.
In `@backend/contributions/views.py`:
- Around line 2153-2166: The active ReviewProposal lookup is duplicated in both
review and question_proposal, so extract it into a shared helper such as
_active_review_proposal(submission) in views.py. Move the
select_for_update().filter(...).order_by('-created_at', '-id').first() logic
into that helper, preserve the existing proposer_id/decided_at eligibility and
locking semantics, and call the helper from both review and question_proposal
(keeping the review-side guard on submission.proposed_by_id).
- Around line 2321-2391: The reviewer reward logic in the active proposal
decision flow should base messaging and status on the actual result of
grant_reviewer_reward, not only on reward_points. Update the branch in the
proposal finalization path so reward_reason and reviewer_reward_note only say
the reward was matched when reward_contribution is truthy; if
grant_reviewer_reward returns None, treat it as a failed grant and record a
failure/no-reward reason instead. Keep the fields on active_proposal and
reviewer_reward_data consistent with the real contribution outcome in the same
block that sets decided_by, final_action, reward_points, and
reward_contribution.
In `@frontend/src/components/SubmissionCard.svelte`:
- Line 1421: The review form condition in SubmissionCard.svelte duplicates the
open-state check; replace the inline submission.state comparison with the
existing isOpenReviewState derived value so the {:else if showReviewForm ...}
branch uses the shared state helper instead of repeating
pending/more_info_needed logic.
---
Outside diff comments:
In `@backend/contributions/views.py`:
- Around line 2996-3021: The type-change path in change_type clears submission
proposal fields but leaves the related ReviewProposal undecided, so the proposal
lifecycle stays open. Update the logic in change_type to also find the matching
ReviewProposal and set its decided_at and final_action when next_type differs
from current_type, keeping the proposal state consistent with the submission
reset.
In `@frontend/src/lib/hiddenContributions.js`:
- Around line 1-7: The constant name is now misleading because
HIDDEN_WELCOME_CONTRIBUTION_SLUGS includes a reviewer-reward slug that is not
“welcome”-related. Rename the Set to a broader, accurate name such as
HIDDEN_ONBOARDING_CONTRIBUTION_SLUGS and update any references to the renamed
symbol so the purpose matches its contents.
🪄 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: 457496a7-f05e-4678-9075-7fbde2907d81
📒 Files selected for processing (19)
backend/api/metrics_views.pybackend/contributions/ai_review/serializers.pybackend/contributions/ai_review/views.pybackend/contributions/constants.pybackend/contributions/migrations/0077_reviewproposal.pybackend/contributions/migrations/0078_create_project_review_reward.pybackend/contributions/models.pybackend/contributions/reviewer_rewards.pybackend/contributions/serializers.pybackend/contributions/tests/test_reviewer_rewards.pybackend/contributions/views.pybackend/leaderboard/models.pybackend/leaderboard/views.pybackend/tally/settings.pyfrontend/src/components/SubmissionCard.sveltefrontend/src/lib/hiddenContributions.jsfrontend/src/lib/searchParser.jsfrontend/src/lib/searchToParams.jsfrontend/src/routes/StewardSubmissions.svelte
| from leaderboard.models import GlobalLeaderboardMultiplier | ||
|
|
||
| try: | ||
| with transaction.atomic(): | ||
| if not GlobalLeaderboardMultiplier.objects.filter( | ||
| contribution_type=contribution_type, | ||
| ).exists(): | ||
| GlobalLeaderboardMultiplier.objects.create( | ||
| contribution_type=contribution_type, | ||
| multiplier_value=1.0, | ||
| valid_from=timezone.now() - timezone.timedelta(days=30), | ||
| description='Default multiplier for project review rewards', | ||
| notes='Auto-created when steward review rewards are granted', | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
TOCTOU race on GlobalLeaderboardMultiplier creation.
The exists() check followed by create() inside transaction.atomic() is not race-safe: two concurrent reward grants that both find no existing multiplier can each pass the check and both insert, unless there's a DB-level unique constraint on contribution_type. Given Contribution.save() relies on this multiplier existing to compute frozen_global_points/leaderboard weighting correctly, duplicate rows could skew leaderboard multiplier lookups.
🔒️ Proposed fix using `get_or_create`
- if not GlobalLeaderboardMultiplier.objects.filter(
- contribution_type=contribution_type,
- ).exists():
- GlobalLeaderboardMultiplier.objects.create(
- contribution_type=contribution_type,
- multiplier_value=1.0,
- valid_from=timezone.now() - timezone.timedelta(days=30),
- description='Default multiplier for project review rewards',
- notes='Auto-created when steward review rewards are granted',
- )
+ GlobalLeaderboardMultiplier.objects.get_or_create(
+ contribution_type=contribution_type,
+ defaults={
+ 'multiplier_value': 1.0,
+ 'valid_from': timezone.now() - timezone.timedelta(days=30),
+ 'description': 'Default multiplier for project review rewards',
+ 'notes': 'Auto-created when steward review rewards are granted',
+ },
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from leaderboard.models import GlobalLeaderboardMultiplier | |
| try: | |
| with transaction.atomic(): | |
| if not GlobalLeaderboardMultiplier.objects.filter( | |
| contribution_type=contribution_type, | |
| ).exists(): | |
| GlobalLeaderboardMultiplier.objects.create( | |
| contribution_type=contribution_type, | |
| multiplier_value=1.0, | |
| valid_from=timezone.now() - timezone.timedelta(days=30), | |
| description='Default multiplier for project review rewards', | |
| notes='Auto-created when steward review rewards are granted', | |
| ) | |
| from leaderboard.models import GlobalLeaderboardMultiplier | |
| try: | |
| with transaction.atomic(): | |
| GlobalLeaderboardMultiplier.objects.get_or_create( | |
| contribution_type=contribution_type, | |
| defaults={ | |
| 'multiplier_value': 1.0, | |
| 'valid_from': timezone.now() - timezone.timedelta(days=30), | |
| 'description': 'Default multiplier for project review rewards', | |
| 'notes': 'Auto-created when steward review rewards are granted', | |
| }, | |
| ) |
🤖 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/reviewer_rewards.py` around lines 62 - 75, The
`GlobalLeaderboardMultiplier` creation in `reviewer_rewards.py` is vulnerable to
a TOCTOU race because `exists()` and `create()` are separate steps. Replace the
manual check inside `transaction.atomic()` with an atomic `get_or_create` flow
on `GlobalLeaderboardMultiplier.objects` keyed by `contribution_type`, and keep
the default field values (`multiplier_value`, `valid_from`, `description`,
`notes`) in that call so concurrent reward grants cannot create duplicate rows.
Adds durable review proposal snapshots for Builder Project reviews, including persistent AI analysis that remains available to stewards after human re-proposals.
Introduces the project review reward contribution type and computes final-review rewards for eligible human proposals.
Adds steward queue filtering and an expandable AI-analysis section in the review UI.
Summary by CodeRabbit
is:ai-reviewed/not:ai-reviewed.