feat: repurpose soft-deleted dweller assets for radio recruitment - #317
Merged
Conversation
- Radio recruit_dweller() now prefers restoring a soft-deleted dweller (reusing existing S3 image/thumbnail/bio/rarity) over creating a blank new one. Falls back to create_random when pool is empty or an override is supplied. - Three new RadioConfig fields: recycle_enabled, recycle_probability, recycle_min_age_days (all env-overridable via RADIO_ prefix). - RecruitmentResponse gains a recycled bool field; endpoint and frontend store propagate it; frontend shows a distinct toast for recycled recruits. - 29/29 radio service tests passing (11 new, existing tests fixed for updated tuple return types). - conftest: swap SQLAlchemy AsyncSession for SQLModel AsyncSession so .exec() is available in all service tests. - psycopg2 -> psycopg2-binary (no pg_config required in dev/CI). - docker-compose.infra.yml: fix postgres volume mount for PG18 (/var/lib/postgresql instead of /var/lib/postgresql/data). - ROADMAP: retire v2.13.2, promote items into v2.14.
Contributor
|
Warning Review limit reached
More reviews will be available in 29 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR implements radio dweller recycling—a feature allowing recruitment to restore soft-deleted dwellers based on configuration and probability checks—alongside Docker/dependency infrastructure updates and roadmap documentation refreshes. ChangesRadio Dweller Recycling Feature
Infrastructure and Dependency Updates
Documentation Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/services/radio_service.py (1)
243-307:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
manual_recruitreturn contract mismatch.
manual_recruitnow returns(dweller, recycled), but its signature/docstring still declare a singleDweller. This can break type checking and mislead callers.Proposed fix
async def manual_recruit( db_session: AsyncSession, vault_id: UUID4, caps_cost: int | None = None, override: DwellerCreateCommonOverride | None = None, - ) -> Dweller: + ) -> tuple[Dweller, bool]: """ Manually recruit a dweller for caps. @@ Returns: - Newly recruited dweller + Tuple of (dweller, recycled) where recycled=True means the recruit + was restored from a soft-deleted dweller.As per coding guidelines,
backend/app/**/*.py: "Add type hints for new Python functions; prefer UUID4/Pydantic types where applicable".🤖 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/app/services/radio_service.py` around lines 243 - 307, The manual_recruit function currently returns (dweller, recycled) but its signature and docstring claim it returns a single Dweller; update the function signature to reflect the actual return type (e.g., -> tuple[Dweller, bool] or Tuple[Dweller, bool] using typing) and adjust the docstring Returns section to describe both values (newly recruited Dweller and recycled flag). Locate manual_recruit in RadioService (and references to RadioService.recruit_dweller) and change any type annotations or imports as needed to satisfy static typing (add Tuple import if using typing.Tuple or use built-in tuple[...]); ensure callers expecting a single Dweller are reviewed/updated or note that the function now returns a (dweller, recycled) pair.
🤖 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/app/api/tasks.py`:
- Line 10: Shorten the inline comment on the import of app.core.dramatiq so it
stays under 120 characters and uses double quotes; update the comment attached
to the import statement "import app.core.dramatiq" (the comment explaining
broker configuration/set_broker) to a more concise phrase or split it across two
lines immediately above the import (e.g., a brief one-line comment like "Ensure
dramatiq broker is configured (calls set_broker)" or a two-line doc comment) so
the line length and quoting conventions are respected.
In `@backend/pyproject.toml`:
- Line 33: The pyproject lists both synchronous PostgreSQL drivers
("psycopg>=3.3.4,<4" and "psycopg2-binary>=2.9.12") but the app and migrations
use asyncpg (see backend/app/core/config.py and backend/app/alembic/env.py), so
remove the unused sync driver entries from backend/pyproject.toml; search for
any imports/usages of "psycopg" or "psycopg2" to confirm no runtime/tooling
depends on them, delete the redundant dependency lines, and if you need to keep
one for a documented tooling reason, add a brief comment explaining why it
remains.
In `@ROADMAP.md`:
- Around line 522-531: The snippet uses double quotes and semicolons which
violate frontend TypeScript style; update the import string, the ref
initialiser, and any line-terminating semicolons to use single quotes and no
semicolons: change import { useMagicKeys } from "`@vueuse/core`"; to use single
quotes and remove the trailing semicolon, change const sequence = ref(""); to
const sequence = ref('') without a semicolon, and remove the trailing semicolons
from the useMagicKeys destructure line and the watch(...) closing line so that
useMagicKeys, ArrowUp/ArrowDown/ArrowLeft/ArrowRight/b/a, sequence, and watch
follow the project's single-quote/no-semicolon conventions.
---
Outside diff comments:
In `@backend/app/services/radio_service.py`:
- Around line 243-307: The manual_recruit function currently returns (dweller,
recycled) but its signature and docstring claim it returns a single Dweller;
update the function signature to reflect the actual return type (e.g., ->
tuple[Dweller, bool] or Tuple[Dweller, bool] using typing) and adjust the
docstring Returns section to describe both values (newly recruited Dweller and
recycled flag). Locate manual_recruit in RadioService (and references to
RadioService.recruit_dweller) and change any type annotations or imports as
needed to satisfy static typing (add Tuple import if using typing.Tuple or use
built-in tuple[...]); ensure callers expecting a single Dweller are
reviewed/updated or note that the function now returns a (dweller, recycled)
pair.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 195cd221-1a55-40f2-96aa-01a10429635e
⛔ Files ignored due to path filters (1)
backend/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
ROADMAP.mdbackend/app/api/tasks.pybackend/app/api/v1/endpoints/radio.pybackend/app/core/game_config.pybackend/app/schemas/radio.pybackend/app/services/radio_service.pybackend/app/tests/conftest.pybackend/app/tests/test_services/test_radio_service.pybackend/pyproject.tomldocker-compose.infra.ymlfrontend/src/modules/radio/models/radio.tsfrontend/src/modules/radio/stores/radio.ts
| import app.core.dramatiq # noqa: F401 - ensures broker is configured (set_broker) when dramatiq CLI imports this module | ||
| from pydantic import UUID4 | ||
|
|
||
| import app.core.dramatiq # noqa: F401 - ensures broker is configured (set_broker) when dramatiq CLI imports this module |
Contributor
There was a problem hiding this comment.
Shorten the inline comment to stay within 120 characters.
Line 10 appears to exceed the configured Python line-length limit. Split the comment across lines or shorten it.
✂️ Suggested edit
-import app.core.dramatiq # noqa: F401 - ensures broker is configured (set_broker) when dramatiq CLI imports this module
+import app.core.dramatiq # noqa: F401
+# Ensures broker is configured (set_broker) when dramatiq CLI imports this module.As per coding guidelines, **/*.py: Use 120 character line length and double quotes in Python.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import app.core.dramatiq # noqa: F401 - ensures broker is configured (set_broker) when dramatiq CLI imports this module | |
| import app.core.dramatiq # noqa: F401 | |
| # Ensures broker is configured (set_broker) when dramatiq CLI imports this module. |
🤖 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/app/api/tasks.py` at line 10, Shorten the inline comment on the
import of app.core.dramatiq so it stays under 120 characters and uses double
quotes; update the comment attached to the import statement "import
app.core.dramatiq" (the comment explaining broker configuration/set_broker) to a
more concise phrase or split it across two lines immediately above the import
(e.g., a brief one-line comment like "Ensure dramatiq broker is configured
(calls set_broker)" or a two-line doc comment) so the line length and quoting
conventions are respected.
… sites test_api/test_radio.py and test_notification_integrations.py were calling recruit_dweller() / manual_recruit() and assigning the result directly to a Dweller variable, broken by the tuple return introduced in the repurpose feature. Updated both to unpack (dweller, _).
- radio_service.py: fix manual_recruit return type annotation -> tuple[Dweller, bool] and update docstring Returns section to describe both values. - pyproject.toml: remove unused sync PG drivers (psycopg, psycopg2-binary); app and Alembic use asyncpg exclusively (confirmed by grep + env.py). - ROADMAP.md: fix TS code snippet to use single quotes and no semicolons, matching the project's frontend style conventions. - tasks.py comment: skipped (line is 120 chars, within limit; ruff passes).
- backend/pyproject.toml: 2.13.1 -> 2.14.0 - frontend/package.json: 2.13.1 -> 2.14.0 - backend/uv.lock: regenerated for v2.14.0 - CHANGELOG.md: added [2.14.0] release entry
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