Fix steward review flow and submission search#744
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughBackend: steward review action now runs atomically with row locking, validates submission states, conditionally persists ProjectMilestoneReview, and broadens steward submission content search/ordering. Frontend: SubmissionCard centralizes project rubric UI, enforces gate-failure automatic rejection/template application, threads rubric_review/template_id to review requests, and search parser/UI gained reviewed/points sorting and token-handling fixes. ChangesSteward Submission Review Flow Enhancement
Builder Project Rubric and Gate-Failure Enforcement
Search Parser and Discovery Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 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 `@backend/contributions/tests/test_steward_permissions.py`:
- Around line 455-570: Move the helper _create_accepted_submission and the three
tests test_accepted_submission_search_matches_submitted_and_converted_evidence,
test_accepted_submission_search_matches_normalized_url, and
test_accepted_submissions_can_order_by_points_and_reviewed_time out of
test_steward_permissions.py into a new test module (e.g.
test_steward_submission_search.py); ensure the new file imports the same
fixtures/helpers (self.client, self.steward_user, self.regular_user,
self.contribution_type, timezone) or refactors them to use module-level
fixtures/setup used by other tests, remove the moved symbols from the original
file, and run/adjust any test setup (authentication calls like
self.client.force_authenticate) so the tests run in isolation under the new
module.
In `@backend/stewards/migrations/0010_builder_project_gate_templates.py`:
- Around line 68-74: The reverse migration remove_templates is too strict
because it filters by both label and text; change it to delete templates by
label only to match the original update_or_create lookup: in remove_templates
use ReviewTemplate.objects.filter(label=template['label']).delete() for each
entry in TEMPLATES so manually-edited texts are still removed; target the
remove_templates function and the ReviewTemplate model referenced in the
migration.
In `@frontend/src/lib/searchParser.js`:
- Around line 155-163: The dangling-tag merge currently appends the next token
whenever a bare tag like "tag:" is found, which can incorrectly swallow a
subsequent real filter (e.g., "sort:-reviewed"); update the condition in the
block that uses danglingTagMatch, KNOWN_TAGS, tokens, token and i so it only
merges when the following token is not itself a tag/filter — i.e., add a check
that tokens[i+1] does not match a tag pattern like /^[a-z-]+:/i (or is not a
known tag with a colon) before doing token = `${token}${tokens[i + 1]}` and i +=
1.
🪄 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: 1e657a47-ba44-493f-a0d6-0ce7d415b0b9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
backend/contributions/tests/test_steward_permissions.pybackend/contributions/views.pybackend/stewards/migrations/0010_builder_project_gate_templates.pyfrontend/src/components/StewardSearchBar.sveltefrontend/src/components/SubmissionCard.sveltefrontend/src/lib/rubricReview.jsfrontend/src/lib/searchParser.jsfrontend/src/lib/searchToParams.jsfrontend/src/routes/StewardSubmissions.sveltefrontend/src/tests/SubmissionCard.test.jsfrontend/src/tests/searchParser.test.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tests/test_steward_submission_search.py`:
- Around line 141-167: The test
test_accepted_submissions_can_order_by_points_and_reviewed_time currently uses
two submissions whose points and reviewed_at correlate the same way, so both
ordering checks return the same order; change the fixtures so the submissions
have inverse correlation (e.g., create high_old with points=90 and
reviewed_at=now()-2 days, and low_new with points=20 and reviewed_at=now()-1
day), then update the two assertions to expect ordering-by-points to return
[high_old, low_new] and ordering-by-reviewed_at to return [low_new, high_old],
leaving the existing API calls and response checks intact.
🪄 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: de6650f6-2f53-44ff-a299-bf915ac68691
📒 Files selected for processing (5)
backend/contributions/tests/test_steward_permissions.pybackend/contributions/tests/test_steward_submission_search.pybackend/stewards/migrations/0010_builder_project_gate_templates.pyfrontend/src/lib/searchParser.jsfrontend/src/tests/searchParser.test.js
Comment on lines
+141
to
+167
| def test_accepted_submissions_can_order_by_points_and_reviewed_time(self): | ||
| older_low = self._create_accepted_submission( | ||
| points=20, | ||
| reviewed_at=timezone.now() - timezone.timedelta(days=2), | ||
| title='Older low points', | ||
| ) | ||
| newer_high = self._create_accepted_submission( | ||
| points=90, | ||
| reviewed_at=timezone.now() - timezone.timedelta(days=1), | ||
| title='Newer high points', | ||
| ) | ||
|
|
||
| response = self.client.get('/api/v1/steward-submissions/', { | ||
| 'state': 'accepted', | ||
| 'ordering': '-converted_contribution__frozen_global_points', | ||
| }) | ||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| result_ids = [str(item['id']) for item in response.data['results']] | ||
| self.assertEqual(result_ids[:2], [str(newer_high.id), str(older_low.id)]) | ||
|
|
||
| response = self.client.get('/api/v1/steward-submissions/', { | ||
| 'state': 'accepted', | ||
| 'ordering': '-reviewed_at', | ||
| }) | ||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| result_ids = [str(item['id']) for item in response.data['results']] | ||
| self.assertEqual(result_ids[:2], [str(newer_high.id), str(older_low.id)]) |
There was a problem hiding this comment.
Consider strengthening the ordering test to verify distinct sort behavior.
Currently, newer_high has both higher points (90) and a more recent review time (now - 1 day), while older_low has both lower points (20) and an older review time (now - 2 days). This means both ordering strategies (-converted_contribution__frozen_global_points and -reviewed_at) produce the same result order: [newer_high, older_low].
To verify that each ordering field actually sorts by its respective criterion, consider creating test data where points and time are inversely correlated:
high_old: 90 points, reviewed 2 days agolow_new: 20 points, reviewed 1 day ago
Then:
- Ordering by points →
[high_old, low_new] - Ordering by reviewed_at →
[low_new, high_old]
The current test does verify that both ordering parameters are accepted by the API without errors, which provides basic coverage.
🤖 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/tests/test_steward_submission_search.py` around lines
141 - 167, The test
test_accepted_submissions_can_order_by_points_and_reviewed_time currently uses
two submissions whose points and reviewed_at correlate the same way, so both
ordering checks return the same order; change the fixtures so the submissions
have inverse correlation (e.g., create high_old with points=90 and
reviewed_at=now()-2 days, and low_new with points=20 and reviewed_at=now()-1
day), then update the two assertions to expect ordering-by-points to return
[high_old, low_new] and ordering-by-reviewed_at to return [low_new, high_old],
leaving the existing API calls and response checks intact.
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 `@backend/contributions/models.py`:
- Around line 728-734: Update backend/CLAUDE.md to document the new
ProjectMilestoneReview rubric-flow by describing that ProjectMilestoneReview is
the canonical structured rubric record for a submitted contribution (superseding
the SubmittedContribution's proposed_* queue summary), explain how it can be
produced by the AI Review Agent or by a steward decision, and outline the flow
from builder rubric -> ProjectMilestoneReview creation -> usage by steward
submissions endpoints (include references to the ProjectMilestoneReview model
name and SubmittedContribution proposed_* fields and mention where consumers
should read the record as the source-of-truth); keep the wording concise and add
one example request/response or sequence bullet showing the transition from
proposal to ProjectMilestoneReview to final steward decision.
In `@backend/contributions/views.py`:
- Around line 1967-1991: When a final review requires a project rubric
(uses_project_rubric) but the payload omits rubric_review, remove any existing
ProjectMilestoneReview to avoid returning stale data: after computing
requires_project_rubric and reading
serializer.validated_data.get('rubric_review'), add logic so that if
requires_project_rubric is True and rubric_review is falsy you call
ProjectMilestoneReview.objects.filter(submitted_contribution=submission).delete();
keep the existing update_or_create path when rubric_review is present, and apply
the same delete-on-absent policy to the analogous block that handles the later
final-review branch.
In `@frontend/src/components/SubmissionCard.svelte`:
- Around line 748-750: Add documentation to frontend/CLAUDE.md describing the
rubric review accept/reject flow: explain that SubmissionCard, when
isProjectReview and reviewAction is 'accept' or 'reject', attaches
data.rubric_review populated by buildRubricReviewPayload(rubricState) to the
submission payload, list the involved symbols (SubmissionCard,
buildRubricReviewPayload, rubricState, rubric_review, isProjectReview,
reviewAction) and document the expected payload shape and side effects for
direct accept/reject decisions so maintainers and integrators know how rubric
reviews are submitted and processed.
🪄 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: 8ceb56a5-1f9e-41b0-8e0e-d75c1bdb06de
📒 Files selected for processing (8)
backend/contributions/models.pybackend/contributions/rubric_review.pybackend/contributions/serializers.pybackend/contributions/tests/test_project_milestone_rubric.pybackend/contributions/views.pyfrontend/src/components/SubmissionCard.sveltefrontend/src/routes/StewardSubmissions.sveltefrontend/src/tests/SubmissionCard.test.js
Comment on lines
728
to
734
| class ProjectMilestoneReview(BaseModel): | ||
| """ | ||
| Latest structured Builder Project rubric proposal for a submitted | ||
| contribution. The existing SubmittedContribution proposed_* fields remain | ||
| the queue summary; this record is the detailed rubric source of truth. | ||
| Latest structured Builder Project rubric review for a submitted | ||
| contribution. This can come from a proposal or a final steward decision. | ||
| The existing SubmittedContribution proposed_* fields remain the queue | ||
| summary; this record is the detailed rubric source of truth. | ||
| """ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether backend/CLAUDE.md changed in this PR and mentions ProjectMilestoneReview/rubric flow context.
git diff --name-only origin/dev...HEAD | sed -n '/^backend\/CLAUDE\.md$/p'
git diff origin/dev...HEAD -- backend/CLAUDE.md | rg -n "ProjectMilestoneReview|rubric|builder_project|steward"Repository: genlayer-foundation/points
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Remotes =="
git remote -v || true
echo
echo "== Branches =="
git branch -a || true
echo
echo "== Candidate base refs merge-base with HEAD =="
candidates=(origin/dev origin/main origin/master dev main master HEAD~1)
for base in "${candidates[@]}"; do
if git rev-parse --verify "${base}" >/dev/null 2>&1; then
if mb=$(git merge-base "${base}" HEAD 2>/dev/null); then
echo "OK: ${base} merge-base=${mb}"
# only check diff for backend/CLAUDE.md to keep output small
names=$(git diff --name-only "${base}"...HEAD -- backend/CLAUDE.md || true)
if [[ -n "${names}" ]]; then
echo "CHANGED FILES: ${names}"
echo
echo "== Diff hits for rubric-flow terms in backend/CLAUDE.md =="
git diff "${base}"...HEAD -- backend/CLAUDE.md | rg -n "ProjectMilestoneReview|rubric|builder_project|steward" || true
else
echo "backend/CLAUDE.md not changed vs ${base}"
fi
echo
else
echo "FAIL: ${base} no merge-base"
fi
else
echo "SKIP: ${base} not present"
fi
done
echo "== Current backend/CLAUDE.md contains terms? =="
rg -n "ProjectMilestoneReview|rubric|builder_project|steward" backend/CLAUDE.md || trueRepository: genlayer-foundation/points
Length of output: 1063
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Term search in backend/CLAUDE.md =="
rg -n "ProjectMilestoneReview|rubric|milestone|builder_project|milestone review|steward review" backend/CLAUDE.md || true
echo
echo "== Was backend/CLAUDE.md changed in the latest commit (HEAD^..HEAD)? =="
git diff --name-only HEAD^..HEAD -- backend/CLAUDE.md || true
echo
echo "== If multi-commit, changed in last 5 commits? =="
# show changed file names for each commit (relative to its parent)
for c in $(git rev-list --max-count=5 --reverse HEAD); do
echo "-- $c --"
git diff --name-only "${c}^..${c}" -- backend/CLAUDE.md | sed 's/^/ /'
doneRepository: genlayer-foundation/points
Length of output: 636
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git status =="
git status -sb || true
echo
echo "== HEAD info =="
git rev-parse HEAD || true
echo
echo "== Does HEAD^ exist? =="
if git rev-parse HEAD^ >/dev/null 2>&1; then
echo "HEAD^ exists"
else
echo "HEAD^ missing (shallow/single commit)"
fi
echo
echo "== Files changed in current HEAD commit (if any) =="
git show --name-only --pretty='' HEAD -- backend/CLAUDE.md || true
echo
echo "== backend/CLAUDE.md: context around line ~60-120 =="
sed -n '50,120p' backend/CLAUDE.md || true
echo
echo "== backend/CLAUDE.md: context around line ~240-310 =="
sed -n '230,310p' backend/CLAUDE.md || true
echo
echo "== Confirm term presence/absence in backend/CLAUDE.md =="
rg -n "ProjectMilestoneReview|builder_project|rubric|milestone review|milestone" backend/CLAUDE.md || trueRepository: genlayer-foundation/points
Length of output: 8295
Ensure backend/CLAUDE.md reflects the ProjectMilestoneReview rubric-flow semantics
backend/CLAUDE.md was updated in this change, but it only documents the AI Review Agent and “Steward Submissions” endpoints; it doesn’t mention ProjectMilestoneReview (or the rubric/builder-to-steward flow clarification). Update backend/CLAUDE.md to explicitly describe how ProjectMilestoneReview fits into the submitted contribution/rubric source-of-truth workflow.
🤖 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/models.py` around lines 728 - 734, Update
backend/CLAUDE.md to document the new ProjectMilestoneReview rubric-flow by
describing that ProjectMilestoneReview is the canonical structured rubric record
for a submitted contribution (superseding the SubmittedContribution's proposed_*
queue summary), explain how it can be produced by the AI Review Agent or by a
steward decision, and outline the flow from builder rubric ->
ProjectMilestoneReview creation -> usage by steward submissions endpoints
(include references to the ProjectMilestoneReview model name and
SubmittedContribution proposed_* fields and mention where consumers should read
the record as the source-of-truth); keep the wording concise and add one example
request/response or sequence bullet showing the transition from proposal to
ProjectMilestoneReview to final steward decision.
Source: Coding guidelines
Comment on lines
+748
to
+750
| if (isProjectReview && (reviewAction === 'accept' || reviewAction === 'reject')) { | ||
| data.rubric_review = buildRubricReviewPayload(rubricState); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if frontend/CLAUDE.md mentions rubric review submission
rg -n -C3 'rubric.*review|buildRubricReviewPayload' frontend/CLAUDE.mdRepository: genlayer-foundation/points
Length of output: 52
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check existence =="
ls -la frontend/CLAUDE.md || true
echo "== Search for rubric review / accept-reject flow =="
rg -n -C3 'rubric|review.*submission|buildRubricReviewPayload|accept|reject' frontend/CLAUDE.md || true
echo "== Search for SubmissionCard rubric-related mentions in docs =="
rg -n -C3 'SubmissionCard|rubricState|rubric_review' frontend/CLAUDE.md || trueRepository: genlayer-foundation/points
Length of output: 963
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Case-insensitive search in frontend/CLAUDE.md =="
rg -n -i -C3 'rubric|rubric_review|buildRubricReviewPayload|review.*submission|accept|reject' frontend/CLAUDE.md || trueRepository: genlayer-foundation/points
Length of output: 808
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -i --heading \
'rubric|rubric_review|reviewAction|buildRubricReviewPayload|review.*submission|accept.*reject|accept|reject' \
frontend/CLAUDE.mdRepository: genlayer-foundation/points
Length of output: 289
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -i --heading \
'(rubric|rubric[_ ]review|review[_ ]submission|submission[_ ]review|review[_ -]?action|rubric_review|buildrubricreviewpayload|approve|approval|deny|denied|decline|rejection|decision)' \
frontend/CLAUDE.md || true
rg -n -i --heading \
'(approve|approval|deny|decline|rejection|decision)' \
frontend/CLAUDE.md || true
rg -n -i --heading \
'review' \
frontend/CLAUDE.md || trueRepository: genlayer-foundation/points
Length of output: 802
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -i 'submissioncard|rubricstate|rubric[_ -]?review|buildrubricreviewpayload' frontend/CLAUDE.md || trueRepository: genlayer-foundation/points
Length of output: 52
Update frontend/CLAUDE.md for rubric review accept/reject flow
frontend/CLAUDE.mdhas no mentions ofrubric,rubric_review,buildRubricReviewPayload,SubmissionCard, or other rubric review submission/decision flow terms—document the direct accept/reject behavior for rubric review submissions.
🤖 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/components/SubmissionCard.svelte` around lines 748 - 750, Add
documentation to frontend/CLAUDE.md describing the rubric review accept/reject
flow: explain that SubmissionCard, when isProjectReview and reviewAction is
'accept' or 'reject', attaches data.rubric_review populated by
buildRubricReviewPayload(rubricState) to the submission payload, list the
involved symbols (SubmissionCard, buildRubricReviewPayload, rubricState,
rubric_review, isProjectReview, reviewAction) and document the expected payload
shape and side effects for direct accept/reject decisions so maintainers and
integrators know how rubric reviews are submitted and processed.
Source: Coding guidelines
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.
Prevents duplicate accept reviews by locking submission rows and rejecting stale final reviews while still allowing more-info follow-up submissions to be reviewed. Cleans up steward search parsing/help, adds URL-safe content search across submitted and converted evidence, and supports reviewed/points sorting. Extends builder project rubric use to direct accept/reject evaluation, auto-selects gate failure reject templates, and seeds those templates. Includes the current lockfile workspace-name update; verified with backend steward/project rubric tests, frontend parser/component tests, and the frontend build.
Summary by CodeRabbit
New Features
Bug Fixes
Tests