add flag to exclude a user from aggregates and scoring#4760
Open
lsabor wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds an ChangesUser Exclusion from Aggregations
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ 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)
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 |
lsabor
force-pushed
the
feat/aggregation-blacklist
branch
from
ab153a7 to
4823924
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scoring/score_math.py (1)
361-369:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlacklisted users are still eligible for peer score rows.
Line 361 keeps
user_forecastsunfiltered, and Lines 496-513 still emit per-user scores from that full set. Also, Line 368 only filtersbase_forecastswhenonly_include_user_idsis not set. This allows excluded users to still be peer-scored.Proposed fix
- user_forecasts = question.user_forecasts.all() + user_forecasts = question.user_forecasts.all().exclude_blacklisted_users() if only_include_user_ids: user_forecasts = user_forecasts.filter(author__id__in=only_include_user_ids) base_forecasts = user_forecasts if not only_include_user_ids: # only include forecasts by non-primary bots if user ids explicitly specified base_forecasts = base_forecasts.exclude_non_primary_bots() - base_forecasts = base_forecasts.exclude_blacklisted_users() if not question.include_bots_in_aggregates: base_forecasts = base_forecasts.exclude(author__is_bot=True)Also applies to: 496-513
🤖 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 `@scoring/score_math.py` around lines 361 - 369, The peer-scoring loop is still using the unfiltered user_forecasts so blacklisted or excluded users can generate per-user rows; ensure the set used to emit per-user peer scores is filtered the same way as aggregates by using base_forecasts (or applying exclude_blacklisted_users()/exclude_non_primary_bots() to user_forecasts) before the per-user scoring section (the code around user_forecasts, base_forecasts, exclude_non_primary_bots, exclude_blacklisted_users, include_bots_in_aggregates and the per-user score emission in the block around lines 496-513). Update the logic so that when only_include_user_ids is set we still remove blacklisted/non-primary-bot users from the dataset used to produce peer score rows.
🤖 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 `@scoring/utils.py`:
- Around line 469-471: The QuerySet assigned to candidates currently selects the
exclude_from_aggregations field but never uses it, so users flagged for
exclusion remain rank-eligible; update the retrieval or post-filtering logic to
enforce exclusion (e.g., add exclude_from_aggregations=False to the
User.objects.filter call or immediately .exclude(exclude_from_aggregations=True)
on the candidates QuerySet) and apply the same change where similar
candidate/user QuerySets are built in the 473-499 region so excluded users are
removed before any ranking or aggregation logic runs.
In `@utils/the_math/aggregations.py`:
- Around line 748-755: The current logic in aggregations.py lets
only_include_user_ids bypass blacklist/bot exclusion: when only_include_user_ids
is provided the code uses forecasts.filter(author_id__in=only_include_user_ids)
but does not call forecasts.exclude_non_primary_bots() or
forecasts.exclude_blacklisted_users(); update the aggregation builders so that
after filtering by only_include_user_ids you also apply
exclude_non_primary_bots() and exclude_blacklisted_users() (or filter out
blacklisted/bot IDs from the provided list) to ensure blacklisted users and
primary bots are always excluded; make the same change in the other occurrence
around the 1002-1009 block.
---
Outside diff comments:
In `@scoring/score_math.py`:
- Around line 361-369: The peer-scoring loop is still using the unfiltered
user_forecasts so blacklisted or excluded users can generate per-user rows;
ensure the set used to emit per-user peer scores is filtered the same way as
aggregates by using base_forecasts (or applying
exclude_blacklisted_users()/exclude_non_primary_bots() to user_forecasts) before
the per-user scoring section (the code around user_forecasts, base_forecasts,
exclude_non_primary_bots, exclude_blacklisted_users, include_bots_in_aggregates
and the per-user score emission in the block around lines 496-513). Update the
logic so that when only_include_user_ids is set we still remove
blacklisted/non-primary-bot users from the dataset used to produce peer score
rows.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 192190df-8943-414a-8b79-6a153b8810c0
📒 Files selected for processing (7)
questions/models.pyscoring/score_math.pyscoring/utils.pyusers/admin.pyusers/migrations/0021_user_exclude_from_aggregations.pyusers/models.pyutils/the_math/aggregations.py
Contributor
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 `@scoring/score_math.py`:
- Around line 365-369: The blacklist exclusion is currently only applied when
not only_include_user_ids, allowing explicit IDs to bypass
exclude_from_aggregations; always apply base_forecasts =
base_forecasts.exclude_blacklisted_users() regardless of the
only_include_user_ids flag. Modify the logic around only_include_user_ids so
that exclude_blacklisted_users() is invoked unconditionally (either move the
call out of the branch or call it in both branches) while keeping
exclude_non_primary_bots() and the question.include_bots_in_aggregates check
behavior unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b34ccdc0-34a6-47c6-abbc-ba5697e09a7c
📒 Files selected for processing (6)
questions/models.pyscoring/score_math.pyusers/admin.pyusers/migrations/0021_user_exclude_from_aggregations.pyusers/models.pyutils/the_math/aggregations.py
Contributor
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
lsabor
requested review from
SylvainChevalier and
hlbmtc
and removed request for
hlbmtc
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 by CodeRabbit
New Features
Bug Fixes