Conversation
Generate free default bios (no AI) via create_random_common_dweller, registering origin + rarity-scaled visited places on the world map. Cap visited places by rarity (common 2, rare 3, legendary 5) instead of flat 5, and skip map registration during pregen to avoid double-registration.
Radio recruitment rolls a rare-chance recruit per RadioConfig.rare_chance instead of always spawning common dwellers, enabling the rare-content pipeline. reward_service now surfaces objective-parse and rarity errors immediately instead of swallowing them, and accepts string UUIDs for dweller ids when granting experience.
Adds tests for procedural bio generation (place pool validity, origin in pool, visited excludes origin, rarity-scaled counts, determinism, template rendering), create_random bio-place registration and skip flag, rarity- scaled map registration, radio rare-chance recruitment, and reward objective/rarity error matching.
Documents the procedural bio pipeline: free default bios, rarity-scaled visited-place caps, and radio rare recruits, validated against existing map_service and DwellerBio linkification systems.
Line-wrapping and whitespace normalization from the project formatter (vp fmt). No behavior changes.
Adds a one-off local script that fills empty dweller bios using SPECIAL-stat-driven templates, includes origin/visited places in each bio, and registers those places on the vault world map. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Adds the template-based dweller bio filler to the 2.28.0 release notes. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Prevents the script from running on import so its functions can be unit-tested. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Covers highest-stat selection, deterministic place picking, rarity-scaled visited counts, bio formatting, and all SPECIAL-stat template branches. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Convert all backend Python scripts to Typer CLIs and consolidate them under backend/scripts/: - create_admin, bio filler, fix image URLs, set RustFS policies, quest migration, room-image downloader - add simulate_* balance scripts (happiness/incident/room/exploration) - remove backend/initial_data.py and app/scripts/, root scripts/ is now shell-only - add backend/scripts/README.md documenting each tool - drop app/scripts from coverage omit (dir no longer exists)
One-off local script that scans dwellers whose bios contain place names but who have no map locations yet, extracts origin/visited places via word-boundary regex against the known place lists, and registers them via map_service.register_bio_places. Supports --vault and --max-dwellers CLI args.
Remove the Happiness nav item from SidePanel.vue; the aggregate HappinessDashboard now renders inside the Dwellers view above the filter panel, and the Map nav item gained hotkey 8. The /vault/:id/happiness route is retained for deep links.
for more information, see https://pre-commit.ci
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughVersion 2.28.0 adds rarity-aware dweller bios and map registration, bio maintenance scripts, standardized Typer CLIs, stricter reward errors, happiness dashboard integration, and sidebar navigation changes. ChangesRarity-aware dweller bios and map registration
Reward validation and error propagation
Backend bio scripts and Typer commands
Dashboard and navigation updates
Release metadata and repository support
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/scripts/download_room_images.py (1)
117-132: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject path components from remote image names.
data-image-nameand the fallback URL segment are remote input. An absolute filename or../component makesPath(DOWNLOAD_DIR) / filenamewrite outside the download directory.Reduce the decoded value to a basename and reject empty or invalid names before constructing
path.Proposed fix
- filename = urllib.parse.unquote(filename) + filename = Path(urllib.parse.unquote(filename)).name + if not filename or filename in {".", ".."}: + continue🤖 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/scripts/download_room_images.py` around lines 117 - 132, Sanitize the decoded remote filename in the filename-selection flow before calling download_image: reduce data-image-name and fallback URL values to a basename, then reject empty or invalid names including absolute paths and traversal components. Ensure the validated basename is the only value used when constructing the download path, while preserving the existing image-extension and skip-list filtering.
🧹 Nitpick comments (1)
frontend/src/modules/dwellers/views/DwellersView.vue (1)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOrder the imports by source group.
Place Vue/core imports first. Place third-party imports next. Place
@/imports before relative imports. The new imports preserve a noncompliant order.As per coding guidelines: “Prefer
@/path aliases for imports and organize imports roughly as Vue/core, third-party,@/, then relative imports.”Also applies to: 28-29
🤖 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/modules/dwellers/views/DwellersView.vue` at line 11, Reorder the imports in DwellersView.vue by source group: Vue/core imports first, third-party packages next, then all `@/` aliased imports including useIncidentStore, followed by relative imports. Preserve the existing import statements and only adjust their ordering.Source: Coding guidelines
🤖 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/core/game_config.py`:
- Around line 470-477: Add non-negative item-level validation to
BioConfig.visited_by_rarity so every rarity cap from BIO_VISITED_BY_RARITY is
rejected during Pydantic configuration loading; reuse the project’s
typed-container validation or add a field_validator for this field. Preserve
max_visited() fallback behavior, and enforce an appropriate upper bound
consistent with the later map-registration cap if required.
In `@backend/app/services/reward_service.py`:
- Around line 350-354: Prevent partial-success errors from being retried as new
operations: in backend/app/services/reward_service.py lines 350-354, update the
quest reward flow around _process_single_reward to make the full reward set
atomic or persist idempotent outcomes before propagating failures; in
backend/app/services/radio_service.py lines 225-234, decouple successful
recruitment from notification delivery by using an outbox or separate delivery
result so notification errors cannot cause another dweller to be recruited.
Ensure both paths return or record successful durable operations without
triggering duplicate retries.
In `@backend/app/tests/test_crud/test_dweller.py`:
- Line 187: Add the explicit -> None return annotation to
test_create_random_registers_bio_places and test_create_random_skip_bio_places
in backend/app/tests/test_crud/test_dweller.py (lines 187-187 and 212-212), and
to test_recruit_dweller_rare_chance_rolls_rare in
backend/app/tests/test_services/test_radio_service.py (line 549-549).
In `@backend/app/tests/test_scripts/test_backfill_dweller_bio_places.py`:
- Around line 57-342: Add an explicit None return annotation to every new test
function in backend/app/tests/test_scripts/test_backfill_dweller_bio_places.py
lines 57-342 and
backend/app/tests/test_scripts/test_fill_dweller_bios_templates.py lines 36-158,
including both synchronous extraction tests and asynchronous main integration
tests; do not otherwise change their behavior.
In `@backend/scripts/backfill_dweller_bio_places.py`:
- Around line 203-209: Update map_service.register_bio_places to propagate
registration failures instead of returning normally after caught exceptions. In
backend/scripts/backfill_dweller_bio_places.py lines 203-209, increment
processed only when registration completes successfully. In
backend/scripts/fill_dweller_bios_templates.py lines 245-255, handle the
propagated failure by avoiding persistence of the generated bio as complete and
returning a failure status or scheduling a retry.
- Around line 237-241: The command entry points in
backend/scripts/backfill_dweller_bio_places.py lines 237-241 and
backend/scripts/fill_dweller_bios_templates.py lines 268-276 must validate
--vault as a UUID before invoking their async workflows. Update each CLI
function to parse the value with UUID or raise typer.BadParameter, then pass
only the validated UUID to main(...), preventing invalid input from reaching
deferred UUID conversion.
In `@backend/scripts/create_admin.py`:
- Around line 83-99: Update create_admin_cli to remove the default values for
email, username, and especially password, requiring the password explicitly or
through a hidden confirmation prompt. Ensure create_admin never prints or echoes
the password after creation while preserving the existing database URL behavior.
In `@backend/scripts/download_room_images.py`:
- Around line 155-158: Add the None return annotation to the new run_download
function in backend/scripts/download_room_images.py (lines 155-158) and the new
run_migration function in backend/scripts/migrate_quest_data.py (lines 224-225),
preserving their existing behavior.
In `@backend/scripts/fill_dweller_bios_templates.py`:
- Around line 54-75: Normalize each candidate in _VISITED_PLACES by trimming
whitespace before sampling, and exclude the normalized origin candidate
“Sanctuary Hills” from the pool before selection. Ensure sampled place names are
canonical so map registration receives no leading spaces or origin collisions.
Add regressions covering “ Zion Canyon” normalization and exclusion of
“Sanctuary Hills”.
In `@docs/features/BIO_MAP_UNCOVERING.md`:
- Line 3: Update the status line in BIO_MAP_UNCOVERING.md from proposed design
to implemented/released, and revise the former-behavior descriptions in the
sections around lines 21-23 and 51-52 so they accurately describe the
implemented feature rather than presenting outdated behavior as current.
In `@frontend/src/modules/dwellers/views/DwellersView.vue`:
- Around line 62-88: Update happinessDashboardData to derive distribution and
idleDwellerCount from an unfiltered dataset containing all vault dwellers,
rather than dwellerStore.dwellers after fetchDwellers applies filterStatus or
filterAgeGroup. Use an existing store value that always contains the complete
vault population or add a dedicated dashboard dataset, while keeping
dwellerCount and the other metrics unchanged.
In `@frontend/tests/unit/views/DwellersView.test.ts`:
- Line 141: Update the fetchIncidents mocks in
frontend/tests/unit/views/DwellersView.test.ts at lines 141-141 and 168-168 to
return a valid empty IncidentListResponse with data.incidents set to an empty
array, so both tests exercise the successful empty-state path.
---
Outside diff comments:
In `@backend/scripts/download_room_images.py`:
- Around line 117-132: Sanitize the decoded remote filename in the
filename-selection flow before calling download_image: reduce data-image-name
and fallback URL values to a basename, then reject empty or invalid names
including absolute paths and traversal components. Ensure the validated basename
is the only value used when constructing the download path, while preserving the
existing image-extension and skip-list filtering.
---
Nitpick comments:
In `@frontend/src/modules/dwellers/views/DwellersView.vue`:
- Line 11: Reorder the imports in DwellersView.vue by source group: Vue/core
imports first, third-party packages next, then all `@/` aliased imports including
useIncidentStore, followed by relative imports. Preserve the existing import
statements and only adjust their ordering.
🪄 Autofix
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 Plus
Run ID: 8fe34041-df6b-49f8-94f1-75a424c2bff0
⛔ Files ignored due to path filters (1)
backend/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (50)
.gitignoreCHANGELOG.mdbackend/app/core/game_config.pybackend/app/crud/dweller.pybackend/app/services/map_service.pybackend/app/services/pregen_service.pybackend/app/services/radio_service.pybackend/app/services/reward_service.pybackend/app/tests/test_crud/test_dweller.pybackend/app/tests/test_scripts/__init__.pybackend/app/tests/test_scripts/test_backfill_dweller_bio_places.pybackend/app/tests/test_scripts/test_fill_dweller_bios_templates.pybackend/app/tests/test_services/test_map_service.pybackend/app/tests/test_services/test_radio_service.pybackend/app/tests/test_services/test_reward_service.pybackend/app/tests/test_services/test_reward_service_extended.pybackend/app/tests/test_utils/test_dwellers.pybackend/app/utils/dwellers.pybackend/initial_data.pybackend/pyproject.tomlbackend/scripts/README.mdbackend/scripts/backfill_dweller_bio_places.pybackend/scripts/create_admin.pybackend/scripts/download_room_images.pybackend/scripts/fill_dweller_bios_templates.pybackend/scripts/fix_dweller_image_urls.pybackend/scripts/migrate_quest_data.pybackend/scripts/set_rustfs_bucket_policies.pydocs/features/BIO_MAP_UNCOVERING.mdfrontend/package.jsonfrontend/src/core/components/common/SidePanel.vuefrontend/src/modules/dwellers/views/DwellersView.vuefrontend/src/modules/map/components/WorldMap.vuefrontend/src/modules/profile/components/ProfileEditor.vuefrontend/tests/e2e/features.spec.tsfrontend/tests/e2e/interaction.spec.tsfrontend/tests/e2e/regression.spec.tsfrontend/tests/e2e/smoke.spec.tsfrontend/tests/e2e/ui-interaction.spec.tsfrontend/tests/unit/components/SidePanel.test.tsfrontend/tests/unit/composables/useGoBack.test.tsfrontend/tests/unit/composables/useTheme.test.tsfrontend/tests/unit/composables/useVisualEffects.test.tsfrontend/tests/unit/modules/map/routes.test.tsfrontend/tests/unit/services/trainingService.test.tsfrontend/tests/unit/stores/dwellerDeath.test.tsfrontend/tests/unit/stores/dwellerFilter.test.tsfrontend/tests/unit/stores/map.test.tsfrontend/tests/unit/stores/training.test.tsfrontend/tests/unit/views/DwellersView.test.ts
💤 Files with no reviewable changes (1)
- backend/initial_data.py
Comment on lines
+350
to
354
| result = await self._process_single_reward(db_session, vault_id, reward.reward_type, reward.reward_data) | ||
| granted_rewards.append(result) | ||
|
|
||
| logger.info(f"Processed {len(granted_rewards)}/{len(rewards)} rewards for quest '{quest.title}'") | ||
| return granted_rewards |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not propagate errors after independently committed operations.
These paths can return an error after an earlier durable operation succeeds. A caller retry can duplicate a quest reward or recruit another dweller.
backend/app/services/reward_service.py#L350-L354: make the complete quest reward set atomic, or record idempotent reward outcomes before propagating a failure.backend/app/services/radio_service.py#L225-L234: deliver notifications through an outbox or separate delivery result from successful recruitment.
📍 Affects 2 files
backend/app/services/reward_service.py#L350-L354(this comment)backend/app/services/radio_service.py#L225-L234
🤖 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/reward_service.py` around lines 350 - 354, Prevent
partial-success errors from being retried as new operations: in
backend/app/services/reward_service.py lines 350-354, update the quest reward
flow around _process_single_reward to make the full reward set atomic or persist
idempotent outcomes before propagating failures; in
backend/app/services/radio_service.py lines 225-234, decouple successful
recruitment from notification delivery by using an outbox or separate delivery
result so notification errors cannot cause another dweller to be recruited.
Ensure both paths return or record successful durable operations without
triggering duplicate retries.
Comment on lines
+203
to
+209
| await map_service.register_bio_places( | ||
| session, | ||
| dweller, | ||
| origin_place=origin or "", | ||
| visited_places=visited, | ||
| ) | ||
| processed += 1 |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Expose map-registration failure to both maintenance scripts.
backend/app/services/map_service.py:110-170 catches registration exceptions and returns normally. Both scripts therefore treat a failed registration as successful.
backend/scripts/backfill_dweller_bio_places.py#L203-L209: incrementprocessedonly after a confirmed registration.backend/scripts/fill_dweller_bios_templates.py#L245-L255: do not persist the generated bio as complete when location registration fails; return a status or schedule a retry.
📍 Affects 2 files
backend/scripts/backfill_dweller_bio_places.py#L203-L209(this comment)backend/scripts/fill_dweller_bios_templates.py#L245-L255
🤖 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/scripts/backfill_dweller_bio_places.py` around lines 203 - 209,
Update map_service.register_bio_places to propagate registration failures
instead of returning normally after caught exceptions. In
backend/scripts/backfill_dweller_bio_places.py lines 203-209, increment
processed only when registration completes successfully. In
backend/scripts/fill_dweller_bios_templates.py lines 245-255, handle the
propagated failure by avoiding persistence of the generated bio as complete and
returning a failure status or scheduling a retry.
Comment on lines
+155
to
+158
| def run_download(): | ||
| download_dir = Path(DOWNLOAD_DIR) | ||
| if not download_dir.exists(): | ||
| download_dir.mkdir(parents=True) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add return annotations to the new entry functions.
backend/scripts/download_room_images.py#L155-L158: add-> Nonetorun_download.backend/scripts/migrate_quest_data.py#L224-L225: add-> Nonetorun_migration.
As per coding guidelines, backend/**/*.py requires type hints for new functions.
📍 Affects 2 files
backend/scripts/download_room_images.py#L155-L158(this comment)backend/scripts/migrate_quest_data.py#L224-L225
🤖 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/scripts/download_room_images.py` around lines 155 - 158, Add the None
return annotation to the new run_download function in
backend/scripts/download_room_images.py (lines 155-158) and the new
run_migration function in backend/scripts/migrate_quest_data.py (lines 224-225),
preserving their existing behavior.
Source: Coding guidelines
- game_config: reject negative visited_by_rarity caps (field_validator) - scripts: UUID validation for --vault (backfill/fill CLIs), resilient per-dweller failure handling in filler loop, sanitize download filenames (basename, no traversal), secure create_admin CLI (required creds, hidden+confirmed prompt, no password echo), -> None annotations on run_download/run_migration - tests: -> None annotations on all new script/service test functions - docs: BIO_MAP_UNCOVERING status -> implemented & shipped in v2.28 - frontend: import order per convention; happiness dashboard aggregates from unfiltered allDwellers (new store fetch) instead of filtered list; fix fetchIncidents mock shape in DwellersView tests
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/modules/dwellers/stores/dwellerFilter.ts (1)
157-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the request into the dwellers service.
fetchAllDwellerscalls Axios directly from the Pinia store. This bypasses the requiredStore → Service → APIboundary and duplicates request construction. Add the request to the existing dwellers service, then keep the store focused on state updates.As per coding guidelines, follow the frontend architecture
Store → Service → API.🤖 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/modules/dwellers/stores/dwellerFilter.ts` around lines 157 - 163, Move the Axios request out of fetchAllDwellers and into the existing dwellers service, exposing a service method for fetching dwellers by vaultId with the authorization token. Update fetchAllDwellers to call that service method and retain only the resulting state updates in the store, preserving the existing response and error behavior.Source: Coding guidelines
🤖 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 `@frontend/src/modules/dwellers/stores/dwellerFilter.ts`:
- Line 31: Update the dweller filter store’s allDwellers/fetchAllDwellers flow
to scope results to the active vault: clear the collection before each load,
track the requested vaultId and latest request, and apply results only when the
response still matches the current vault and latest request; preserve the
cleared state when requests fail or become stale.
- Around line 153-164: Update fetchAllDwellers to include an explicit
complete-fetch query strategy, using skip=0 and a sufficiently large non-null
limit in its axios request. Preserve the existing authorization and
allDwellers.value assignment behavior.
---
Nitpick comments:
In `@frontend/src/modules/dwellers/stores/dwellerFilter.ts`:
- Around line 157-163: Move the Axios request out of fetchAllDwellers and into
the existing dwellers service, exposing a service method for fetching dwellers
by vaultId with the authorization token. Update fetchAllDwellers to call that
service method and retain only the resulting state updates in the store,
preserving the existing response and error behavior.
🪄 Autofix
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 Plus
Run ID: dd9bd761-9d43-416a-b10a-1b0eec70f3dd
📒 Files selected for processing (13)
backend/app/core/game_config.pybackend/app/tests/test_crud/test_dweller.pybackend/app/tests/test_scripts/test_backfill_dweller_bio_places.pybackend/app/tests/test_scripts/test_fill_dweller_bios_templates.pybackend/app/tests/test_services/test_radio_service.pybackend/scripts/backfill_dweller_bio_places.pybackend/scripts/create_admin.pybackend/scripts/download_room_images.pybackend/scripts/fill_dweller_bios_templates.pydocs/features/BIO_MAP_UNCOVERING.mdfrontend/src/modules/dwellers/stores/dwellerFilter.tsfrontend/src/modules/dwellers/views/DwellersView.vuefrontend/tests/unit/views/DwellersView.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- backend/app/core/game_config.py
- backend/app/tests/test_services/test_radio_service.py
- backend/app/tests/test_crud/test_dweller.py
- frontend/src/modules/dwellers/views/DwellersView.vue
- backend/scripts/download_room_images.py
- backend/app/tests/test_scripts/test_fill_dweller_bios_templates.py
- backend/app/tests/test_scripts/test_backfill_dweller_bio_places.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/modules/dwellers/stores/dwellerFilter.ts (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRoute
fetchDwellersByVaultthrough the new service too.
getDwellersByVaultnow owns the URL construction, the snake_case parameter mapping, and the bearer header.fetchDwellersByVaultat lines 125-164 repeats all of that against raw axios. The two copies will diverge on the next parameter change.Move
fetchDwellersByVaultonto the same service call. This also aligns the store with theStore → Service → APIarchitecture.As per coding guidelines: "Follow the frontend architecture
Store → Service → API."🤖 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/modules/dwellers/stores/dwellerFilter.ts` at line 6, Update fetchDwellersByVault to call getDwellersByVault instead of constructing the URL, mapping parameters, or issuing raw axios requests. Remove the duplicated request logic while preserving the store’s existing state and error-handling behavior, so the flow follows Store → Service → API.Source: Coding guidelines
🤖 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 `@frontend/src/modules/dwellers/stores/dwellerFilter.ts`:
- Around line 9-14: Update the doc comment for ALL_DWELLERS_FETCH_LIMIT to
describe 1000 as a maximum request limit rather than guaranteeing all dwellers
are returned; do not claim complete-fetch behavior unless the associated
fetchAllDwellers logic is changed to paginate until a response contains fewer
rows than the limit.
In `@frontend/tests/unit/stores/dwellerFilter.test.ts`:
- Around line 142-151: Update the dweller fixtures assigned to store.allDwellers
in the affected test cases, including the assignments near the existing “old”
fixture and the one near line 231, to satisfy the DwellerReadLess/DwellerShort
type. Add all required health, radiation, health-bound, thumbnail_url, and
SPECIAL-stat fields while preserving the existing fixture values and test
behavior.
---
Nitpick comments:
In `@frontend/src/modules/dwellers/stores/dwellerFilter.ts`:
- Line 6: Update fetchDwellersByVault to call getDwellersByVault instead of
constructing the URL, mapping parameters, or issuing raw axios requests. Remove
the duplicated request logic while preserving the store’s existing state and
error-handling behavior, so the flow follows Store → Service → API.
🪄 Autofix
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 Plus
Run ID: 259bf374-aff5-4f26-859d-527cf03e5f41
📒 Files selected for processing (4)
frontend/src/modules/dwellers/services/dwellerService.tsfrontend/src/modules/dwellers/services/index.tsfrontend/src/modules/dwellers/stores/dwellerFilter.tsfrontend/tests/unit/stores/dwellerFilter.test.ts
Comment on lines
+9
to
+14
| /** | ||
| * Non-null limit for complete-fetch requests (fetchAllDwellers). The backend | ||
| * default limit is 100; this ensures ALL dwellers are returned for dashboard | ||
| * aggregates regardless of vault population. | ||
| */ | ||
| export const ALL_DWELLERS_FETCH_LIMIT = 1000 |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the guarantee stated in the doc comment.
ALL_DWELLERS_FETCH_LIMIT is a hard cap, not a complete-fetch guarantee. If a vault holds more than 1000 dwellers, the backend truncates the response and the dashboard aggregates are silently wrong. The comment states the opposite.
Either state the cap in the comment, or paginate until the backend returns fewer rows than the limit.
📝 Proposed comment fix
/**
- * Non-null limit for complete-fetch requests (fetchAllDwellers). The backend
- * default limit is 100; this ensures ALL dwellers are returned for dashboard
- * aggregates regardless of vault population.
+ * Upper bound for complete-fetch requests (fetchAllDwellers). The backend
+ * default limit is 100, which truncates dashboard aggregates. This raises the
+ * cap to 1000. Vaults above 1000 dwellers still truncate; paginate if that
+ * becomes reachable.
*/
export const ALL_DWELLERS_FETCH_LIMIT = 1000📝 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
| /** | |
| * Non-null limit for complete-fetch requests (fetchAllDwellers). The backend | |
| * default limit is 100; this ensures ALL dwellers are returned for dashboard | |
| * aggregates regardless of vault population. | |
| */ | |
| export const ALL_DWELLERS_FETCH_LIMIT = 1000 | |
| /** | |
| * Upper bound for complete-fetch requests (fetchAllDwellers). The backend | |
| * default limit is 100, which truncates dashboard aggregates. This raises the | |
| * cap to 1000. Vaults above 1000 dwellers still truncate; paginate if that | |
| * becomes reachable. | |
| */ | |
| export const ALL_DWELLERS_FETCH_LIMIT = 1000 |
🤖 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/modules/dwellers/stores/dwellerFilter.ts` around lines 9 - 14,
Update the doc comment for ALL_DWELLERS_FETCH_LIMIT to describe 1000 as a
maximum request limit rather than guaranteeing all dwellers are returned; do not
claim complete-fetch behavior unless the associated fetchAllDwellers logic is
changed to paginate until a response contains fewer rows than the limit.
Comment on lines
+142
to
+151
| store.allDwellers = [ | ||
| { | ||
| id: 'old', | ||
| first_name: 'Old', | ||
| last_name: 'Data', | ||
| status: 'idle', | ||
| level: 1, | ||
| happiness: 50, | ||
| }, | ||
| ] |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the required fields of DwellerShort.
fd -t f 'dweller.ts' frontend/src/modules/dwellers/models --exec cat -n {}Repository: ElderEvil/falloutProject
Length of output: 6331
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Generated DwellerReadLess schema and related schemas:"
fd -t f 'api.generated.ts' frontend --exec sh -c 'echo "--- $1"; rg -n "DwellerReadLess|DwellerReadFull|DwellerCreate|DwellerUpdate|id:|first_name:|last_name:|status:|level:|happiness:" "$1" | head -200' sh {}
echo
echo "Test fixture lines:"
cat -n frontend/tests/unit/stores/dwellerFilter.test.ts | sed -n '120,165p;218,240p'
echo
echo "Store type usage for allDwellers:"
rg -n "allDwellers|DwellerShort" frontend/src frontend/tests -g '*.ts' -g '*.tsx' | sed -n '1,200p'Repository: ElderEvil/falloutProject
Length of output: 7930
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Generated file candidates:"
git ls-files 'frontend/**/*' | rg '(^frontend/.*/api\.generated\.(ts|json|yaml)|frontend/.*/openapi|swagger|api-generated)' || true
echo
echo "Text-only schema search in tracked frontend files:"
rg -n "DwellerReadLess|DwellerReadFull|DwellerCreate|DwellerUpdate|DwellerRead" frontend || true
echo
echo "Generated schema context (if present):"
git ls-files | rg '(^frontend/.*/api\.generated\.(ts|json|yaml)$|openapi|swagger)' | xargs -r -I{} sh -c 'echo "--- {}"; rg -n "DwellerRead|DwellerCreate|DwellerUpdate|first_name|last_name|id:|status:|level:|happiness:" "{}" | head -200'Repository: ElderEvil/falloutProject
Length of output: 9871
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Exact DwellerReadLess schema:"
sed -n '4318,4355p' frontend/src/core/types/api.generated.ts | cat -n
echo
echo "Adjacent Dweller schemas for inheritance/comparison:"
sed -n '4011,4305p' frontend/src/core/types/api.generated.ts | cat -n
echo
echo "Type checker availability and test for inline assignment shape:"
node - <<'JS'
try {
const { spawnSync } = require('node:child_process')
const fs = require('node:fs')
const tmp = require('node:os').tmpdir()
const path = require('node:path')
const ts = path.join(tmp, `dweller_fixture_${process.pid}.ts`)
const mts = path.join(tmp, `dweller_fixture_${process.pid}.mts`)
const tsconfig = path.join(tmp, `dweller_fixture_tsconfig_${process.pid}.json`)
fs.writeFileSync(ts, `
type RequiredLike = { id: string; first_name: string });
const fixtures: RequiredLike[] = [
{ id: 'old', first_name: 'Old', last_name: 'Data', status: 'idle', level: 1, happiness: 50 },
]
`)
fs.writeFileSync(tsconfig, JSON.stringify({
compilerOptions: { strict: true, noEmit: true, target: 'ESNext', module: 'commonjs', moduleResolution: 'node', types: [] }
}, null, 2))
const res = spawnSync('npx', ['tsc', '-p', tsconfig, '--noEmit'], { cwd: tmp, encoding: 'utf8', timeout: 30000 })
console.log({ code: res.status, stdout: res.stdout, stderr: res.stderr })
fs.unlinkSync(ts)
fs.unlinkSync(tsconfig)
} catch (error) {
console.log('Unavailable:', error && error.code ? error.code : String(error && error.message))
}
JSRepository: ElderEvil/falloutProject
Length of output: 12023
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Full DwellerReadLess schema:"
sed -n '4318,4358p' frontend/src/core/types/api.generated.ts | cat -n
echo
echo "Type checker behavior for inline fixtures vs DwellerReadLess:"
node - <<'JS'
const { spawnSync } = require('node:child_process')
const fs = require('node:fs')
const tmp = require('node:os').tmpdir()
const ts = `
type DwellerReadLess = {
id: string
first_name: string
last_name: string | null
thumbnail_url: string | null
level: number
health: number
max_health: number
radiation: number
happiness: number
room_id?: string | null
status: string
is_adult: boolean
age_group: string
gender: string
birth_date?: string | null
strength: number
perception: number
endurance: number
charisma: number
intelligence: number
agility: number
luck: number
}
const fixtures: DwellerReadLess[] = [
{ id: 'old', first_name: 'Old', last_name: 'Data', status: 'idle', level: 1, happiness: 50 },
]
`
const tsconfig = JSON.stringify({
compilerOptions: { strict: true, noEmit: true, target: 'ESNext', module: 'commonjs', moduleResolution: 'node' }
}, null, 2)
fs.writeFileSync(`${tmp}/fixture.ts`, ts)
fs.writeFileSync(`${tmp}/tsconfig.json`, tsconfig)
const res = spawnSync('npx', ['typescript', '--noEmit'], { cwd: tmp, encoding: 'utf8', timeout: 30000 })
fs.unlinkSync(`${tmp}/fixture.ts`)
fs.unlinkSync(`${tmp}/tsconfig.json`)
console.log({ code: res.status, stdout: res.stdout, stderr: res.stderr })
JSRepository: ElderEvil/falloutProject
Length of output: 2337
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Run TypeScript behavioral probe without relying on npx or repo env:"
node - <<'JS'
const fs = require('fs')
if (!fs.existsSync('/usr/local/lib/node_modules/typescript') && !fs.existsSync('/tmp/tsprobe/lib')) {
console.log('typescript runtime unavailable in /usr/local/lib/node_modules/typescript')
process.exit(0)
}
const ts = require('typescript')
const tsProbe = fs.realpathSync('/usr/local/lib/node_modules/typescript')
const lang = ts.createLanguageService({
getCompilationSettings() { return { strict: true, noEmit: true, target: ts.ScriptTarget.ESNext, module: ts.ModuleKind.CommonJS, moduleResolution: ts.ModuleResolutionKind.Node10, types: [] } },
getScriptFileNames() { return ['probe.ts'] },
getScriptVersion() { return '1' },
getScriptSnapshot(name) {
if (name !== 'probe.ts') return undefined
return {
getLength() { return source.length },
getText(a, b) { return source.slice(a, b) },
getChangeRange() { return undefined }
}
},
directoryExists() { return false },
fileExists() { return false },
readFile() { return '' },
getFileNames() { return ['probe.ts'] },
getCurrentDirectory() { return process.cwd() },
getDefaultLibFileName() { return tsProbe + '/lib/lib.d.ts' },
useCaseSensitiveFileNames() { return true },
trace() {},
log() {},
write() {}
}, ts.createDocumentRegistry())
const source = `
type DwellerReadLess = {
id: string
first_name: string
last_name: string | null
thumbnail_url: string | null
level: number
health: number
max_health: number
radiation: number
happiness: number
room_id?: string | null
status: string
is_adult: boolean
age_group: string
gender: string
birth_date?: string | null
strength: number
perception: number
endurance: number
charisma: number
intelligence: number
agility: number
luck: number
};
const fixtures: DwellerReadLess[] = [{ id: 'old', first_name: 'Old', last_name: 'Data', status: 'idle', level: 1, happiness: 50 }];
`
lang.updateProgram();
const diag = lang.getSyntacticDiagnostics('probe.ts').concat(lang.getSemanticDiagnostics('probe.ts')).concat(lang.getSuggestionDiagnostics('probe.ts'));
console.log(JSON.stringify({ count: diag.length, diagnostics: diag.map(d => ({ message: ts.flattenDiagnosticMessageText(d.messageText, '\n'), line: d.file && d.file.getLineAndCharacterOfPosition(d.start).line + 1 })) }, null, 2))
JSRepository: ElderEvil/falloutProject
Length of output: 843
Supply the DwellerReadLess fields in these fixtures.
DwellerShort is components['schemas']['DwellerReadLess'], and those fixtures only include id, first_name, last_name, status, level, and happiness. Add the required DWL fields, such as the health/radiation/health bounds, thumbnail_url, and SPECIAL stats, so vue-tsc accepts the assignments at lines 142 and 231.
🤖 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/tests/unit/stores/dwellerFilter.test.ts` around lines 142 - 151,
Update the dweller fixtures assigned to store.allDwellers in the affected test
cases, including the assignments near the existing “old” fixture and the one
near line 231, to satisfy the DwellerReadLess/DwellerShort type. Add all
required health, radiation, health-bound, thumbnail_url, and SPECIAL-stat fields
while preserving the existing fixture values and test behavior.
… with xdist - backend/pyproject.toml: remove --cov and --cov-report flags from addopts - .github/workflows/backend-ci.yml: add -n auto + explicit --cov flags on CI test step
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
Documentation