feat(career): host higher venues as opt-in content packs - #1023
Conversation
Move the club and arena venue packs (~678 MB of crowd MP4s) out of the bundle and download them on demand, keeping the bar starter bundled so career still works offline. Leans on career's existing pack pipeline (_download_pack: stream -> sha256 -> extract -> validate -> swap), which already degrades gracefully when a pack is absent. - venues.json: club/arena gain `pack` URLs pointing at per-pack, versioned, immutable releases (venue-<id>-v<N>, matching the existing venue-arena-v1). Arena's sha256/bytes are the real published asset (verified end-to-end); club is a placeholder until its release is published. - tools/content_packs.py: reusable, reproducible pack build/publish/manifest tool. Byte-identical output for identical media (fixed order/mtime/perms, STORED) so a pack's hash can be known before upload. --local (file://) for offline tests, --publish for the per-pack release. Has a --selfcheck. - .github/workflows/content-packs.yml: workflow_dispatch automation that builds/publishes packs and opens the venues.json manifest-bump PR, so publishing is never a manual checklist. - test: round-trips a tool-built pack through career's real _download_pack. Part of the nightly-slimming effort (feedBack-desktop#122). The desktop bundle change (stop shipping club/arena) is a companion PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds deterministic career venue-pack ZIP creation and GitHub publishing, updates venue manifest metadata through a manual workflow, and prevents placeholder packs from appearing or downloading until they have non-zero published sizes. ChangesCareer content pack pipeline
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Maintainer
participant Workflow as content-packs workflow
participant ContentPacks as tools/content_packs.py
participant Venues as plugins/career/venues.json
participant PullRequest as manifest-bump pull request
Maintainer->>Workflow: provide venues and version
Workflow->>ContentPacks: build and publish venue packs
ContentPacks-->>Workflow: write packs manifest
Workflow->>Venues: update URL, SHA-256, and bytes
Workflow->>PullRequest: create venue manifest PR
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
A committed venues.json entry carries a 0-byte placeholder (and all-zero sha) until its release exists. Previously has_pack was true as soon as a `pack` object was present, so the UI showed a "Download" button that could only fail (the placeholder URL 404s). Gate on a real, publish-stamped size via _pack_published(): the card shows "coming soon" and the download endpoint 404s until the pack is actually published. Caught by a real bundle+runtime smoke. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/content-packs.yml:
- Around line 47-52: Update the workflow’s content-pack publishing step to stop
interpolating github.event.inputs.venues and github.event.inputs.version
directly into Bash: expose both through the step env, validate venues and
version against their expected safe formats, build the content-pack paths in a
Bash argument array, and invoke tools/content_packs.py using that array while
preserving the existing --publish behavior.
In `@tools/content_packs.py`:
- Around line 43-57: Update the packing flow around the file collection and
archive creation to reject any top-level source file whose archive name is not
accepted by the downloader’s whitelist, reusing the existing shared
validation/contract symbol rather than duplicating rules. Ensure validation
occurs before publishing the ZIP, and add a test covering a rejected extra file
such as .DS_Store.
- Around line 54-57: Set ZipInfo.create_system explicitly when constructing the
ZipInfo in the archive-writing flow, using a fixed platform value to ensure
identical hashes across operating systems. Keep the existing fixed timestamp,
compression, permissions, and writestr behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d2dfbaf-f524-4581-8977-17533e942567
📒 Files selected for processing (5)
.github/workflows/content-packs.ymlplugins/career/routes.pyplugins/career/venues.jsontests/plugins/career/test_routes.pytools/content_packs.py
|
Ready to go — the POC slice of #122 (slim the nightly). Moves the What to look at:
Behavior in the next nightly: arena downloads live today; club coming-soon (graceful); bar plays offline. Progress is untouched — Merge note: land together with (or just before) got-feedBack/feedBack-desktop#123, which strips the packs from the bundle — the desktop strip shouldn't merge ahead of this download path. |
|
cc @byrongamatos @OmikronApex — you were CodeRabbit's suggested reviewers for this one; would appreciate a look when you have a moment. (Couldn't add you via the reviewer field from my fork.) The desktop companion is got-feedBack/feedBack-desktop#123. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
- workflow: stop interpolating dispatch inputs into Bash (template injection flagged by zizmor). Pass venues/version via env, validate formats, use an argument array. - content_packs: reject top-level files the career downloader would refuse (PACK_FILENAME_RE) before publishing — a stray .DS_Store would otherwise ship and fail _validate_pack_dir for every client. + test. - content_packs: pin ZipInfo.create_system=3 so packs hash identically across Windows/Unix runners (was the documented reproducibility caveat). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Signed-off-by: Matthew Harris Glover <matthew@harrisglover.com>
Signed-off-by: byrongamatos <xasiklas@gmail.com> # Conflicts: # CHANGELOG.md
--publish is a flag (no tag arg) and publish() deliberately omits --clobber; the docstring said otherwise. Signed-off-by: byrongamatos <xasiklas@gmail.com>
Rebased onto merged main (was stacked on #1023/#1024, whose venue work is now in main) so it no longer carries a stale content_packs.py that would revert 1023's build_pack fixes. - build_vst_pack: slice a fat .vst3 tree to one platform (keep its binary dir + shared bundle files, drop the two foreign platform dirs and src/ build trees). Pins create_system=3 like build_pack — without it the same tree hashes differently on a Windows runner (native .vst3 are built there), breaking the precomputable-hash guarantee exactly where it matters. - Publish wiring: 'python tools/content_packs.py <vst-root> --vst --version N --publish' builds+uploads vst-<plat>-vN releases for mac/win/linux and emits a platform-keyed {url,sha256,bytes} manifest — the shape rig_builder's data/vst_packs.json consumes. publish() refactored onto a shared _publish_release helper (venue behaviour unchanged). - Tests: slice keeps target+shared/drops foreign, per-platform binary, reproducibility, unknown-platform reject, and a simulated-win32 guard that fails if the create_system pin is dropped. selfcheck covers the VST path. Original build_vst_pack by Matthew Harris Glover; reworked for the create_system fix, publish wiring, and rebase. Signed-off-by: byrongamatos <xasiklas@gmail.com> Co-authored-by: byrongamatos <xasiklas@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
What
Moves the club and arena career venue packs (~678 MB of crowd MP4s) out of the desktop bundle and downloads them on demand, keeping the bar starter bundled so career works offline. Uses career's existing pack pipeline (
_download_pack: stream → sha256 → extract → validate → swap), which already degrades gracefully when a pack is absent. First (POC) slice of the nightly-slimming effort feedBack-desktop#122. Self-contained; PRs 2/3 build on it but nothing here depends on them.Why this is a real POC
arena is already published and verified (
venue-arena-v1), so on a build of this branch a player can cross 150★ and download + play the real 351 MB arena pack — the whole opt-in model demonstrated end-to-end against a live release asset, not a mock.clubshows "coming soon" until its release is cut.What's here
venues.json— club/arena gainpackURLs (per-pack, versioned, immutable releasesvenue-<id>-v<N>). Arena'ssha256/bytesare the real published asset; club is a placeholder until published._pack_published); until then the card reads "coming soon" and the download endpoint 404s, so there's never a button that can only fail.tools/content_packs.py— reusable, reproducible pack build/publish/manifest tool (byte-identical output → hash known before upload;create_systempinned so Windows and Unix runners agree). Rejects files the downloader would refuse before publishing.--local/--publish,--selfcheck..github/workflows/content-packs.yml—workflow_dispatchautomation that builds/publishes packs and opens the manifest-bump PR. Dispatch inputs are validated and passed via env (no shell interpolation)._download_pack; placeholder-not-offered + rejected-extra-file regressions.Verification
arena-pack-v1.zip, sha verifies, extracts, validates, installs. ✅electron-buildermacOS.app: career = bar only, ~678 MB off. ✅Companion PRs (separate, not required to merge this)
venue-club-v1→ club flips to downloadable automatically.feedpak surface
got-feedback/feedpak-spec#___(once it merges, re-run this PR's checks and the gate goes green)Checklist
CHANGELOG.md[Unreleased]updated (user-visible changes) — entry written; lands with the signed-off push belowgit commit -s) — pending: re-signing the branch with the correct email (see below)Summary by CodeRabbit