feat(setbuilder): Engine DJ + Lexicon exports via Rekordbox XML (#401)#453
Merged
Conversation
Capture the approved design for exposing Engine DJ and Lexicon as export targets. Research established both platforms import the existing Rekordbox DJ_PLAYLISTS XML, so the work is labeled presets of the existing renderer plus an ISRC fidelity fix — not new serializers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ordered TDD plan: ISRC fidelity fix, schema Literal widening, renderer registration, OpenAPI + generated-types regen, and the ExportModal Engine DJ + Lexicon options. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DJ_PLAYLISTS has no native ISRC attribute, so the identifier was silently dropped on every rekordbox/Engine DJ/Lexicon export. Carry it in the TRACK Comments attribute as "ISRC:<isrc>", sanitized through the existing control-char cleaner. Absent ISRC omits Comments entirely. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Neither platform has a proprietary import format — both ingest the Rekordbox DJ_PLAYLISTS XML WrzDJ already generates. Widen the export schema Literals with distinct "enginedj"/"lexicon" keys and map both to render_rekordbox_xml (application/xml, .xml). Distinct keys (not aliases) keep them listed separately in the UI and leave room for future per-platform tweaks without churning the contract. Regenerates openapi.json. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Flip Engine DJ from "coming soon" to shipped and add a Lexicon option. Both download the existing Rekordbox DJ_PLAYLISTS XML (neither platform has a proprietary import format), so they share one download path with rekordbox via XML_PLATFORMS, plus an in-modal import-then-relink note. Regenerates api-types.generated.ts so the widened export Literals reach the frontend types. 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesEngine DJ + Lexicon Export Targets
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
dashboard/app/(dj)/setbuilder/components/__tests__/ExportModal.test.tsx (1)
444-451: 💤 Low valueConsider extending the relink-note test to cover both targets.
The relink note test only verifies
enginedj. Since the UI condition isplatformId !== 'rekordbox', bothenginedjandlexiconshould show the note. For completeness, consider parameterizing this assertion or adding a second case forlexicon.♻️ Optional: parameterize the relink-note assertion
- it('shows the import-then-relink note for these targets', async () => { - mockApi.exportPreflight.mockResolvedValue(makePreflightClean('enginedj')); - render(<ExportModal {...baseProps} />); - fireEvent.click(screen.getByText('Engine DJ XML')); - await waitFor(() => { - expect(screen.getByText(/relink/i)).toBeTruthy(); - }); - }); + it.each([ + ['Engine DJ XML', 'enginedj'], + ['Lexicon', 'lexicon'], + ])('%s shows the import-then-relink note', async (label, format) => { + mockApi.exportPreflight.mockResolvedValue( + makePreflightClean(format as ExportPreflight['target']) + ); + render(<ExportModal {...baseProps} />); + fireEvent.click(screen.getByText(label)); + await waitFor(() => { + expect(screen.getByText(/relink/i)).toBeTruthy(); + }); + });🤖 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 `@dashboard/app/`(dj)/setbuilder/components/__tests__/ExportModal.test.tsx around lines 444 - 451, The test "shows the import-then-relink note for these targets" only verifies the relink note displays for the 'enginedj' target, but since the UI condition checks platformId !== 'rekordbox', both 'enginedj' and 'lexicon' should show this note. Extend the test to cover both targets by either parameterizing the test to run the assertion for each target separately, or adding a second test case that clicks on the lexicon option and verifies the relink note appears for that target as well.
🤖 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 `@server/tests/test_setbuilder_export_api.py`:
- Around line 283-292: The test_enginedj_export_does_not_mutate_status method
posts an export request to the client but does not validate that the request
succeeded before checking the status. Capture the response from the client.post
call and assert that it returns a successful status code (such as 200) before
proceeding with the db.expire_all() and the assertion that verifies the Set
status remains as "draft". This ensures the test is actually validating the
correct behavior and not passing due to a failed request.
---
Nitpick comments:
In `@dashboard/app/`(dj)/setbuilder/components/__tests__/ExportModal.test.tsx:
- Around line 444-451: The test "shows the import-then-relink note for these
targets" only verifies the relink note displays for the 'enginedj' target, but
since the UI condition checks platformId !== 'rekordbox', both 'enginedj' and
'lexicon' should show this note. Extend the test to cover both targets by either
parameterizing the test to run the assertion for each target separately, or
adding a second test case that clicks on the lexicon option and verifies the
relink note appears for that target as well.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b8ee6e1-b9ed-418c-9237-3ac16c830e14
📒 Files selected for processing (11)
dashboard/app/(dj)/setbuilder/components/ExportModal.tsxdashboard/app/(dj)/setbuilder/components/__tests__/ExportModal.test.tsxdashboard/lib/api-types.generated.tsdocs/superpowers/plans/2026-06-17-issue-401-engine-lexicon-export.mddocs/superpowers/specs/2026-06-17-issue-401-design.mdserver/app/api/setbuilder.pyserver/app/schemas/setbuilder.pyserver/app/services/setbuilder/export_files.pyserver/openapi.jsonserver/tests/test_setbuilder_export_api.pyserver/tests/test_setbuilder_export_service.py
- Assert export POST returns 200 before checking the set status stays "draft", so the no-mutation test cannot pass on a failed request. - Parameterize the import-then-relink note test over both Engine DJ and Lexicon, since the note renders for every non-rekordbox XML target. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
CodeRabbit body nitpick addressed (🧹 |
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.
Why
Issue #401 asks for Engine DJ and Lexicon as export targets. Research
(captured in the design doc) established that neither platform has a
proprietary import format — both ingest the Rekordbox
DJ_PLAYLISTSXML WrzDJ already generates. So the honest, low-risk shipping path is
labeled presets of the existing renderer, not two new serializers. The
one real gap surfaced along the way: ISRC had no slot in
DJ_PLAYLISTSand was silently dropped on every export.
What
Rekordbox XML renderer now emits
track.isrcvia the TRACKCommentsattribute as
ISRC:<isrc>, run through the existing control-charsanitizer. Absent ISRC omits
Comments.Literals with distinctenginedj/lexiconkeys (not aliases — so theUI lists them separately and future per-platform tweaks don't churn the
contract) and mapped both to
render_rekordbox_xml(
application/xml,.xml).is added; both download the Rekordbox XML through one shared
XML_PLATFORMSblock, with an in-modal import-then-relink note.server/openapi.jsonanddashboard/lib/api-types.generated.ts.Out of scope (documented as non-viable in the design): Engine Library
SQLite DB writes (Denon-discouraged) and the Lexicon Local API
(localhost / unauth / read-mostly).
Testing
pytest— 2891 passed, coverage 88% ≥ 85% gate)Comments, omitted when absent, control chars sanitizedenginedj/lexiconroute to the XML renderer (200,application/xml,.xml); preflight uses the file branch; export does not mutate set statusvitest— 1264 passed); ExportModal picker shows 8 rows / 3 "coming soon", Engine DJ + Lexicon download.xmland show the relink noteruff check/ruff format/banditclean;npm run lint/tsc --noEmitcleanalembic upgrade head && alembic check— no new migration, no drift.xmlinto current Engine DJ (Database tab → set Rekordbox XML path → Import) and Lexicon (import Rekordbox XML), confirm the playlist + metadata land and tracks relink to local audio🤖 Co-authored by Claude Opus 4.8. Closes #401.
Summary by CodeRabbit