feat(editor): separate project saves from library exports#352
Conversation
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe editor now treats Save as editable project persistence and Export to Library as publishing. Create-mode builds support separate destinations, external saves are session-scoped, Back navigation is guarded, and documentation and tests reflect the revised workflow. ChangesSave and Export Workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant BuildAPI
participant SessionExport
participant Library
Editor->>BuildAPI: request session save or library export
BuildAPI->>SessionExport: write session package for session destination
BuildAPI->>Library: write DLC package for library destination
BuildAPI-->>Editor: return destination-specific result
Editor->>SessionExport: download session package when saving
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 |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @ChrisBeWithYou. * #352 (comment) The following files were modified: * `routes.py` * `src/create.js` * `src/file-ops.js` * `src/main.js` * `src/replace-audio.js` * `src/stem-tracks.js` * `tests/test_create_save_destination.py`
![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
🧹 Nitpick comments (1)
routes.py (1)
8771-8789: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate title/artist sanitization and
suggested_namecomputation.The same
title/artist/safe_t/safe_a/suggested_namelogic is computed once inside_build_sloppak()(to pick the "library" output path) and again in the outer scope after the executor call (to populateexport_filename/the responsefilename). Currently the results match, but the duplication is a drift risk — a future edit to one copy (e.g. adding the truncation/trailing-dot stripping thatcreate_sloppak's_truncate_utf8already applies) could silently diverge from the other and desync the exported download name from what's actually inside the pack.♻️ Suggested consolidation — compute once, return it from the closure
def _build_sloppak(): ... safe_t = re.sub(r'[<>:"/\\|?*]', '_', title) safe_a = re.sub(r'[<>:"/\\|?*]', '_', artist) suggested_name = ( f"{safe_t}_{safe_a}.feedpak" if safe_a else f"{safe_t}.feedpak" ) if destination == "session": output = (Path(session["dir"]) / f"project-save-{time.time_ns()}.feedpak") else: dlc_dir = get_dlc_dir() if not dlc_dir: raise RuntimeError("DLC folder not configured") output = dlc_dir / suggested_name - return _write_sloppak_pak(...) + return _write_sloppak_pak(...), suggested_name try: - output_path = await asyncio.get_event_loop().run_in_executor( + output_path, suggested_name = await asyncio.get_event_loop().run_in_executor( None, _build_sloppak ) ... - title = meta.get("title", "Untitled") - artist = meta.get("artistName") or meta.get("artist", "Unknown") - safe_t = re.sub(r'[<>:"/\\|?*]', '_', title) - safe_a = re.sub(r'[<>:"/\\|?*]', '_', artist) - suggested_name = ( - f"{safe_t}_{safe_a}.feedpak" if safe_a - else f"{safe_t}.feedpak" - ) previous_export = session.get("export_path")Also applies to: 8821-8828
🤖 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 `@routes.py` around lines 8771 - 8789, Consolidate the duplicated title/artist sanitization and suggested_name computation used by _build_sloppak() and the outer export response flow. Compute the filename once inside _build_sloppak(), return it alongside the generated output/path data, and reuse that returned value when setting export_filename and the response filename so both remain synchronized.
🤖 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 `@README.md`:
- Around line 59-62: Update README.md lines 59-62 to state that durable Save and
reused handles apply only when the native picker is available, while browser
fallback downloads a copy without saving the project. Update CHANGELOG.md lines
12-19 to document the browser fallback behavior or explicitly scope destination
reuse to the native desktop contract.
In `@routes.py`:
- Around line 8829-8846: Update the cleanup block around previous_export so a
previously stored export file is unlinked whenever the destination changes away
from the prior session export, including session-to-library transitions; do not
gate this cleanup on destination == "session". Preserve the existing path
comparison and OSError handling, and add a regression test covering a session
build followed by a library build that verifies the original temporary package
is removed.
In `@src/file-ops.js`:
- Around line 936-939: Update the save routing around the saved assignment in
src/file-ops.js: prioritize S.createMode so archive create sessions call
saveCDLC({ skipExternal: true }), while retaining editorSaveAsSloppakConfirm()
only for non-create archive sessions; update tests/create_save_routing.test.mjs
in lines 60-77 with an archive create Save As case asserting /build receives
destination: 'session' and no library export occurs.
---
Nitpick comments:
In `@routes.py`:
- Around line 8771-8789: Consolidate the duplicated title/artist sanitization
and suggested_name computation used by _build_sloppak() and the outer export
response flow. Compute the filename once inside _build_sloppak(), return it
alongside the generated output/path data, and reuse that returned value when
setting export_filename and the response filename so both remain synchronized.
🪄 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: a636417d-8ccd-4bbe-b3c2-6b2e2612c4bc
📒 Files selected for processing (15)
CHANGELOG.mdREADME.mddocs/USER-GUIDE.mdroutes.pyscreen.htmlsrc/create.jssrc/file-ops.jssrc/main.jssrc/menu-bar.jssrc/replace-audio.jssrc/stem-tracks.jstests/create_save_routing.test.mjstests/editor_back_guard.test.mjstests/first_save_picker.test.mjstests/test_create_save_destination.py
1 participant
Summary
This draft separates ordinary project persistence from publishing a playable library copy.
.feedpakproject name and location.The backend keeps
/buildbackward compatible: omitted/librarydestination exports to the configured library, whiledestination: "session"creates an immutable temporary save generation for/session/export. Temporary paths stay private and superseded generations are cleaned up.Why
Importing content should not silently create a library package. The expected document workflow is:
.feedpak.This also keeps quick edits lightweight: Save does not open export settings or run export-only tone preflight.
Important draft boundaries
This is intentionally not merge-ready until the cross-repo contracts below are resolved:
.feedpakcan be reopened without first copying it into the library.beforeunloadwould silently refuse to close rather than show the required dialog. The host needs an awaited navigation guard and the desktop main process needs a close/save/discard/cancel handshake.Safety and regression coverage
Validation
npm test— 311 passedpython -m pytest— 384 passed, 5 skippednpm run lint— 0 errors, 3 pre-existing warningsgit diff --check origin/main...HEADSummary by CodeRabbit