Skip to content

fix(editor): restore from-scratch create — artist and audio are optional - #174

Merged
byrongamatos merged 1 commit into
mainfrom
fix/blank-create-shadowed-by-legacy
Jul 9, 2026
Merged

byrongamatos merged 1 commit into
mainfrom
fix/blank-create-shadowed-by-legacy

Conversation

@byrongamatos

@byrongamatos byrongamatos commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to the duplicate-function bug found in #173.

The symptom

Type a title. The Create button enables. Click it:

Artist is required.

Supply an artist:

Audio is required for an audio-only project.

So the advertised draft-now, audio-later flow was impossible. And even when it did succeed, the roster chips (“What are you arranging?”) were ignored — every draft came out as a lone Lead arrangement — and the eight extended-metadata fields the modal collects (album artist, track, disc, genres, language, ISRC, MBID, authors) were silently dropped.

Root cause

main.js defined _editorDoBlankCreate twice. Function declarations hoist, and the last one in source order silently wins, so PR #45’s redesign never ran. The older definition kept sending the server’s documented back-compat payload (initial_arrangement + init_drum_tab) instead of the roster the server asks for.

Why this is a bug fix, not a product call

Everything else in the system already agreed. Only the handler disagreed:

says
_createGateOpen (unit-tested) enables Create on a title + one instrument
screen.html marks only Title (required)
editorDoCreate, right above the call “only a title is required; audio + artist are optional”
create_sloppak (routes.py) “title required” · “Artist is OPTIONAL for a draft” · “Draft-now, audio-later: audio is OPTIONAL” · calls initial_arrangement/init_drum_tab the “Legacy shape”

The fix

Validate a title and a non-empty instrument roster. Artist and audio optional. Send arrangements plus the spec-complete metadata through the same _createExtendedMeta() helper the Guitar Pro and EOF paths already use.

  • Kept from the old function: the art-upload retry (art normally uploads on selection; this covers a selection whose upload failed).
  • Added: the roster check now re-enables the button on failure — the redesign’s version forgot to.
  • Removed: createState.initialArrangement and createState.initDrumTab, which now have no reader. The separate, older editorShowCreateSloppakModal dialog still sends the legacy shape with its own drum checkbox, and the server still accepts it. Untouched.

A Codex finding I did not take

Codex reported a regression: the old payload defaulted init_drum_tab to true, so a default create used to seed Drums too.

It reads that way — but the line below the default overwrites it:

initDrumTab: true,                                                    // line 49
createState.initDrumTab = !!document.getElementById(editor-create-drum-tab)?.checked;   // line 2092

#editor-create-drum-tab was deleted from screen.html by the same redesign. Verified against the live DOM: the element does not exist, the expression is false, and the server appended nothing. The default roster was [Lead] before this change and is [Lead] after it. No behaviour lost.

Verification

verify_blank_create.py drives the real modal and reads the real POST body.

check fixed pre-fix
the gate enables Create with a title only PASS PASS
the handler agrees and POSTs PASS FAIL (Artist is required.)
no artist sent or demanded PASS FAIL
no audio required (draft-now) PASS FAIL
roster chips arrive as arrangements PASS FAIL
no back-compat keys sent PASS FAIL
extended metadata travels (genres) PASS FAIL

End to end, with a title and Lead + Drums chosen, the server writes an audio-less draft:

manifest.yaml     artist:    stems: []   genres: [Progressive]
                  arrangements: [lead, drums]
drum_tab.json
arrangements/lead.json
arrangements/drums.json

node --test 89/89 · pytest 248/248 · npm run lint 0 errors · all 16 headless harnesses pass.

cc @ChrisBeWithYou — this restores your #45 work.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Create draft projects with a title and at least one instrument role, without requiring audio.
    • Selected album artwork is uploaded automatically during creation when needed.
    • Project rosters and extended metadata are now preserved when creating a song.
  • Bug Fixes

    • Fixed from-scratch song creation sending incomplete or incorrect project details.
    • Improved creation reliability when audio is added later or artwork uploads require a retry.
  • Documentation

    • Updated the changelog with details about the creation-flow corrections.

Typing a title enabled the Create button. Clicking it said "Artist is required."
Supplying an artist then said "Audio is required for an audio-only project",
which made the advertised draft-now-audio-later flow impossible. The roster
chips were ignored — every draft came out as a lone Lead arrangement — and the
eight extended-metadata fields the modal collects were never sent.

