feat(career): bundle dive bar venue pack#927
Conversation
📝 WalkthroughWalkthroughCareer venue packs now support plugin-bundled assets alongside downloaded packs. Downloaded packs take precedence, bundled packs are validated and reported through the state API, asset serving supports both locations, and the UI prevents removing bundled packs. ChangesCareer bundled venue packs
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant VenueCard
participant CareerRoutes
participant PackDirectory
VenueCard->>CareerRoutes: Request career state
CareerRoutes->>PackDirectory: Select downloaded or bundled pack
PackDirectory-->>CareerRoutes: Return manifest and asset path
VenueCard->>CareerRoutes: Request venue asset
CareerRoutes->>PackDirectory: Resolve and constrain asset path
PackDirectory-->>VenueCard: Serve bundled or downloaded asset
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/plugins/career/test_routes.py (1)
90-105: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover downloaded-over-bundled precedence.
These tests cover bundled-only and downloaded-only packs, but not the key case where both exist. Install a local
baroverride, assert it is served, delete it, then assert the bundled manifest/media are served again.🤖 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 `@tests/plugins/career/test_routes.py` around lines 90 - 105, Extend test_bundled_bar_pack_is_installed_and_served to create and install a local bar override, then assert the override manifest and media are served in preference to the bundled pack. Delete the local override and repeat the manifest/media assertions to verify serving falls back to the bundled assets.
🤖 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 `@plugins/career/screen.js`:
- Around line 92-97: Update the removal-control logic in the action markup
around the remove variable to use an explicit flag indicating that the selected
pack source is a downloaded override, rather than relying solely on v.bundled.
Keep the delete button visible for downloaded overrides so users can restore the
bundled pack, and hide it only when the installation is bundled-only; propagate
the source flag from routes.py’s pack-selection response.
---
Nitpick comments:
In `@tests/plugins/career/test_routes.py`:
- Around line 90-105: Extend test_bundled_bar_pack_is_installed_and_served to
create and install a local bar override, then assert the override manifest and
media are served in preference to the bundled pack. Delete the local override
and repeat the manifest/media assertions to verify serving falls back to the
bundled assets.
🪄 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 Plus
Run ID: 06cc50e1-0874-4e5b-a4b4-60de2ed0b534
⛔ Files ignored due to path filters (8)
plugins/career/venue-packs/bar/bar-ambience.mp3is excluded by!**/*.mp3plugins/career/venue-packs/bar/bored.mp4is excluded by!**/*.mp4plugins/career/venue-packs/bar/cheer.mp4is excluded by!**/*.mp4plugins/career/venue-packs/bar/clap.mp4is excluded by!**/*.mp4plugins/career/venue-packs/bar/ecstatic.mp4is excluded by!**/*.mp4plugins/career/venue-packs/bar/engaged.mp4is excluded by!**/*.mp4plugins/career/venue-packs/bar/intro.mp4is excluded by!**/*.mp4plugins/career/venue-packs/bar/neutral.mp4is excluded by!**/*.mp4
📒 Files selected for processing (5)
plugins/career/routes.pyplugins/career/screen.jsplugins/career/venue-packs/bar/manifest.jsontests/js/career_plugin.test.jstests/plugins/career/test_routes.py
| const remove = v.bundled | ||
| ? '' | ||
| : `<button data-career-delete="${esc(v.id)}" class="career-btn career-btn-ghost">Remove pack</button>`; | ||
| action = `<div class="flex items-center gap-2"> | ||
| ${main} | ||
| <button data-career-delete="${esc(v.id)}" class="career-btn career-btn-ghost">Remove pack</button> | ||
| ${remove} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Keep removal available for downloaded overrides.
routes.py sets bundled whenever the bundled manifest exists, even when _pack_dir() is currently selecting a downloaded override. This hides the delete button after overriding a bundled venue, leaving no UI path to restore the bundled pack. Expose a downloaded/selected-source flag and hide removal only for bundled-only installations.
🤖 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 `@plugins/career/screen.js` around lines 92 - 97, Update the removal-control
logic in the action markup around the remove variable to use an explicit flag
indicating that the selected pack source is a downloaded override, rather than
relying solely on v.bundled. Keep the delete button visible for downloaded
overrides so users can restore the bundled pack, and hide it only when the
installation is bundled-only; propagate the source flag from routes.py’s
pack-selection response.
Summary
Testing
Summary by CodeRabbit
New Features
Bug Fixes