Add clip editing, mixer controls, scene launch, undo, and audio track support#84
Open
LofiFren wants to merge 3 commits intoahujasid:mainfrom
Open
Add clip editing, mixer controls, scene launch, undo, and audio track support#84LofiFren wants to merge 3 commits intoahujasid:mainfrom
LofiFren wants to merge 3 commits intoahujasid:mainfrom
Conversation
… support New MCP tools and Remote Script handlers: Clip editing: - get_clip_notes: Read all MIDI notes back from a clip - remove_notes_from_clip: Remove notes by range (or all notes) - delete_clip: Delete a clip from a slot entirely - duplicate_clip_to: Copy a clip to another slot on the same track Mixer controls: - set_track_volume: Set track volume (0.0-1.0) - set_track_pan: Set track panning (-1.0 to 1.0) - set_track_send: Set send amounts to return tracks Session management: - fire_scene: Launch an entire scene row across all tracks - create_audio_track: Create audio tracks (was referenced but unimplemented) - undo: Undo the last action in Ableton These additions address workflow gaps discovered while using this already awesome MCP, where inability to edit notes, clear clips, or control mixing forced unnecessary workarounds.
Review Summary by QodoAdd clip editing, mixer controls, and session management tools
WalkthroughsDescription• Add 10 new MCP tools for clip editing, mixer control, and session management • Implement get_clip_notes, remove_notes_from_clip, delete_clip, duplicate_clip_to for clip manipulation • Implement set_track_volume, set_track_pan, set_track_send for mixer control • Implement fire_scene, create_audio_track, undo for session management • Update command routing to schedule all new operations on main thread Diagramflowchart LR
MCP["MCP Server Tools"]
Remote["Remote Script Handlers"]
Ableton["Ableton Live API"]
MCP -->|get_clip_notes| Remote
MCP -->|remove_notes_from_clip| Remote
MCP -->|delete_clip| Remote
MCP -->|duplicate_clip_to| Remote
MCP -->|set_track_volume| Remote
MCP -->|set_track_pan| Remote
MCP -->|set_track_send| Remote
MCP -->|fire_scene| Remote
MCP -->|create_audio_track| Remote
MCP -->|undo| Remote
Remote -->|clip operations| Ableton
Remote -->|mixer operations| Ableton
Remote -->|session operations| Ableton
File Changes1. AbletonMCP_Remote_Script/__init__.py
|
Code Review by Qodo
1.
|
📝 WalkthroughWalkthroughAdds 10 Ableton command handlers to the remote script (including clip note retrieval and various clip/track/session mutators), updates main-thread scheduling and timeout cancellation behavior, and exposes 10 corresponding MCP server endpoints that call the remote script and return JSON or confirmation strings. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as MCP Server
participant Remote as Remote Script
participant Ableton as Ableton Session
Client->>Server: HTTP RPC / tool call (command, params)
Server->>Remote: ableton.send_command(command, params)
alt read-only (get_clip_notes)
Remote->>Ableton: direct read (get notes)
else mutating command
Remote->>Remote: enqueue for main-thread execution
Remote->>Remote: wait for worker / handle queue timeout (set cancelled)
Remote->>Ableton: execute handler on main thread (remove/delete/duplicate/set/fire/create/undo)
end
Ableton-->>Remote: return result
Remote-->>Server: forward formatted result
Server->>Client: respond (JSON or confirmation)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@MCP_Server/server.py`:
- Around line 108-111: The list of state-modifying commands in server.py
incorrectly includes "get_clip_notes", causing unnecessary delay and longer
timeout; remove "get_clip_notes" from the is_modifying_command list (the array
that currently contains "start_playback", "stop_playback", ..., "undo") so it is
treated as a read-only command, and run/adjust any related tests or callers that
assume it was modifying state to ensure no other logic depends on its presence
in that list.
🪄 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
Run ID: 555970b1-154d-425d-bd68-201a20f5cd18
📒 Files selected for processing (2)
AbletonMCP_Remote_Script/__init__.pyMCP_Server/server.py
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
…ect execution Thanks for the thoughtful feedback on the timeout race condition! 1. Move get_clip_notes to direct execution block (alongside get_session_info and get_track_info) since it is a read-only operation that does not need main-thread scheduling. 2. Add cancellation flag to main-thread scheduled tasks. When the 10s wait times out, the flag prevents the queued callback from executing late. This avoids the scenario where a destructive command (delete_clip, remove_notes, undo) runs after the client already received a timeout error and potentially retried. 3. Remove get_clip_notes from is_modifying_command in server.py so reads do not incur the artificial delay meant for state-modifying operations.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MCP_Server/server.py (1)
104-111:⚠️ Potential issue | 🟠 MajorClassify
load_browser_itemas the modifying wire command here.The tool at Lines 423-426 sends
load_browser_item, but this list still marksload_instrument_or_effectinstead. That means browser loads still take the read-only path here and miss the extra delay/timeout budget intended for main-thread mutations.🔧 Suggested fix
is_modifying_command = command_type in [ "create_midi_track", "create_audio_track", "set_track_name", "create_clip", "add_notes_to_clip", "set_clip_name", "set_tempo", "fire_clip", "stop_clip", "set_device_parameter", - "start_playback", "stop_playback", "load_instrument_or_effect", + "start_playback", "stop_playback", "load_browser_item", "remove_notes_from_clip", "delete_clip", "duplicate_clip_to", "set_track_volume", "set_track_pan", "set_track_send", "fire_scene", "undo" ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCP_Server/server.py` around lines 104 - 111, The list computing is_modifying_command is missing "load_browser_item", causing browser loads to be treated as read-only; update the array in the is_modifying_command expression (the list that currently includes "load_instrument_or_effect") to also include "load_browser_item" so that commands sent by send/load_browser_item take the modifying path and get the extra delay/timeout budget.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AbletonMCP_Remote_Script/__init__.py`:
- Around line 243-252: The current cancellation flag (cancelled = [False]) and
the pre-start check inside main_thread_task incorrectly report a timed-out
command as "cancelled" even if it started; change to a three-state tracker
(e.g., state = ["pending"/"running"/"finished"]) alongside cancelled, set state
to "running" immediately at the start of main_thread_task and to "finished" at
the end, and update the timeout/handler logic that currently flips cancelled to
only mark and log a cancellation when state == "pending" (leave commands that
reached "running" or "finished" reported differently or not marked cancelled).
Ensure you update any logging that used cancelled[0] to consult the new state
variable and reference the existing symbols main_thread_task and cancelled to
locate where to modify the timeout handling and pre-start checks.
---
Outside diff comments:
In `@MCP_Server/server.py`:
- Around line 104-111: The list computing is_modifying_command is missing
"load_browser_item", causing browser loads to be treated as read-only; update
the array in the is_modifying_command expression (the list that currently
includes "load_instrument_or_effect") to also include "load_browser_item" so
that commands sent by send/load_browser_item take the modifying path and get the
extra delay/timeout budget.
🪄 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
Run ID: 622be003-1f68-44df-803f-b01aa4bddc9d
📒 Files selected for processing (2)
AbletonMCP_Remote_Script/__init__.pyMCP_Server/server.py
Comment on lines
+243
to
+252
| # Cancellation flag: if the wait times out, prevent late execution | ||
| # of destructive operations that haven't started yet | ||
| cancelled = [False] | ||
|
|
||
| # Define a function to execute on the main thread | ||
| def main_thread_task(): | ||
| try: | ||
| if cancelled[0]: | ||
| self.log_message("Skipping cancelled command: " + command_type) | ||
| return |
There was a problem hiding this comment.
Don’t report a timed-out command as cancelled unless it never started.
Line 250 only checks cancelled[0] before the handler begins. If the main-thread task starts just before Line 360 times out, the Ableton mutation will still run, but the client gets told it was cancelled and may safely retry a destructive command. Track pending/running/finished state and only use the cancellation wording when the task was still pending at timeout.
Also applies to: 360-364
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AbletonMCP_Remote_Script/__init__.py` around lines 243 - 252, The current
cancellation flag (cancelled = [False]) and the pre-start check inside
main_thread_task incorrectly report a timed-out command as "cancelled" even if
it started; change to a three-state tracker (e.g., state =
["pending"/"running"/"finished"]) alongside cancelled, set state to "running"
immediately at the start of main_thread_task and to "finished" at the end, and
update the timeout/handler logic that currently flips cancelled to only mark and
log a cancellation when state == "pending" (leave commands that reached
"running" or "finished" reported differently or not marked cancelled). Ensure
you update any logging that used cancelled[0] to consult the new state variable
and reference the existing symbols main_thread_task and cancelled to locate
where to modify the timeout handling and pre-start checks.
LofiFren
commented
Mar 27, 2026
Author
LofiFren
left a comment
LofiFren
left a comment
There was a problem hiding this comment.
Thanks for the great feedback! Pushed a fix
The MCP server tool sends 'load_browser_item' as the wire command but the is_modifying_command list had 'load_instrument_or_effect' instead, causing browser loads to miss the extra delay and timeout budget meant for state-modifying operations. This was a pre-existing issue, not introduced by this PR.
Author
|
Thanks for the thorough review! Fixed the Regarding the suggestion to replace the boolean We're intentionally keeping the simple boolean. Here's the reasoning:
Simple code that does the job > clever code that handles theoretical scenarios. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
MCP_Server/server.py (1)
522-524: Consider using explicit conversion flags (file-wide).Static analysis flags
str(e)in f-strings across all new functions. The idiomatic form isf"{e!s}". However, this pattern is consistent with all existing tool endpoints in this file, so changing only the new code would introduce inconsistency.If you want to address this, it should be done file-wide as a separate cleanup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCP_Server/server.py` around lines 522 - 524, The f-strings in the exception handler use str(e) which static analysis flags; update the file to consistently use the explicit conversion flag f"{e!s}" instead of f"{str(e)}" (e.g., the logger.error and return statements that currently contain "Error getting clip notes: {str(e)}"), but do this as a file-wide cleanup so the pattern stays consistent across all endpoints in MCP_Server/server.py rather than changing only this new handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@MCP_Server/server.py`:
- Around line 522-524: The f-strings in the exception handler use str(e) which
static analysis flags; update the file to consistently use the explicit
conversion flag f"{e!s}" instead of f"{str(e)}" (e.g., the logger.error and
return statements that currently contain "Error getting clip notes: {str(e)}"),
but do this as a file-wide cleanup so the pattern stays consistent across all
endpoints in MCP_Server/server.py rather than changing only this new handler.
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
get_clip_notes,remove_notes_from_clip,delete_clip,duplicate_clip_to- enables reading notes back, clearing/editing clips, and duplicating for variationsset_track_volume,set_track_pan,set_track_send- allows mixing directly via MCPfire_scene(launch entire scene rows),create_audio_track(was referenced but unimplemented),undoThese 11 new tools address workflow gaps discovered while using this already awesome MCP for composition sessions. The biggest pain points were:
All new handlers follow the existing patterns: Remote Script methods with main-thread scheduling, MCP server tool endpoints with proper error handling, and command routing in both
_process_commandandis_modifying_command.Test plan
get_clip_notesreturns correct note data from an existing clipremove_notes_from_clipclears all notes (default params) and targeted rangesdelete_clipremoves a clip and leaves the slot emptyduplicate_clip_tocopies clip to an empty target slotset_track_volumeandset_track_panupdate mixer valuesset_track_sendroutes to return tracksfire_scenelaunches all clips in a scene rowcreate_audio_trackcreates a track (was previously a dead code path)undoreverses the last actionSummary by CodeRabbit