Open
Conversation
Convert both pyproject.toml files from Poetry to PEP 621 format, add Ruff config replacing Black + Flake8, delete .flake8, and swap poetry.lock files for uv.lock. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Automated formatting pass with `ruff format .` to establish Ruff as the project formatter (replacing Black). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace Poetry with UV in the main Dockerfile backend_deps stage. Simplify screenshot Dockerfile by removing pyenv and build-essential dependencies — UV handles Python version management directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace Poetry with UV in unit_tests.yml and integration_tests.yml workflows, using astral-sh/setup-uv@v5 with built-in caching. Replace all poetry run with uv run in shell scripts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace pyenv/Poetry setup instructions with UV, and update all poetry run commands to uv run throughout the documentation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cemreinanc
temporarily deployed
to
testing_env
GitHub Actions
Inactive
cemreinanc
temporarily deployed
to
testing_env
GitHub Actions
Inactive
Contributor
📝 WalkthroughWalkthroughThe PR migrates the project from Poetry to UV for Python dependency management and from Black/Flake8 to Ruff for linting and formatting. Configuration files, CI/CD workflows, Dockerfiles, documentation, and scripts are updated accordingly, along with numerous minor formatting adjustments throughout the codebase. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
Contributor
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
cemreinanc
temporarily deployed
to
testing_env
GitHub Actions
Inactive
cemreinanc
temporarily deployed
to
testing_env
GitHub Actions
Inactive
![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 (2)
manage.py (1)
10-14:⚠️ Potential issue | 🟡 MinorMisplaced docstring becomes dead code.
The docstring on line 13 appears after
load_dotenv(), so Python treats it as a string literal expression (no-op) rather than the function's docstring. Move it before any executable code to be recognized asmain.__doc__.Proposed fix
def main(): + """Run administrative tasks.""" load_dotenv() - - """Run administrative tasks.""" os.environ.setdefault("DJANGO_SETTINGS_MODULE", "metaculus_web.settings")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@manage.py` around lines 10 - 14, The docstring for main() is placed after executable code (load_dotenv()) so it is treated as a no-op string instead of main.__doc__; move the triple-quoted docstring so it appears immediately after the def main(): line (before calling load_dotenv()) so Python recognizes it as the function docstring; update the main() function (which contains load_dotenv() and os.environ.setdefault("DJANGO_SETTINGS_MODULE", "metaculus_web.settings")) accordingly.questions/views.py (1)
159-172:⚠️ Potential issue | 🟡 MinorBug: Error message will always show
Noneinstead of the actual question ID.The validation at line 171-172 runs after
withdrawal["question"]is overwritten with the (possiblyNone) question object at line 161. WhenquestionisNone, the error message prints"Wrong question id None"instead of the original invalid ID.Compare with
bulk_create_forecasts_api_viewwhere the check happens before the overwrite (lines 109-112).Proposed fix: validate before overwriting
for withdrawal in validated_data: question = questions_map.get(withdrawal["question"]) - withdrawal["question"] = question # used in withdraw_foreacst_bulk withdraw_at = withdrawal.get("withdraw_at", now) withdrawal["withdraw_at"] = withdraw_at # used in withdraw_foreacst_bulk if now > withdraw_at: return Response( {"error": f"Withdrawal time {withdraw_at} cannot be in the past"}, status=status.HTTP_405_METHOD_NOT_ALLOWED, ) if not question: raise ValidationError(f"Wrong question id {withdrawal['question']}") + withdrawal["question"] = question # used in withdraw_foreacst_bulk + # Check permissions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@questions/views.py` around lines 159 - 172, The code overwrites withdrawal["question"] with the question object before validating existence, so the ValidationError prints None instead of the original id; change the order in the loop in questions/views.py to first check question = questions_map.get(withdrawal["question"]) and if not question raise ValidationError using the original withdrawal["question"] value, then set withdrawal["question"] = question (so withdraw_foreacst_bulk sees the object); keep the withdraw_at logic and the now > withdraw_at check as-is but ensure validation happens before the overwrite.
🧹 Nitpick comments (5)
screenshot/Dockerfile (2)
26-27: Consider adding--no-devto exclude dev dependencies.Unlike the main Dockerfile which uses
uv sync --frozen --no-dev, this usesuv sync --frozenwhich will include dev dependencies (e.g.,ruff). For a production screenshot service, dev dependencies are likely unnecessary and would increase image size.Proposed fix
-RUN uv sync --frozen +RUN uv sync --frozen --no-dev🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@screenshot/Dockerfile` around lines 26 - 27, Change the uv sync invocation to exclude dev dependencies: replace the RUN command that calls "uv sync --frozen" with a call including "--no-dev" (i.e., "uv sync --frozen --no-dev") so dev packages like ruff are not installed in the production screenshot image; keep the subsequent "RUN uv run playwright install chromium --with-deps" as-is to ensure browser deps are installed.
8-8: Pin the UV image version for reproducibility.Same recommendation as the main Dockerfile: using
:latestmay cause inconsistent builds. Consider pinning to a specific version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@screenshot/Dockerfile` at line 8, The Dockerfile uses the image reference ghcr.io/astral-sh/uv:latest in the COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /usr/local/bin/ instruction; pin that image to a specific semantic version tag or immutable digest (e.g., replace :latest with a concrete tag or `@sha256`:...) so builds are reproducible and update the COPY --from reference accordingly.pyproject.toml (1)
92-93: Consider documenting the high McCabe complexity threshold.
max-complexity = 56is significantly higher than the default of 10. While this is likely intentional to avoid breaking existing code during the tooling migration, consider adding a comment explaining this choice or creating a follow-up task to gradually reduce it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 92 - 93, The McCabe threshold under [tool.ruff.lint.mccabe] is set to an unusually high value (max-complexity = 56); add a brief inline comment next to this setting explaining why 56 was chosen (e.g., to ease migration) and/or add a TODO with a reference to a new follow-up task/issue to gradually lower the threshold to a safer default, ensuring the comment references "max-complexity" and the "[tool.ruff.lint.mccabe]" section so reviewers can find and track the rationale.scripts/tests/run_integration_tests.sh (1)
20-22: Minor: Extra space in command.Line 20 has a double space:
uv run ./manage.py. This doesn't affect functionality but is inconsistent with the other commands.Proposed fix
-uv run ./manage.py collectstatic --noinput +uv run ./manage.py collectstatic --noinput🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/tests/run_integration_tests.sh` around lines 20 - 22, Remove the extra double space in the collectstatic command so it matches the other commands' formatting: change the invocation in the script where the line uses "uv run ./manage.py collectstatic --noinput" to use a single space after "run" (i.e., "uv run ./manage.py collectstatic --noinput") to ensure consistent spacing with the other commands like the gunicorn and rundramatiq lines.README.md (1)
102-115: Consider adding migration guidance for existing Poetry users.The UV setup instructions are technically correct and will work well for new contributors. However, existing developers migrating from Poetry might benefit from additional context:
- Cleanup steps (removing
.venv, clearing Poetry cache)- Brief note that this replaces Poetry
- Link to UV documentation for reference
📝 Suggested enhancement for existing users
Consider adding a note after line 103:
## UV and Python & Dependencies You'll need to install python version 3.12.3 (or higher), and we use [uv](https://docs.astral.sh/uv/) for dependency and Python version management. + +> **Note for existing contributors:** If you previously used Poetry, you can remove your old `.venv` directory before running `uv sync`. Install uv:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 102 - 115, Add migration guidance for existing Poetry users in the "UV and Python & Dependencies" section: explain that uv replaces Poetry, recommend cleanup steps (remove project .venv, optional pipx/virtualenv remnants, and clear Poetry cache via `poetry cache clear --all`), advise reinstalling deps with `uv sync`, and include a short pointer/link to the UV docs for more details; place this note directly after the uv installation instructions so migrating devs see it next to the `uv sync` step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Around line 44-47: The Dockerfile is copying artifacts from the UV builder
image using the floating tag ghcr.io/astral-sh/uv:latest which makes builds
non-reproducible; change the COPY --from reference in the lines that use
ghcr.io/astral-sh/uv:latest to a specific stable tag (for example
ghcr.io/astral-sh/uv:0.10.6) so COPY --from=... /uv /uvx /usr/local/bin/ pulls
from a pinned UV image, and update any related build docs or CI that rely on the
UV tag to the same pinned version.
In `@README.md`:
- Line 103: Update the README sentence that currently says "python version
3.12.3 (or higher)" to match the constraint in pyproject.toml (requires-python =
">=3.12,<3.13"); specifically replace the phrase with either "Python 3.12.x" or
"Python 3.12 (>=3.12, <3.13)" so the README and the pyproject.toml's
requires-python declaration are consistent.
---
Outside diff comments:
In `@manage.py`:
- Around line 10-14: The docstring for main() is placed after executable code
(load_dotenv()) so it is treated as a no-op string instead of main.__doc__; move
the triple-quoted docstring so it appears immediately after the def main(): line
(before calling load_dotenv()) so Python recognizes it as the function
docstring; update the main() function (which contains load_dotenv() and
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "metaculus_web.settings"))
accordingly.
In `@questions/views.py`:
- Around line 159-172: The code overwrites withdrawal["question"] with the
question object before validating existence, so the ValidationError prints None
instead of the original id; change the order in the loop in questions/views.py
to first check question = questions_map.get(withdrawal["question"]) and if not
question raise ValidationError using the original withdrawal["question"] value,
then set withdrawal["question"] = question (so withdraw_foreacst_bulk sees the
object); keep the withdraw_at logic and the now > withdraw_at check as-is but
ensure validation happens before the overwrite.
---
Nitpick comments:
In `@pyproject.toml`:
- Around line 92-93: The McCabe threshold under [tool.ruff.lint.mccabe] is set
to an unusually high value (max-complexity = 56); add a brief inline comment
next to this setting explaining why 56 was chosen (e.g., to ease migration)
and/or add a TODO with a reference to a new follow-up task/issue to gradually
lower the threshold to a safer default, ensuring the comment references
"max-complexity" and the "[tool.ruff.lint.mccabe]" section so reviewers can find
and track the rationale.
In `@README.md`:
- Around line 102-115: Add migration guidance for existing Poetry users in the
"UV and Python & Dependencies" section: explain that uv replaces Poetry,
recommend cleanup steps (remove project .venv, optional pipx/virtualenv
remnants, and clear Poetry cache via `poetry cache clear --all`), advise
reinstalling deps with `uv sync`, and include a short pointer/link to the UV
docs for more details; place this note directly after the uv installation
instructions so migrating devs see it next to the `uv sync` step.
In `@screenshot/Dockerfile`:
- Around line 26-27: Change the uv sync invocation to exclude dev dependencies:
replace the RUN command that calls "uv sync --frozen" with a call including
"--no-dev" (i.e., "uv sync --frozen --no-dev") so dev packages like ruff are not
installed in the production screenshot image; keep the subsequent "RUN uv run
playwright install chromium --with-deps" as-is to ensure browser deps are
installed.
- Line 8: The Dockerfile uses the image reference ghcr.io/astral-sh/uv:latest in
the COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /usr/local/bin/
instruction; pin that image to a specific semantic version tag or immutable
digest (e.g., replace :latest with a concrete tag or `@sha256`:...) so builds are
reproducible and update the COPY --from reference accordingly.
In `@scripts/tests/run_integration_tests.sh`:
- Around line 20-22: Remove the extra double space in the collectstatic command
so it matches the other commands' formatting: change the invocation in the
script where the line uses "uv run ./manage.py collectstatic --noinput" to use
a single space after "run" (i.e., "uv run ./manage.py collectstatic --noinput")
to ensure consistent spacing with the other commands like the gunicorn and
rundramatiq lines.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
poetry.lockis excluded by!**/*.lockscreenshot/poetry.lockis excluded by!**/*.lockscreenshot/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.flake8.github/workflows/integration_tests.yml.github/workflows/unit_tests.ymlClaude.mdDockerfileREADME.mdcomments/management/commands/update_top_comments_of_week_batch.pyfab_management/utils.pyfab_management/views.pymanage.pymetaculus_web/settings.pymisc/services/itn.pyposts/admin.pyprojects/views/communities.pypyproject.tomlquestions/management/commands/build_forecasts.pyquestions/models.pyquestions/serializers/common.pyquestions/tasks.pyquestions/views.pyscoring/jobs.pyscoring/management/commands/update_global_bot_leaderboard.pyscreenshot/Dockerfilescreenshot/pyproject.tomlscreenshot/start.shscripts/create_minimal_db.shscripts/tests/run_integration_tests.shtests/unit/test_coherence/factories.pytests/unit/test_comments/factories.pytests/unit/test_notifications/factories.pytests/unit/test_posts/factories.pytests/unit/test_projects/factories.pytests/unit/test_questions/factories.pytests/unit/test_questions/test_views.pytests/unit/test_scoring/factories.pytests/unit/test_scoring/test_score_math.pytests/unit/test_scoring/test_utils.pytests/unit/test_users/factories.pytests/unit/test_users/test_views.pytests/unit/test_utils/test_the_math/test_aggregations.pytests/unit/test_utils/test_the_math/test_formulas.pyusers/services/bots_management.pyutils/models.pyutils/tasks.pyutils/the_math/aggregations.py
💤 Files with no reviewable changes (8)
- tests/unit/test_utils/test_the_math/test_aggregations.py
- .flake8
- tests/unit/test_utils/test_the_math/test_formulas.py
- tests/unit/test_scoring/test_utils.py
- tests/unit/test_questions/test_views.py
- questions/management/commands/build_forecasts.py
- fab_management/views.py
- utils/the_math/aggregations.py
Contributor
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 (1)
questions/views.py (1)
159-172:⚠️ Potential issue | 🟠 MajorFix logic bug: question validation happens after reassignment.
Pre-existing issue (not introduced by this PR): The check
if not question:on line 171 occurs afterwithdrawal["question"] = questionon line 161. WhenquestionisNone, the error message at line 172 displays "Wrong question id None" instead of the actual invalid ID.Compare with
bulk_create_forecasts_api_view(lines 106-112), where the validation happens before reassignment.🐛 Proposed fix to check before reassignment
for withdrawal in validated_data: question = questions_map.get(withdrawal["question"]) + + if not question: + raise ValidationError(f"Wrong question id {withdrawal['question']}") + withdrawal["question"] = question # used in withdraw_foreacst_bulk withdraw_at = withdrawal.get("withdraw_at", now) withdrawal["withdraw_at"] = withdraw_at # used in withdraw_foreacst_bulk if now > withdraw_at: return Response( {"error": f"Withdrawal time {withdraw_at} cannot be in the past"}, status=status.HTTP_405_METHOD_NOT_ALLOWED, ) - if not question: - raise ValidationError(f"Wrong question id {withdrawal['question']}") - # Check permissions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@questions/views.py` around lines 159 - 172, The validation for question existence is done after overwriting withdrawal["question"] with the resolved object, causing the error message to show "None" instead of the original id; move the check "if not question:" to before "withdrawal['question'] = question" and raise ValidationError using the original id (e.g., use the pre-reassignment withdrawal["question"] value) so the message shows the invalid id; keep the withdraw_at handling and the now > withdraw_at Response logic unchanged and ensure withdraw_foreacst_bulk still receives the resolved question object.
♻️ Duplicate comments (2)
README.md (1)
103-103:⚠️ Potential issue | 🟡 MinorUpdate README to clarify Python version constraints.
Line 103 states "python version 3.12.3 (or higher)", but
pyproject.tomlspecifiesrequires-python = ">=3.12,<3.13". The "(or higher)" phrasing is misleading since versions 3.13+ are not supported. Consider updating to "Python 3.12.x" for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 103, Update the README phrasing to match the pyproject.toml Python constraint: replace "python version 3.12.3 (or higher)" with a clear statement like "Python 3.12.x (>=3.12,<3.13)" or "Python >=3.12,<3.13" so it reflects the requires-python = ">=3.12,<3.13" setting in pyproject.toml and removes the misleading "or higher" wording.Dockerfile (1)
44-47:⚠️ Potential issue | 🟡 MinorPin the UV image version to ensure build reproducibility.
Using
ghcr.io/astral-sh/uv:latestrisks inconsistent builds as UV releases frequently. Pin to a specific stable version (e.g.,ghcr.io/astral-sh/uv:0.6.6or the version used locally) for reproducible builds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 44 - 47, Replace the floating image tag in the Dockerfile COPY line that references ghcr.io/astral-sh/uv:latest with an explicit, pinned UV release (for example ghcr.io/astral-sh/uv:0.6.6) to guarantee reproducible builds; update the COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /usr/local/bin/ line to use the chosen version and search for any other occurrences of ghcr.io/astral-sh/uv:latest in the Dockerfile to change them as well.
🧹 Nitpick comments (4)
screenshot/pyproject.toml (1)
8-8: Tight version constraint for FastAPI.
fastapi>=0.109.1,<0.110restricts to only the 0.109.x patch releases. If this is intentional for stability, it's fine, but it may prevent receiving security fixes in newer minor versions. Consider relaxing tofastapi>=0.109.1,<1unless there's a known incompatibility with newer versions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@screenshot/pyproject.toml` at line 8, The FastAPI dependency in pyproject.toml is pinned too narrowly as "fastapi>=0.109.1,<0.110"; relax the constraint to allow newer minor/major releases (for example change to "fastapi>=0.109.1,<1") unless there is a known incompatibility, so update the version spec for the FastAPI dependency line accordingly to permit receiving security/feature fixes while still bounding breaking changes.pyproject.toml (2)
92-93: Unusually high cyclomatic complexity threshold.
max-complexity = 56is significantly higher than typical values (10-15). This effectively disables complexity warnings for most code. Was this value intentionally preserved from the previous Flake8 configuration, or should it be lowered to enforce better code quality going forward?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 92 - 93, The mccabe cyclomatic complexity threshold is set too high; update the [tool.ruff.lint.mccabe] section to lower max-complexity (e.g., 10–15, commonly 12) to re-enable useful complexity warnings. Locate the max-complexity setting under the [tool.ruff.lint.mccabe] table and change the value from 56 to a sensible target (suggest 12) so Ruff enforces reasonable complexity limits going forward.
67-67: Consider adding an upper bound forruff.Unlike other dependencies in this file,
ruff>=0.11.0lacks an upper bound. Ruff releases frequently and occasionally introduces breaking lint rule changes. Adding an upper bound (e.g.,ruff>=0.11.0,<0.12) would provide more predictable CI behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` at line 67, The dependency declaration for ruff ("ruff>=0.11.0") has no upper bound; update the requirement in pyproject.toml to include a conservative upper bound (for example change "ruff>=0.11.0" to "ruff>=0.11.0,<0.12") so frequent ruff releases won't unexpectedly break CI; locate the ruff entry in the dependencies list and add the chosen "<0.12" upper constraint.screenshot/Dockerfile (1)
26-26: Consider using--no-devfor production builds.
uv sync --frozenwill install all dependencies including dev dependencies. For a production Docker image, adding--no-devwould reduce image size and attack surface by excluding development tools.Proposed fix
-RUN uv sync --frozen +RUN uv sync --frozen --no-dev🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@screenshot/Dockerfile` at line 26, The Dockerfile RUN step currently uses the command "uv sync --frozen" which installs dev dependencies; update the RUN instruction that invokes uv sync to include the production flag (add "--no-dev") so it runs "uv sync --frozen --no-dev" (or equivalent uv production install flag) to avoid installing dev dependencies and shrink the production image.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@screenshot/Dockerfile`:
- Line 8: The Dockerfile uses an unpinned image reference in the COPY
instruction ("COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx
/usr/local/bin/"), which makes builds non-reproducible; change the image tag to
a specific, stable version (e.g., ghcr.io/astral-sh/uv:0.10.6) so the COPY
--from source is pinned and the build is reproducible.
---
Outside diff comments:
In `@questions/views.py`:
- Around line 159-172: The validation for question existence is done after
overwriting withdrawal["question"] with the resolved object, causing the error
message to show "None" instead of the original id; move the check "if not
question:" to before "withdrawal['question'] = question" and raise
ValidationError using the original id (e.g., use the pre-reassignment
withdrawal["question"] value) so the message shows the invalid id; keep the
withdraw_at handling and the now > withdraw_at Response logic unchanged and
ensure withdraw_foreacst_bulk still receives the resolved question object.
---
Duplicate comments:
In `@Dockerfile`:
- Around line 44-47: Replace the floating image tag in the Dockerfile COPY line
that references ghcr.io/astral-sh/uv:latest with an explicit, pinned UV release
(for example ghcr.io/astral-sh/uv:0.6.6) to guarantee reproducible builds;
update the COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /usr/local/bin/ line
to use the chosen version and search for any other occurrences of
ghcr.io/astral-sh/uv:latest in the Dockerfile to change them as well.
In `@README.md`:
- Line 103: Update the README phrasing to match the pyproject.toml Python
constraint: replace "python version 3.12.3 (or higher)" with a clear statement
like "Python 3.12.x (>=3.12,<3.13)" or "Python >=3.12,<3.13" so it reflects the
requires-python = ">=3.12,<3.13" setting in pyproject.toml and removes the
misleading "or higher" wording.
---
Nitpick comments:
In `@pyproject.toml`:
- Around line 92-93: The mccabe cyclomatic complexity threshold is set too high;
update the [tool.ruff.lint.mccabe] section to lower max-complexity (e.g., 10–15,
commonly 12) to re-enable useful complexity warnings. Locate the max-complexity
setting under the [tool.ruff.lint.mccabe] table and change the value from 56 to
a sensible target (suggest 12) so Ruff enforces reasonable complexity limits
going forward.
- Line 67: The dependency declaration for ruff ("ruff>=0.11.0") has no upper
bound; update the requirement in pyproject.toml to include a conservative upper
bound (for example change "ruff>=0.11.0" to "ruff>=0.11.0,<0.12") so frequent
ruff releases won't unexpectedly break CI; locate the ruff entry in the
dependencies list and add the chosen "<0.12" upper constraint.
In `@screenshot/Dockerfile`:
- Line 26: The Dockerfile RUN step currently uses the command "uv sync --frozen"
which installs dev dependencies; update the RUN instruction that invokes uv sync
to include the production flag (add "--no-dev") so it runs "uv sync --frozen
--no-dev" (or equivalent uv production install flag) to avoid installing dev
dependencies and shrink the production image.
In `@screenshot/pyproject.toml`:
- Line 8: The FastAPI dependency in pyproject.toml is pinned too narrowly as
"fastapi>=0.109.1,<0.110"; relax the constraint to allow newer minor/major
releases (for example change to "fastapi>=0.109.1,<1") unless there is a known
incompatibility, so update the version spec for the FastAPI dependency line
accordingly to permit receiving security/feature fixes while still bounding
breaking changes.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
poetry.lockis excluded by!**/*.lockscreenshot/poetry.lockis excluded by!**/*.lockscreenshot/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.flake8.github/workflows/integration_tests.yml.github/workflows/unit_tests.ymlClaude.mdDockerfileREADME.mdcomments/management/commands/update_top_comments_of_week_batch.pyfab_management/utils.pyfab_management/views.pymanage.pymetaculus_web/settings.pymisc/services/itn.pyposts/admin.pyprojects/views/communities.pypyproject.tomlquestions/management/commands/build_forecasts.pyquestions/models.pyquestions/serializers/common.pyquestions/tasks.pyquestions/views.pyscoring/jobs.pyscoring/management/commands/update_global_bot_leaderboard.pyscreenshot/Dockerfilescreenshot/pyproject.tomlscreenshot/start.shscripts/create_minimal_db.shscripts/tests/run_integration_tests.shtests/unit/test_coherence/factories.pytests/unit/test_comments/factories.pytests/unit/test_notifications/factories.pytests/unit/test_posts/factories.pytests/unit/test_projects/factories.pytests/unit/test_questions/factories.pytests/unit/test_questions/test_views.pytests/unit/test_scoring/factories.pytests/unit/test_scoring/test_score_math.pytests/unit/test_scoring/test_utils.pytests/unit/test_users/factories.pytests/unit/test_users/test_views.pytests/unit/test_utils/test_the_math/test_aggregations.pytests/unit/test_utils/test_the_math/test_formulas.pyusers/services/bots_management.pyutils/models.pyutils/tasks.pyutils/the_math/aggregations.py
💤 Files with no reviewable changes (8)
- tests/unit/test_utils/test_the_math/test_formulas.py
- .flake8
- utils/the_math/aggregations.py
- fab_management/views.py
- tests/unit/test_utils/test_the_math/test_aggregations.py
- tests/unit/test_questions/test_views.py
- questions/management/commands/build_forecasts.py
- tests/unit/test_scoring/test_utils.py
cemreinanc
temporarily deployed
to
testing_env
GitHub Actions
Inactive
cemreinanc
temporarily deployed
to
testing_env
GitHub Actions
Inactive
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)
questions/views.py (1)
159-173:⚠️ Potential issue | 🟡 MinorPreserve the original invalid question ID in the error message.
At Line 161,
withdrawal["question"]is overwritten before validation, so Line 172 can emitNoneinstead of the submitted ID.💡 Suggested fix
# Replacing prefetched optimized questions for withdrawal in validated_data: - question = questions_map.get(withdrawal["question"]) - withdrawal["question"] = question # used in withdraw_foreacst_bulk + question_id = withdrawal["question"] + question = questions_map.get(question_id) withdraw_at = withdrawal.get("withdraw_at", now) withdrawal["withdraw_at"] = withdraw_at # used in withdraw_foreacst_bulk if now > withdraw_at: return Response( {"error": f"Withdrawal time {withdraw_at} cannot be in the past"}, status=status.HTTP_405_METHOD_NOT_ALLOWED, ) if not question: - raise ValidationError(f"Wrong question id {withdrawal['question']}") + raise ValidationError(f"Wrong question id {question_id}") + withdrawal["question"] = question # used in withdraw_foreacst_bulk🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@questions/views.py` around lines 159 - 173, The loop overwrites withdrawal["question"] with the resolved Question object before validating existence, so the ValidationError may report None; before assigning withdrawal["question"] = question in the loop over validated_data (and before using withdraw_at), save the original id (e.g., orig_qid = withdrawal.get("question")) and use that original id in the ValidationError message (referencing validated_data, questions_map, withdrawal, and the withdraw_foreacst_bulk usage) so the error shows the submitted ID instead of the replaced value.
♻️ Duplicate comments (1)
Dockerfile (1)
44-44:⚠️ Potential issue | 🟡 MinorPin UV image to an immutable reference.
Line 44 still uses a floating minor tag (
ghcr.io/astral-sh/uv:0.10), which can change over time and break reproducibility. Prefer a patch tag plus digest.🔧 Suggested update
+ARG UV_IMAGE=ghcr.io/astral-sh/uv:0.10.6 -COPY --from=ghcr.io/astral-sh/uv:0.10 /uv /uvx /usr/local/bin/ +COPY --from=${UV_IMAGE} /uv /uvx /usr/local/bin/Is ghcr.io/astral-sh/uv:0.10 a moving tag, and what is the recommended way to pin uv images for reproducible Docker builds (specific version tag vs digest)?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 44, The Dockerfile uses a floating minor tag in the build stage COPY (--from=ghcr.io/astral-sh/uv:0.10 /uv /uvx /usr/local/bin/) which harms reproducibility; update the image reference to an immutable value by replacing ghcr.io/astral-sh/uv:0.10 with a specific patch tag or, preferably, the image digest (sha256:...) so the COPY --from= reference points to a fixed immutable image; locate the COPY --from line and substitute the tag with the recommended patch-level tag or the full `@sha256` digest.
🧹 Nitpick comments (1)
.github/workflows/integration_tests.yml (1)
55-57: Pinsetup-uvto an exact patch version.The
version: "0.10.x"selector on line 55 allows any patch version within 0.10 to be used, which can cause non-deterministic CI behavior if a new patch is released between runs. Pinning to a specific patch keeps CI reproducible and makes tool upgrades explicit.Proposed change
- version: "0.10.x" + version: "0.10.6"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/integration_tests.yml around lines 55 - 57, The workflow uses the setup-uv action with a floating patch selector ("version: \"0.10.x\""), which can lead to non-deterministic CI; update the setup for the setup-uv action by replacing the version selector "0.10.x" with a specific patch version (e.g., "0.10.3") so the action is pinned to an exact release; locate the block containing version: "0.10.x" for the setup-uv step and set the version field to the desired exact patch string to make CI reproducible and upgrades explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Line 17: Update the dependency constraint for django-sql-utils so it is pinned
exactly to 0.7.0 instead of a range; locate the dependency entry
"django-sql-utils>=0.7.0,<0.8" and replace it with an exact spec for version
0.7.0 to prevent silent patch-level upgrades and ensure compatibility with
Django 5.2.
In `@screenshot/Dockerfile`:
- Around line 11-14: The Dockerfile currently runs an unverified remote script
via "curl | sh" to install the Heroku CLI; replace that unsafe step in the RUN
layer with a verified installation flow: either install a pinned Heroku CLI via
npm (e.g., run npm install -g heroku@<version> after installing node/npm) or
download the official Heroku Linux tarball, fetch its published checksum or
signature, verify the checksum/signature, extract and install the binary, and
then purge build artifacts (replacing the existing curl/pipe and subsequent
purge). Update the RUN block in the Dockerfile to perform the chosen verified
method (download+verify+extract or npm install with a pinned version) and remove
the curl|sh pipeline to eliminate the supply-chain risk.
---
Outside diff comments:
In `@questions/views.py`:
- Around line 159-173: The loop overwrites withdrawal["question"] with the
resolved Question object before validating existence, so the ValidationError may
report None; before assigning withdrawal["question"] = question in the loop over
validated_data (and before using withdraw_at), save the original id (e.g.,
orig_qid = withdrawal.get("question")) and use that original id in the
ValidationError message (referencing validated_data, questions_map, withdrawal,
and the withdraw_foreacst_bulk usage) so the error shows the submitted ID
instead of the replaced value.
---
Duplicate comments:
In `@Dockerfile`:
- Line 44: The Dockerfile uses a floating minor tag in the build stage COPY
(--from=ghcr.io/astral-sh/uv:0.10 /uv /uvx /usr/local/bin/) which harms
reproducibility; update the image reference to an immutable value by replacing
ghcr.io/astral-sh/uv:0.10 with a specific patch tag or, preferably, the image
digest (sha256:...) so the COPY --from= reference points to a fixed immutable
image; locate the COPY --from line and substitute the tag with the recommended
patch-level tag or the full `@sha256` digest.
---
Nitpick comments:
In @.github/workflows/integration_tests.yml:
- Around line 55-57: The workflow uses the setup-uv action with a floating patch
selector ("version: \"0.10.x\""), which can lead to non-deterministic CI; update
the setup for the setup-uv action by replacing the version selector "0.10.x"
with a specific patch version (e.g., "0.10.3") so the action is pinned to an
exact release; locate the block containing version: "0.10.x" for the setup-uv
step and set the version field to the desired exact patch string to make CI
reproducible and upgrades explicit.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
poetry.lockis excluded by!**/*.lockscreenshot/poetry.lockis excluded by!**/*.lockscreenshot/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.flake8.github/workflows/integration_tests.yml.github/workflows/unit_tests.ymlClaude.mdDockerfileREADME.mdcomments/management/commands/update_top_comments_of_week_batch.pyfab_management/utils.pyfab_management/views.pymanage.pymetaculus_web/settings.pymisc/services/itn.pyposts/admin.pyprojects/views/communities.pypyproject.tomlquestions/management/commands/build_forecasts.pyquestions/models.pyquestions/serializers/common.pyquestions/tasks.pyquestions/views.pyscoring/jobs.pyscoring/management/commands/update_global_bot_leaderboard.pyscreenshot/Dockerfilescreenshot/pyproject.tomlscreenshot/start.shscripts/create_minimal_db.shscripts/tests/run_integration_tests.shtests/unit/test_coherence/factories.pytests/unit/test_comments/factories.pytests/unit/test_notifications/factories.pytests/unit/test_posts/factories.pytests/unit/test_projects/factories.pytests/unit/test_questions/factories.pytests/unit/test_questions/test_views.pytests/unit/test_scoring/factories.pytests/unit/test_scoring/test_score_math.pytests/unit/test_scoring/test_utils.pytests/unit/test_users/factories.pytests/unit/test_users/test_views.pytests/unit/test_utils/test_the_math/test_aggregations.pytests/unit/test_utils/test_the_math/test_formulas.pyusers/services/bots_management.pyutils/models.pyutils/tasks.pyutils/the_math/aggregations.py
💤 Files with no reviewable changes (8)
- tests/unit/test_questions/test_views.py
- tests/unit/test_scoring/test_utils.py
- .flake8
- tests/unit/test_utils/test_the_math/test_aggregations.py
- fab_management/views.py
- utils/the_math/aggregations.py
- tests/unit/test_utils/test_the_math/test_formulas.py
- questions/management/commands/build_forecasts.py
elisescu
approved these changes
Feb 26, 2026
Contributor
elisescu
left a comment
elisescu
left a comment
There was a problem hiding this comment.
LGTM and this is great!
Maybe split it in two separate PRs, one that updates the tooling to use uv and the other that reformats the code? This would make it possible for people to rebase their existing PRs in an easier way, i.e. get fewer merge conflicts.
But ask the rest and if they don't have PRs waiting to be merged, then no need to split it.
12 tasks
lsabor
approved these changes
Feb 26, 2026
Contributor
lsabor
left a comment
lsabor
left a comment
There was a problem hiding this comment.
LGTM
I agree with Elis about splitting the uv dependency from the Ruff dependency as 2 PRs.
But on that note I don't have any large PRs that would be hurt from the formatting change, so if this is a convenient time - let's just go for it. Definitely double check with Hlib and Nikita about any outstanding major PRs.
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
4 participants
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.
CI Backend Checks Setup Phase:
closes #4097
Summary by CodeRabbit
Refactor
Documentation
Style