Root cause is the duplicate `_editorDoBlankCreate` found while extracting
src/create.js (#173): main.js defined it twice, function declarations hoist, and
the LAST one in source order silently won. So PR #45's redesign never ran, and
the older definition kept sending the server's documented BACK-COMPAT payload
(`initial_arrangement` + `init_drum_tab`) instead of the roster it asks for.

Everything else already agreed, which is what makes this unambiguous rather than
a product call:

  _createGateOpen   enables Create on a title + one instrument (and is unit-tested)
  screen.html       marks only Title "(required)"
  editorDoCreate    comments, right above the call: "only a title is required;
                    audio + artist are optional"
  create_sloppak    "title required"; "Artist is OPTIONAL for a draft"; "Draft-now,
                    audio-later: audio is OPTIONAL"; and it calls
                    initial_arrangement/init_drum_tab the "Legacy shape"

Only the handler disagreed. It now validates a title and a non-empty instrument
roster, treats artist and audio as optional, and sends `arrangements` plus the
spec-complete metadata via the same _createExtendedMeta() helper the Guitar Pro
and EOF paths already use.

Kept from the old function: the art-upload retry (art normally uploads on
selection; this covers a selection whose upload failed). Added: the roster
check re-enables the button on failure, which the redesign's version forgot.

Removed createState.initialArrangement and createState.initDrumTab — nothing
reads them now. The separate, older editorShowCreateSloppakModal dialog still
sends the legacy shape with its own drum checkbox, and the server still accepts
it; untouched.

Codex flagged one apparent regression: that the old payload defaulted
init_drum_tab to true, so a default create used to seed Drums as well. It reads
that way, but the line below the default overwrites it with a lookup for
`#editor-create-drum-tab` — an element the same redesign deleted from
screen.html. Verified against the live DOM: the element does not exist, the
expression is false, and the server appended nothing. The default roster was
['Lead'] before this change and is ['Lead'] after it.

verify_blank_create.py drives the real modal and reads the real POST body: a
title alone POSTs, no artist and no audio are sent or demanded, the roster chips
arrive as `arrangements`, no back-compat keys are sent, and the extended fields
travel. It fails on 6 of 8 checks against the pre-fix code. End to end the
server writes an audio-less draft (`stems: []`) with arrangements/lead.json,
arrangements/drums.json and drum_tab.json when Drums is chosen.

node --test 89/89, pytest 248/248, npm run lint 0 errors, all 16 headless
harnesses pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 9, 2026 23:23
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e1525651-df9c-40cb-9ef6-c1bf5c44c372

📥 Commits

Reviewing files that changed from the base of the PR and between f9fbf78 and f14b261.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • src/create.js

📝 Walkthrough

Walkthrough

Changes

Blank creation flow

Layer / File(s) Summary
Remove legacy arrangement state
src/create.js
Legacy arrangement and string-count state is removed, with comments documenting the roster-based payload flow.
Update blank creation request
src/create.js, CHANGELOG.md
Blank creation now requires a title and instrument role, permits missing audio, retries album-art uploads, and sends roster plus extended metadata through create_sloppak. The changelog records the corrected duplicate-handler regression.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CreateUI
  participant BlankCreate
  participant UploadArt
  participant CreateSloppak
  CreateUI->>BlankCreate: Submit title and roster
  BlankCreate->>UploadArt: Retry album-art upload when artPath is unset
  UploadArt-->>BlankCreate: Return art_path
  BlankCreate->>CreateSloppak: Send roster and extended metadata
  CreateSloppak-->>CreateUI: Complete blank creation
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing editor blank-create flow and making artist/audio optional.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/blank-create-shadowed-by-legacy

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the “Blank / from-scratch” create flow in src/create.js so it matches the intended draft workflow: only a title + non-empty instrument roster are required, while artist and audio are optional. It also restores sending the roster-based arrangements payload and forwards the modal’s extended metadata via the shared helper, aligning the client with the backend contract.

Changes:

  • Remove obsolete blank-create state (initialArrangement, initDrumTab) and stop sending legacy back-compat keys.
  • Update _editorDoBlankCreate validation and payload to (a) require title + at least one instrument, (b) allow missing artist/audio, and (c) send arrangements plus extended metadata.
  • Document the fix in CHANGELOG.md.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/create.js Fixes blank-create validation and payload to support draft-now (artist/audio optional) and send roster-based arrangements + extended metadata.
CHANGELOG.md Adds a detailed Unreleased “Fixed” entry describing the restored blank-create behavior and root cause.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@byrongamatos
byrongamatos merged commit 429789c into main Jul 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants