Conversation
add exclusion_status enum 'ExclusionStatuses' to LeaderboardEntry and MedalExclusionRecord new settings allow for fine tuned display rules set up to depricate excluded and show_when_excluded fields
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughMigrate leaderboard exclusion from two booleans ( Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
front_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/table_row.tsx (1)
42-80:⚠️ Potential issue | 🟠 MajorShow rank for
EXCLUDE_PRIZE_AND_SHOWentries.Status (1) is supposed to take rank, but the current condition hides the rank for any non-INCLUDE entry, so prize-excluded entries lose their rank display.
🐛 Suggested fix
const t = useTranslations(); + const hidesRank = + exclusion_status === ExclusionStatuses.EXCLUDE_AND_SHOW || + exclusion_status === ExclusionStatuses.EXCLUDE_AND_SHOW_IN_ADVANCED || + exclusion_status === ExclusionStatuses.EXCLUDE; const highlight = user?.id === userId || exclusion_status !== ExclusionStatuses.INCLUDE; @@ - {exclusion_status !== ExclusionStatuses.INCLUDE ? ( + {hidesRank ? ( <> <ExcludedEntryTooltip /> </> ) : (front_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/table_row.tsx (1)
70-96:⚠️ Potential issue | 🟠 MajorShow rank for
EXCLUDE_PRIZE_AND_SHOWentries.Status (1) should still take rank, but the current condition hides the rank for any non-INCLUDE entry.
🐛 Suggested fix
const t = useTranslations(); + const hidesRank = + exclusion_status === ExclusionStatuses.EXCLUDE_AND_SHOW || + exclusion_status === ExclusionStatuses.EXCLUDE_AND_SHOW_IN_ADVANCED || + exclusion_status === ExclusionStatuses.EXCLUDE; @@ - {exclusion_status != ExclusionStatuses.INCLUDE ? ( + {hidesRank ? ( <> <ExcludedEntryTooltip /> </> ) : (scoring/views.py (1)
250-255:⚠️ Potential issue | 🟡 MinorTotal entries should align with rank‑eligible statuses.
total_entriescurrently counts entries up toEXCLUDE_AND_SHOW_IN_ADVANCED, which includes rows excluded from ranks. This can inflate totals. Consider counting only rank‑eligible statuses.🔧 Suggested fix
- exclusion_status__lte=ExclusionStatuses.EXCLUDE_AND_SHOW_IN_ADVANCED, + exclusion_status__lte=ExclusionStatuses.EXCLUDE_PRIZE_AND_SHOW,scoring/utils.py (1)
860-875:⚠️ Potential issue | 🟡 MinorNormalize
exclusion_statuswhen importing CSV.
row.getreturns strings/empties; passing these directly to an IntegerField can raise or store unexpected values. Consider converting to int with a safe default.🔧 Suggested fix
- exclusion_status = row.get("exclusion_status") + exclusion_status = row.get("exclusion_status") + exclusion_status = ( + int(exclusion_status) + if exclusion_status not in (None, "") + else ExclusionStatuses.INCLUDE + ) ... - exclusion_status=exclusion_status, + exclusion_status=exclusion_status,scoring/management/commands/update_global_bot_leaderboard.py (1)
760-768:⚠️ Potential issue | 🔴 CriticalCommand must update deprecated
excludedfield to maintain consistency withexclusion_status.The command only sets
exclusion_statusbut leavesexcludedandshow_when_excludedat their default values (both False). At runtime,scoring/utils.py:667filters entries byexcluded=Falseto calculate leaderboard statistics. Entries markedexclusion_status=EXCLUDEwill still haveexcluded=False, causing them to be incorrectly included in stats calculations. Either update both the deprecated fields here to match theexclusion_statuslogic, or refactor the statistics query to checkexclusion_statusinstead of the deprecated fields.
🤖 Fix all issues with AI agents
In `@scoring/views.py`:
- Around line 71-78: The rank threshold calculation uses
entries.filter(exclusion_status=ExclusionStatuses.INCLUDE).count(), which omits
entries like ExclusionStatuses.EXCLUDE_PRIZE_AND_SHOW that still consume ranks;
update the count to include all rank-eligible statuses (e.g., replace the
.filter(...) call with a filter that includes both ExclusionStatuses.INCLUDE and
ExclusionStatuses.EXCLUDE_PRIZE_AND_SHOW or the inverse by excluding only
non-rank-eligible statuses), so the max(... * 0.05) uses the correct total;
adjust the expression around rank__lte=max(...) accordingly.
🧹 Nitpick comments (2)
scoring/models.py (1)
345-355: Consider indexingexclusion_statusto replace the oldexcludedindex.If queries now filter on
exclusion_status, the previousdb_indexonexcludedwon’t help and could introduce a perf regression. Adding an index (and migration) keeps filtering fast.♻️ Suggested adjustment
- exclusion_status: ExclusionStatuses = models.IntegerField( - max_length=200, + exclusion_status: ExclusionStatuses = models.IntegerField( + db_index=True, + max_length=200, choices=ExclusionStatuses.choices, default=ExclusionStatuses.INCLUDE, help_text="""This sets the exclusion status of this entry.front_end/src/types/scoring.ts (1)
52-68: PreferExclusionStatusesforexclusion_statustype safety.Now that the enum exists, using it directly prevents invalid values and keeps the type aligned with backend semantics.
♻️ Suggested refactor
- exclusion_status: number; + exclusion_status: ExclusionStatuses;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
front_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/index.tsxfront_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/table_row.tsxfront_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsxfront_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/table_row.tsxfront_end/src/types/scoring.tsscoring/admin.pyscoring/management/commands/update_global_bot_leaderboard.pyscoring/migrations/0020_leaderboardentry_and_medalexclusionrecord_exclusion_status.pyscoring/models.pyscoring/serializers.pyscoring/utils.pyscoring/views.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-20T16:49:51.584Z
Learnt from: aseckin
Repo: Metaculus/metaculus PR: 4001
File: front_end/src/app/(futureeval)/futureeval/components/futureeval-leaderboard-table.tsx:15-22
Timestamp: 2026-01-20T16:49:51.584Z
Learning: The mockTranslate function in front_end/src/app/(futureeval)/futureeval/components/futureeval-leaderboard-table.tsx is an intentional workaround to satisfy the entryLabel function signature from shared AIB utils while keeping FutureEval translation-free.
Applied to files:
front_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/table_row.tsxfront_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/index.tsxfront_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsxfront_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/table_row.tsx
📚 Learning: 2026-01-15T19:29:58.940Z
Learnt from: hlbmtc
Repo: Metaculus/metaculus PR: 4075
File: authentication/urls.py:24-26
Timestamp: 2026-01-15T19:29:58.940Z
Learning: In this codebase, DRF is configured to use IsAuthenticated as the default in REST_FRAMEWORK['DEFAULT_PERMISSION_CLASSES'] within metaculus_web/settings.py. Therefore, explicit permission_classes([IsAuthenticated]) decorators are unnecessary on DRF views unless a view needs to override the default. When reviewing Python files, verify that views relying on the default are not redundantly decorated, and flag cases where permissions are being over-specified or when a non-default permission is explicitly required.
Applied to files:
scoring/models.pyscoring/admin.pyscoring/serializers.pyscoring/migrations/0020_leaderboardentry_and_medalexclusionrecord_exclusion_status.pyscoring/management/commands/update_global_bot_leaderboard.pyscoring/utils.pyscoring/views.py
🧬 Code graph analysis (10)
front_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/table_row.tsx (1)
scoring/models.py (1)
ExclusionStatuses(17-22)
front_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/index.tsx (1)
scoring/models.py (1)
ExclusionStatuses(17-22)
scoring/models.py (1)
projects/permissions.py (1)
choices(10-15)
front_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsx (1)
scoring/models.py (1)
ExclusionStatuses(17-22)
scoring/migrations/0020_leaderboardentry_and_medalexclusionrecord_exclusion_status.py (1)
scoring/models.py (2)
LeaderboardEntry(314-380)MedalExclusionRecord(410-495)
scoring/management/commands/update_global_bot_leaderboard.py (2)
scoring/models.py (3)
Leaderboard(105-292)LeaderboardEntry(314-380)ExclusionStatuses(17-22)front_end/src/types/scoring.ts (1)
LeaderboardEntry(60-75)
front_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/table_row.tsx (1)
scoring/models.py (1)
ExclusionStatuses(17-22)
scoring/utils.py (2)
scoring/models.py (1)
ExclusionStatuses(17-22)projects/models.py (1)
BotLeaderboardStatus(248-252)
front_end/src/types/scoring.ts (1)
scoring/models.py (2)
ExclusionStatuses(17-22)LeaderboardEntry(314-380)
scoring/views.py (2)
scoring/models.py (3)
Leaderboard(105-292)LeaderboardEntry(314-380)ExclusionStatuses(17-22)front_end/src/types/scoring.ts (1)
LeaderboardEntry(60-75)
🪛 Ruff (0.14.14)
scoring/admin.py
[warning] 156-156: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
scoring/migrations/0020_leaderboardentry_and_medalexclusionrecord_exclusion_status.py
[warning] 7-7: Unused function argument: schema_editor
(ARG001)
[warning] 27-27: Unused function argument: schema_editor
(ARG001)
[warning] 56-58: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
[warning] 60-104: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build Docker Image / Build Docker Image
🔇 Additional comments (11)
scoring/models.py (3)
17-22: ExclusionStatuses enum looks good.Clear mapping and explicit values make the new status system easy to reason about.
193-196: No review notes for this TODO block.
450-460: LGTM:exclusion_statusadded to MedalExclusionRecord.This aligns with the shared enum and keeps defaults explicit.
scoring/admin.py (1)
156-156: LGTM: admin list_display now surfacesexclusion_status.scoring/serializers.py (1)
15-40: LGTM: serializer now exposesexclusion_statusalongside legacy fields.front_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsx (1)
107-123: LGTM: status-based filtering matches the new enum semantics.front_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/index.tsx (1)
92-103: LGTM: exclusion-status gating is clear and consistent.scoring/views.py (1)
84-87: Advanced-only entries may be exposed to standard users.
EXCLUDE_AND_SHOW_IN_ADVANCEDis included for all non‑staff. If this endpoint backs the standard leaderboard view, advanced‑only entries will leak. Please confirm the intended audience and gate this filter on an explicit “advanced” flag if needed.scoring/migrations/0020_leaderboardentry_and_medalexclusionrecord_exclusion_status.py (1)
7-24: Migration + backfill mapping looks consistent.Forward and reverse mappings align with the deprecated boolean semantics and provide a clear downgrade path.
Also applies to: 27-51, 60-103
scoring/utils.py (2)
43-44: Exclusion status propagation through ranks/prizes looks consistent.Rank gating, prize pool participation, and prize assignment align with the new enum semantics.
Also applies to: 468-528, 741-742
545-553: Confirm medal eligibility forEXCLUDE_PRIZE_AND_SHOW.Medals are currently limited to
INCLUDE. IfEXCLUDE_PRIZE_AND_SHOWentries should still be medal‑eligible (since they take rank), this will skip them.🔧 Possible adjustment (if medal‑eligible)
- entry_count = len( - [e for e in entries if e.exclusion_status == ExclusionStatuses.INCLUDE] - ) + entry_count = len( + [ + e + for e in entries + if e.exclusion_status <= ExclusionStatuses.EXCLUDE_PRIZE_AND_SHOW + ] + ) ... - if entry.exclusion_status != ExclusionStatuses.INCLUDE: + if entry.exclusion_status > ExclusionStatuses.EXCLUDE_PRIZE_AND_SHOW: continue
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
front_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/index.tsx
Outdated
Show resolved
Hide resolved
…ntry-exclusion-subtlety
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@front_end/src/app/`(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsx:
- Around line 152-171: The map callback in leaderboardEntries.map currently
re-checks exclusion_status and returns nothing when the condition fails,
producing undefined slots; remove the inner conditional entirely and always
return the TableRow for each mapped entry (keep the same props: key using
entry.user?.id ?? entry.aggregation_method, rowEntry={entry}, userId,
maxCoverage, withPrizePool, isAdvanced). Rely on the earlier filteredEntries
logic (the filtering by ExclusionStatuses done where filteredEntries is
computed) so you only map over already-filtered entries and avoid implicit
undefineds from the map.
In `@scoring/management/commands/update_global_bot_leaderboard.py`:
- Around line 936-940: The code in update_global_bot_leaderboard.py currently
does direct dict access user.metadata["bot_details"] which can raise KeyError or
TypeError if metadata is None or missing; change the check in the block that
handles uid ints to safely fetch bot_details via
user.metadata.get("bot_details") (or default to {}), then check
bot_details.get("display_in_leaderboard") and set excluded accordingly; ensure
you also guard for user.metadata being None before calling .get and prefer a
safe local variable (e.g., bot_details = (user.metadata or
{}).get("bot_details", {})) so the logic around excluded remains correct.
🧹 Nitpick comments (2)
front_end/src/types/scoring.ts (1)
69-69: Use theExclusionStatusesenum type instead ofnumber.The
ExclusionStatusesenum is defined in this file butexclusion_statusis typed asnumber, losing the type safety the enum provides. Comparisons in downstream components (e.g.,entry.exclusion_status <= ExclusionStatuses.EXCLUDE_AND_SHOW) would benefit from the stricter typing.Proposed fix
- exclusion_status: number; + exclusion_status: ExclusionStatuses;front_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsx (1)
59-63: Use strict equality (===) instead of==.Lines 60 and 62: the comparison with
ExclusionStatusesvalues should use===for idiomatic TypeScript.Proposed fix
(entry) => - entry.exclusion_status <= ExclusionStatuses.EXCLUDE_AND_SHOW || + entry.exclusion_status <= ExclusionStatuses.EXCLUDE_AND_SHOW || (isAdvanced && - entry.exclusion_status == + entry.exclusion_status === ExclusionStatuses.EXCLUDE_AND_SHOW_IN_ADVANCED)
...end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scoring/models.py`:
- Around line 210-213: Update the TODO comment that currently misspells
"depricate" — change it to "deprecate" in the comment near the
bot_exclusion_status and human_exclusion_status references; ensure the sentence
mentioning ExclusionStatuses, bot_status, Project, and leaderboard keeps the
same meaning but uses the correct spelling "deprecate".
🧹 Nitpick comments (4)
front_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/index.tsx (1)
92-102: Use strict equality (===) instead of loose equality (==).Lines 96-98 use
==for comparingexclusionStatuswith enum values. TypeScript best practice is to use===to avoid type coercion surprises—especially since the backend sends these as integers and there's a risk of string-vs-number coercion if the value ever arrives unsanitized.♻️ Proposed fix
const exclusionStatus = entry.exclusion_status; if ( - exclusionStatus == ExclusionStatuses.EXCLUDE || - (exclusionStatus == - ExclusionStatuses.EXCLUDE_AND_SHOW_IN_ADVANCED && + exclusionStatus === ExclusionStatuses.EXCLUDE || + (exclusionStatus === + ExclusionStatuses.EXCLUDE_AND_SHOW_IN_ADVANCED && !currentUser?.is_staff) ) {front_end/src/app/(main)/(leaderboards)/leaderboard/components/leaderboard_table/table_row.tsx (1)
70-72: Reliance on enum ordering for>comparisons is fragile.The
exclusion_status > ExclusionStatuses.EXCLUDE_PRIZE_AND_SHOWpattern works correctly today because the enum values are ordered by severity (0→4). However, if the enum is ever extended with a value inserted between existing ones, this comparison would silently break. An explicit set check (e.g.,[EXCLUDE_AND_SHOW, EXCLUDE_AND_SHOW_IN_ADVANCED, EXCLUDE].includes(...)) would be more resilient.Not urgent since the enum is new and unlikely to be reordered soon, but worth considering for long-term clarity.
Also applies to: 88-96
front_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/table_row.tsx (1)
42-44: Highlight conflates user-row and exclusion-row styling.Both "current user" and "excluded entry" conditions produce the same orange highlight. In the global leaderboard's
table_row.tsx, these use distinct colors (blue for user, purple for excluded). If that's intentional for the project leaderboard—fine; just flagging the difference.scoring/models.py (1)
373-382: Adddb_index=Truetoexclusion_statusfield.The
exclusion_statusfield is actively filtered in database queries (e.g.,exclusion_status__ltefilters inscoring/views.pylines 93 and 276). Since the deprecatedexcludedfield hasdb_index=Trueand this field is its direct replacement with comparable query patterns, adding an index will improve performance for these filtering operations.♻️ Proposed fix
exclusion_status = models.IntegerField( choices=ExclusionStatuses.choices, default=ExclusionStatuses.INCLUDE, + db_index=True, help_text="""This sets the exclusion status of this entry.
2 participants
new settings allow for fine tuned display rules for leaderboard entries
add exclusion_status enum
ExclusionStatusestoLeaderboardEntryandMedalExclusionRecordset up to deprecate excluded and show_when_excluded fields
Summary by CodeRabbit