feat: add conditioning modes to the VST3 workflow#913
Closed
ChuxiJ wants to merge 1 commit intocodex/v2-906-workflow-modelfrom
Closed
feat: add conditioning modes to the VST3 workflow#913ChuxiJ wants to merge 1 commit intocodex/v2-906-workflow-modelfrom
ChuxiJ wants to merge 1 commit intocodex/v2-906-workflow-modelfrom
Conversation
Contributor
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughThis PR implements workflow-mode-aware generation and a V2 UI redesign for the ACE-Step VST3 plugin: introduces WorkflowMode and TransportState enums, persists mode/session/result metadata, adds new V2 look/ chrome/ UI components (status strip, synth panel, transport, result/preview decks), wires mode-specific validation in the processor, and builds mode-aware backend request payloads. Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor
participant Processor
participant Backend
User->>Editor: Select WorkflowMode / populate inputs
Editor->>Editor: update UI fields & persist state
User->>Editor: Click Render / Generate
Editor->>Processor: requestGeneration()
Processor->>Processor: validate by WorkflowMode
alt Validation fails
Processor->>Editor: set jobStatus/transportState = failed, set errorMessage
else Validation succeeds
Processor->>Processor: clear previous results, set transportState=submitting
Processor->>Backend: startGeneration(payload based on WorkflowMode)
Backend->>Backend: build payload (prompt, task_type, reference/src paths, codes, cover strength)
Backend-->>Processor: submit / poll / final result callbacks
Processor->>Editor: update state (resultTakes, transportState, session fields)
end
Editor->>User: update UI/status/preview/result deck
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/acestep_vst3/src/PluginEditor.cpp (1)
57-57:⚠️ Potential issue | 🟡 MinorRemove redundant no-op line.
This line sets
promptEditor_bounds to its own current bounds, which has no effect. This appears to be leftover from editing.🧹 Proposed fix
lyricsLabel_.setBounds(left.removeFromTop(18)); - promptEditor_.setBounds(promptEditor_.getBounds()); lyricsEditor_.setBounds(left.removeFromTop(44));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/acestep_vst3/src/PluginEditor.cpp` at line 57, Remove the redundant no-op call to promptEditor_.setBounds(promptEditor_.getBounds()); in PluginEditor.cpp — simply delete that line where promptEditor_ is being set to its own bounds (occurrence using promptEditor_.setBounds and promptEditor_.getBounds) so the code no longer contains the ineffective self-assignment.
🧹 Nitpick comments (1)
plugins/acestep_vst3/src/PluginEditorState.cpp (1)
151-208: Add a round-trip regression test for the new mode sync.
syncFromProcessor()andpersistTextFields()now have to keep combo ordering,WorkflowMode, and persisted state aligned for four modes plusaudioCoverStrength. A focused test that exercises all four modes through both directions would catch silent session-restore regressions early. Based on learnings, AI-Agent Workflow: Add/update focused tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/acestep_vst3/src/PluginEditorState.cpp` around lines 151 - 208, Add a round-trip unit test that verifies syncFromProcessor() and persistTextFields() keep combo ordering and WorkflowMode in sync for all four modes and audioCoverStrength: for each WorkflowMode value set processor_.getMutableState() fields (workflowMode, audioCoverStrength, modelPreset, qualityMode, durationSeconds, seed, backendBaseUrl, prompt, lyrics, referenceAudioPath, sourceAudioPath, customConditioningCodes), call ACEStepVST3AudioProcessorEditor::syncFromProcessor(), then immediately call persistTextFields() and assert the processor state equals the original (including workflowMode and audioCoverStrength); also add the inverse case where you set GUI controls (modeBox_.setSelectedId / coverStrengthSlider_.setValue etc.), call persistTextFields(), construct a fresh processor state from that persisted state and call syncFromProcessor() and assert UI reflects the same values—use the same helper symbols (syncFromProcessor, persistTextFields, modeBox_, workflowMode, coverStrengthSlider_) and the project test harness for setup/teardown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/acestep_vst3/src/PluginEditorPreview.cpp`:
- Around line 9-18: The shared helper launchAudioChooser currently allows
multiple audio formats but conditioning/reference pickers must be WAV-only;
modify launchAudioChooser to accept an allowedExtensions parameter (or add a new
launchWavChooser wrapper) and use "*.wav" for conditioning/reference pickers,
then update the calls that create conditioning/source file choosers (the
occurrences referenced in PluginEditorPreview.cpp) to pass/ call the
WAV-restricted variant so only .wav files are selectable.
In `@plugins/acestep_vst3/src/PluginEditorState.cpp`:
- Around line 191-193: The code is currently persisting machine-local absolute
paths by assigning state.referenceAudioPath =
referenceAudioEditor_.getText().trim() and state.sourceAudioPath =
sourceAudioEditor_.getText().trim(); instead, convert these absolute file paths
into project-relative/imported asset references or avoid serializing them:
detect if the chosen file is inside the project/asset folder and store a
relative path or asset ID (e.g., transform referenceAudioEditor_.getText() into
a project-relative path or lookup an AssetRegistry entry), and if not, leave
state.referenceAudioPath/state.sourceAudioPath empty or mark them session-only
so they are not written to the saved plugin XML; keep
state.customConditioningCodes as-is if intended to be serializable.
---
Outside diff comments:
In `@plugins/acestep_vst3/src/PluginEditor.cpp`:
- Line 57: Remove the redundant no-op call to
promptEditor_.setBounds(promptEditor_.getBounds()); in PluginEditor.cpp — simply
delete that line where promptEditor_ is being set to its own bounds (occurrence
using promptEditor_.setBounds and promptEditor_.getBounds) so the code no longer
contains the ineffective self-assignment.
---
Nitpick comments:
In `@plugins/acestep_vst3/src/PluginEditorState.cpp`:
- Around line 151-208: Add a round-trip unit test that verifies
syncFromProcessor() and persistTextFields() keep combo ordering and WorkflowMode
in sync for all four modes and audioCoverStrength: for each WorkflowMode value
set processor_.getMutableState() fields (workflowMode, audioCoverStrength,
modelPreset, qualityMode, durationSeconds, seed, backendBaseUrl, prompt, lyrics,
referenceAudioPath, sourceAudioPath, customConditioningCodes), call
ACEStepVST3AudioProcessorEditor::syncFromProcessor(), then immediately call
persistTextFields() and assert the processor state equals the original
(including workflowMode and audioCoverStrength); also add the inverse case where
you set GUI controls (modeBox_.setSelectedId / coverStrengthSlider_.setValue
etc.), call persistTextFields(), construct a fresh processor state from that
persisted state and call syncFromProcessor() and assert UI reflects the same
values—use the same helper symbols (syncFromProcessor, persistTextFields,
modeBox_, workflowMode, coverStrengthSlider_) and the project test harness for
setup/teardown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c874025f-e29c-4f7a-8fce-c959ffa763c0
📒 Files selected for processing (10)
plugins/acestep_vst3/src/PluginBackendClient.cppplugins/acestep_vst3/src/PluginEditor.cppplugins/acestep_vst3/src/PluginEditor.hplugins/acestep_vst3/src/PluginEditorPreview.cppplugins/acestep_vst3/src/PluginEditorState.cppplugins/acestep_vst3/src/PluginEnums.cppplugins/acestep_vst3/src/PluginEnums.hplugins/acestep_vst3/src/PluginProcessor.cppplugins/acestep_vst3/src/PluginState.cppplugins/acestep_vst3/src/PluginState.h
Comment on lines
+9
to
+18
| void launchAudioChooser(std::unique_ptr<juce::FileChooser>& chooser, | ||
| const juce::String& title, | ||
| std::function<void(const juce::FileChooser&)> onComplete) | ||
| { | ||
| chooser = std::make_unique<juce::FileChooser>( | ||
| title, juce::File(), "*.wav;*.aiff;*.flac;*.ogg;*.mp3;*.m4a"); | ||
| chooser->launchAsync(juce::FileBrowserComponent::openMode | ||
| | juce::FileBrowserComponent::canSelectFiles, | ||
| std::move(onComplete)); | ||
| } |
Contributor
There was a problem hiding this comment.
Restrict conditioning pickers to WAV.
Issue #907 scopes the conditioning modes as raw WAV inputs, but this shared helper now exposes AIFF/FLAC/OGG/MP3/M4A for the reference/source pickers too. That widens the UI contract beyond what these workflows are supposed to accept.
🎯 Proposed split between preview and conditioning filters
void launchAudioChooser(std::unique_ptr<juce::FileChooser>& chooser,
const juce::String& title,
+ const juce::String& wildcard,
std::function<void(const juce::FileChooser&)> onComplete)
{
- chooser = std::make_unique<juce::FileChooser>(
- title, juce::File(), "*.wav;*.aiff;*.flac;*.ogg;*.mp3;*.m4a");
+ chooser = std::make_unique<juce::FileChooser>(title, juce::File(), wildcard);
chooser->launchAsync(juce::FileBrowserComponent::openMode
| juce::FileBrowserComponent::canSelectFiles,
std::move(onComplete));
}
...
launchAudioChooser(previewChooser_,
"Select preview audio file",
+ "*.wav;*.aiff;*.flac;*.ogg;*.mp3;*.m4a",
[this](const juce::FileChooser& chooser) {
...
launchAudioChooser(conditioningChooser_,
"Select reference audio file",
+ "*.wav",
[this](const juce::FileChooser& chooser) {
...
launchAudioChooser(conditioningChooser_,
"Select source audio file",
+ "*.wav",
[this](const juce::FileChooser& chooser) {Also applies to: 23-24, 66-67, 92-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/acestep_vst3/src/PluginEditorPreview.cpp` around lines 9 - 18, The
shared helper launchAudioChooser currently allows multiple audio formats but
conditioning/reference pickers must be WAV-only; modify launchAudioChooser to
accept an allowedExtensions parameter (or add a new launchWavChooser wrapper)
and use "*.wav" for conditioning/reference pickers, then update the calls that
create conditioning/source file choosers (the occurrences referenced in
PluginEditorPreview.cpp) to pass/ call the WAV-restricted variant so only .wav
files are selectable.
Comment on lines
+191
to
+193
| state.referenceAudioPath = referenceAudioEditor_.getText().trim(); | ||
| state.sourceAudioPath = sourceAudioEditor_.getText().trim(); | ||
| state.customConditioningCodes = conditioningCodesEditor_.getText(); |
Contributor
There was a problem hiding this comment.
Don't persist machine-local conditioning paths verbatim.
These assignments feed PluginState.cpp, which now serializes the strings into the saved plugin XML. For a DAW recall path, that makes the new reference/source workflows brittle across machines and embeds user-local directory names in project/preset files. Please persist a project-relative/imported asset reference instead, or keep these fields session-local.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/acestep_vst3/src/PluginEditorState.cpp` around lines 191 - 193, The
code is currently persisting machine-local absolute paths by assigning
state.referenceAudioPath = referenceAudioEditor_.getText().trim() and
state.sourceAudioPath = sourceAudioEditor_.getText().trim(); instead, convert
these absolute file paths into project-relative/imported asset references or
avoid serializing them: detect if the chosen file is inside the project/asset
folder and store a relative path or asset ID (e.g., transform
referenceAudioEditor_.getText() into a project-relative path or lookup an
AssetRegistry entry), and if not, leave
state.referenceAudioPath/state.sourceAudioPath empty or mark them session-only
so they are not written to the saved plugin XML; keep
state.customConditioningCodes as-is if intended to be serializable.
6 tasks
ChuxiJ
force-pushed
the
codex/v2-907-conditioning
branch
from
d62dea7 to
eec9d82
Compare
This was referenced Mar 20, 2026
Contributor
Author
|
Superseded by consolidated V2 implementation PR #919, which now carries this stack forward against . Closing this split PR to keep the review surface clean. |
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.
Summary
Validation
Closes #907
Refs #903
Summary by CodeRabbit
New Features
Bug Fixes / Validation