Feature Request: Add ability to read MIDI notes from existing clips #73
Feature Request: Add ability to read MIDI notes from existing clips #73Jholl6 wants to merge 1 commit intoahujasid:mainfrom
Conversation
Review Summary by QodoAdd ability to read MIDI notes from existing clips
WalkthroughsDescription• Add get_clip_notes function to read MIDI notes from existing clips • Extract note data including pitch, timing, velocity, and mute status • Retrieve clip metadata: length, quantization, and scale information • Integrate command into main thread task queue for thread-safe execution Diagramflowchart LR
A["MCP Server Request"] -- "get_clip_notes command" --> B["Remote Script Handler"]
B -- "validate track/clip indices" --> C["Retrieve Clip Object"]
C -- "verify MIDI clip" --> D["Extract Note Data"]
D -- "gather metadata" --> E["Return Notes + Metadata"]
E -- "JSON response" --> F["MCP Tool Result"]
File Changes1. AbletonMCP_Remote_Script/__init__.py
|
📝 WalkthroughWalkthroughThis pull request introduces a new "get_clip_notes" feature that retrieves MIDI note data from specified clips in Ableton Live. The implementation spans the remote script backend for clip note extraction and a corresponding MCP server tool function to expose this capability. Changes
Sequence DiagramsequenceDiagram
participant Client as Client (MCP)
participant Server as MCP Server
participant Remote as Remote Script
participant Ableton as Ableton Live
Client->>Server: Call get_clip_notes(track_index, clip_index)
Server->>Remote: Send "get_clip_notes" command with parameters
activate Remote
Remote->>Remote: Validate track and clip indices
Remote->>Ableton: Query MIDI clip at track/clip position
Ableton-->>Remote: Return clip data (notes, length, quantization)
Remote->>Remote: Extract and structure note information
Remote-->>Server: Return notes, clip_length, quantization, scale_info
deactivate Remote
Server->>Server: Format result as JSON
Server-->>Client: Return pretty-printed JSON response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
Code Review by Qodo
1. get_clip_notes returns raw exception
|
| try: | ||
| ableton = get_ableton_connection() | ||
| result = ableton.send_command("get_clip_notes", { | ||
| "track_index": track_index, | ||
| "clip_index": clip_index | ||
| }) | ||
| return json.dumps(result, indent=2) | ||
| except Exception as e: | ||
| logger.error(f"Error getting clip notes: {str(e)}") | ||
| return f"Error getting clip notes: {str(e)}" |
There was a problem hiding this comment.
1. get_clip_notes returns raw exception 📘 Rule violation ⛨ Security
The new get_clip_notes tool returns str(e) directly to the caller, which can expose internal implementation details and environment-specific errors. This violates secure error handling expectations for user-facing responses.
Agent Prompt
## Issue description
`get_clip_notes` returns raw exception text (`str(e)`) to the caller, which can leak internal details.
## Issue Context
Secure error handling requires generic user-facing messages while keeping detailed diagnostics in server logs.
## Fix Focus Areas
- MCP_Server/server.py[359-368]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| clip_length = clip.length | ||
| quantization = getattr(clip, "launch_quantization", None) | ||
|
|
||
| scale_info = None | ||
|
|
||
| if hasattr(self._song, "scale_mode"): | ||
| scale_info = { | ||
| "enabled": self._song.scale_mode, | ||
| "root_note": getattr(self._song, "root_note", None), | ||
| "scale_name": getattr(self._song, "scale_name", None), | ||
| "scale_intervals": getattr(self._song, "scale_intervals", None) | ||
| } | ||
|
|
||
|
|
||
| result = { | ||
| "notes": notes, | ||
| "clip_length": clip_length, | ||
| "quantization": quantization, | ||
| "scale_info": scale_info | ||
| } |
There was a problem hiding this comment.
2. Non-serializable response fields 🐞 Bug ⛯ Reliability
_get_clip_notes returns quantization (and possibly other Live objects) directly in the response, but the Remote Script and server both serialize responses using json.dumps without a custom encoder. If quantization (or any note element) isn’t a JSON-primitive, json.dumps will raise and the command will fail.
Agent Prompt
### Issue description
`get_clip_notes` can fail at runtime because it returns values that may not be JSON-serializable (e.g., `clip.launch_quantization`), but responses are serialized with `json.dumps()`.
### Issue Context
The Remote Script sends responses via `json.dumps(response)` over the socket, and the MCP server additionally does `json.dumps(result, indent=2)`.
### Fix Focus Areas
- AbletonMCP_Remote_Script/__init__.py[158-174]
- AbletonMCP_Remote_Script/__init__.py[518-537]
- MCP_Server/server.py[359-366]
### Suggested approach
- Convert `quantization` to a safe primitive (e.g., `int(quantization)` when possible, else `str(quantization)` or `None`).
- Ensure `scale_info` values are primitives/lists (convert tuples to lists explicitly if desired).
- If note items can be objects/tuples, normalize them into dicts/lists of primitives (see other finding).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
AbletonMCP_Remote_Script/__init__.py (1)
228-232: Misleading comment:get_clip_notesis a read-only operation.The comment "Commands that modify Live's state" is inaccurate for
get_clip_notes. Routing through the main thread is still the correct approach since all Live API calls must originate from the main thread, but a clarifying update avoids confusion.✏️ Suggested comment update
- # Commands that modify Live's state should be scheduled on the main thread + # All Live API access (reads and writes) must be scheduled on the main thread elif command_type in ["create_midi_track", "set_track_name", "create_clip", "get_clip_notes", "add_notes_to_clip", "set_clip_name",🤖 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 228 - 232, The comment above the command_type check is misleading because it says "Commands that modify Live's state" but the list includes read-only operations like get_clip_notes; update the comment near the command_type check to state that these are Live API calls that must be executed on the main thread (including both modifying and read-only operations such as get_clip_notes), so that functions like get_clip_notes, create_midi_track, set_track_name, create_clip, add_notes_to_clip, set_clip_name, set_tempo, fire_clip, stop_clip, start_playback, stop_playback, and load_browser_item are clearly documented as main-thread Live API calls rather than all being mutating commands.
🤖 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 509-516: The code currently treats only list returns from
clip.get_all_notes_extended() and drops tuples, and it leaves Live API note
objects un-serialized causing json.dumps to fail; update the check for all_notes
to accept both list and tuple (e.g., isinstance(all_notes, (list, tuple))) or
treat any iterable, then normalize notes by mapping each note into a JSON-safe
structure: convert nested tuples to lists and for Live/MidiNoteSpecification
objects produce a plain dict using attribute access (e.g., pitch, start_time,
duration, velocity, mute via getattr with sensible defaults); ensure the final
notes variable is a list of only primitives/ dicts/ lists so server.py’s
json.dumps(result, indent=2) will succeed.
---
Nitpick comments:
In `@AbletonMCP_Remote_Script/__init__.py`:
- Around line 228-232: The comment above the command_type check is misleading
because it says "Commands that modify Live's state" but the list includes
read-only operations like get_clip_notes; update the comment near the
command_type check to state that these are Live API calls that must be executed
on the main thread (including both modifying and read-only operations such as
get_clip_notes), so that functions like get_clip_notes, create_midi_track,
set_track_name, create_clip, add_notes_to_clip, set_clip_name, set_tempo,
fire_clip, stop_clip, start_playback, stop_playback, and load_browser_item are
clearly documented as main-thread Live API calls rather than all being mutating
commands.
| all_notes = clip.get_all_notes_extended() | ||
| if isinstance(all_notes, dict): | ||
| notes = all_notes.get("notes", []) | ||
| elif isinstance(all_notes, list): | ||
| notes = all_notes | ||
| else: | ||
| notes = [] | ||
| notes = list(notes) # Convert to list if it's a tuple or other iterable |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the _get_clip_notes method around lines 509-516
cd /tmp && find . -name "*.py" | head -20Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Get the full file structure
git ls-files | grep -E "(init|AbletonMCP)" | head -20Repository: ahujasid/ableton-mcp
Length of output: 123
🏁 Script executed:
# Read the entire file to understand context
wc -l AbletonMCP_Remote_Script/__init__.pyRepository: ahujasid/ableton-mcp
Length of output: 105
🏁 Script executed:
# Read the section with the _get_clip_notes method
sed -n '488,542p' AbletonMCP_Remote_Script/__init__.pyRepository: ahujasid/ableton-mcp
Length of output: 2072
🏁 Script executed:
# Search for get_all_notes_extended usage in the codebase
rg "get_all_notes_extended" --type pyRepository: ahujasid/ableton-mcp
Length of output: 154
🏁 Script executed:
# Search for any serialization/json handling of notes
rg "json\.dumps.*note" --type py -A 2 -B 2Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Search for note serialization or conversion logic
rg "MidiNoteSpecification\|notes.*json\|json.*note" --type py -C 3Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Search for where _get_clip_notes is called
rg "_get_clip_notes" --type py -B 2 -A 5Repository: ahujasid/ableton-mcp
Length of output: 1516
🏁 Script executed:
# Check MCP_Server for JSON serialization of results
sed -n '1,100p' MCP_Server/__init__.pyRepository: ahujasid/ableton-mcp
Length of output: 270
🏁 Script executed:
# Search for JSON serialization of responses in MCP_Server
rg "json\|dumps\|serialize" MCP_Server/__init__.py -B 3 -A 3Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Look for any version compatibility checks or hasattr guards in the main file
rg "hasattr.*clip\|version.*Live\|Live.*version" AbletonMCP_Remote_Script/__init__.py -B 2 -A 2Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Search for any imports or references to Ableton API documentation
head -50 AbletonMCP_Remote_Script/__init__.pyRepository: ahujasid/ableton-mcp
Length of output: 1476
🏁 Script executed:
# Look for test files or examples showing get_all_notes_extended usage
fd "test|example" --type f | head -10Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Find where the result from _get_clip_notes is handled/sent back
rg "get_clip_notes" --type py -A 10 | head -40Repository: ahujasid/ableton-mcp
Length of output: 2775
🏁 Script executed:
# Look at the server side handling to see if results are JSON serialized
cat MCP_Server/__init__.pyRepository: ahujasid/ableton-mcp
Length of output: 270
🏁 Script executed:
# Search for response handling or result transmission
rg "json\.dumps\|result.*send\|return.*result" AbletonMCP_Remote_Script/__init__.py -B 2 -A 2 | head -50Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Look for how commands are processed and responses sent
rg "def.*command\|def.*handle" AbletonMCP_Remote_Script/__init__.py -A 15 | head -60Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Get the full get_clip_notes function from server.py
rg "def get_clip_notes" MCP_Server/server.py -A 30Repository: ahujasid/ableton-mcp
Length of output: 1112
🏁 Script executed:
# Check the server.py file structure to understand result handling
cat MCP_Server/server.py | head -100Repository: ahujasid/ableton-mcp
Length of output: 3896
🏁 Script executed:
# Look at the entire get_clip_notes implementation in server.py
sed -n '1,50p' MCP_Server/server.pyRepository: ahujasid/ableton-mcp
Length of output: 1733
🏁 Script executed:
# Get more context from server.py around the get_clip_notes function
wc -l MCP_Server/server.pyRepository: ahujasid/ableton-mcp
Length of output: 88
🏁 Script executed:
# Read the full MCP_Server/server.py file
cat MCP_Server/server.pyRepository: ahujasid/ableton-mcp
Length of output: 27275
🏁 Script executed:
# Search for any documentation or comments about get_all_notes_extended
rg "get_all_notes_extended" --type py -B 5 -A 5Repository: ahujasid/ableton-mcp
Length of output: 850
🏁 Script executed:
# Look for any version checks or Live API compatibility code
rg "Live\|version\|hasattr.*clip" AbletonMCP_Remote_Script/__init__.py | head -30Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Check if there are any tests or examples of note handling
find . -name "*.py" -type f | xargs grep -l "notes\|MidiNoteSpecification" 2>/dev/nullRepository: ahujasid/ableton-mcp
Length of output: 125
🏁 Script executed:
# Search the entire codebase for references to note format/structure
rg "pitch.*start_time\|velocity.*mute" --type py -B 2 -A 2Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Look at add_notes_to_clip to understand expected note format
sed -n '560,650p' AbletonMCP_Remote_Script/__init__.pyRepository: ahujasid/ableton-mcp
Length of output: 3096
🏁 Script executed:
# Look at the entire _get_clip_notes method to see if there's error handling after line 516
sed -n '488,545p' AbletonMCP_Remote_Script/__init__.pyRepository: ahujasid/ableton-mcp
Length of output: 2190
🏁 Script executed:
# Search for any documentation or comments about note format
rg "notes.*tuple\|tuple.*note\|MidiNoteSpecification\|get_all_notes" --type py -iRepository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Check how notes are used in result dict - look for serialization
sed -n '509,540p' AbletonMCP_Remote_Script/__init__.pyRepository: ahujasid/ableton-mcp
Length of output: 1287
🏁 Script executed:
# Search for any exception handling that might reveal the actual type
rg "TypeError\|json.*serial\|not.*json" AbletonMCP_Remote_Script/__init__.py -B 3 -A 3Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Look at what happens with the result dict after _get_clip_notes returns
rg "get_clip_notes.*result\|result.*get_clip_notes" --type py -A 10Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Check if there's a README or documentation
find . -name "README*" -o -name "*.md" | head -5Repository: ahujasid/ableton-mcp
Length of output: 75
🏁 Script executed:
# Read README for any API documentation
cat README.md 2>/dev/null | head -100Repository: ahujasid/ableton-mcp
Length of output: 3859
🏁 Script executed:
# Look for any error messages or type information in comments
rg "tuple\|list\|dict" AbletonMCP_Remote_Script/__init__.py | grep -i "note\|return"Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Check if there are any tests or example usage
find . -name "*.py" -type f -exec grep -l "get_all_notes_extended\|_get_clip_notes" {} \;Repository: ahujasid/ableton-mcp
Length of output: 102
🏁 Script executed:
# Look at git history or comments about get_all_notes_extended
git log --all --oneline --grep="get_all_notes_extended\|notes" 2>/dev/null | head -10Repository: ahujasid/ableton-mcp
Length of output: 156
🏁 Script executed:
# Check if there are any imports or references to Ableton Live documentation
head -100 AbletonMCP_Remote_Script/__init__.py | grep -i "import\|ableton\|live"Repository: ahujasid/ableton-mcp
Length of output: 1280
🏁 Script executed:
# Search for any test code that might call get_all_notes_extended or _get_clip_notes
find . -type f \( -name "*test*.py" -o -name "*example*.py" \)Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Look for any issue reports or TODOs about note serialization
rg "TODO\|FIXME\|BUG\|XXX\|HACK" AbletonMCP_Remote_Script/__init__.py -B 2 -A 2Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Let me check if there's any indication in git history about what get_all_notes_extended returns
git show 834ca5a 2>/dev/null | head -200Repository: ahujasid/ableton-mcp
Length of output: 7382
🏁 Script executed:
# Search for any comments or documentation about Live 11 API differences
rg "Live.*11\|Live.*10\|get_all_notes\|get_notes" --type py -B 3 -A 3Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Check if there's a requirements file or setup that might indicate Ableton version support
find . -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml"Repository: ahujasid/ableton-mcp
Length of output: 80
🏁 Script executed:
# Let me verify the logic bug: trace what happens with tuple input
python3 << 'EOF'
# Simulate the code logic
test_cases = {
"dict_case": {"notes": [{"pitch": 60}]},
"list_case": [{"pitch": 60}],
"tuple_case": ({"pitch": 60},),
"none_case": None
}
for name, all_notes in test_cases.items():
if isinstance(all_notes, dict):
notes = all_notes.get("notes", [])
print(f"{name}: dict branch -> notes = {notes}")
elif isinstance(all_notes, list):
notes = all_notes
print(f"{name}: list branch -> notes = {notes}")
else:
notes = []
print(f"{name}: else branch -> notes = {notes}")
EOFRepository: ahujasid/ableton-mcp
Length of output: 238
🏁 Script executed:
# Check pyproject.toml for dependencies and Ableton version info
cat pyproject.tomlRepository: ahujasid/ableton-mcp
Length of output: 863
🏁 Script executed:
# Look for any Ableton documentation or comments about get_all_notes_extended return type
rg "Ableton.*api\|Live.*api\|get_all_notes" --type py --type md -i -B 2 -A 2Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Check if there's any version-specific handling or compatibility code
rg "hasattr.*get_all_notes\|AttributeError\|try.*get_all_notes" --type py -B 3 -A 3Repository: ahujasid/ableton-mcp
Length of output: 46
🏁 Script executed:
# Now test what happens when trying to json.dumps an object (like MidiNoteSpecification)
python3 << 'EOF'
import json
# Simulate a C++ extension-like object (cannot be JSON serialized)
class MidiNoteSpecification:
def __init__(self, pitch, start_time, duration, velocity, mute):
self.pitch = pitch
self.start_time = start_time
self.duration = duration
self.velocity = velocity
self.mute = mute
# Test what happens when we try to serialize
notes_with_objects = [
MidiNoteSpecification(60, 0.0, 1.0, 100, False),
MidiNoteSpecification(64, 1.0, 1.0, 100, False)
]
result = {
"notes": notes_with_objects,
"clip_length": 4.0,
"quantization": None,
"scale_info": None
}
try:
json.dumps(result, indent=2)
print("SUCCESS: JSON serialization worked")
except TypeError as e:
print(f"ERROR: {e}")
EOFRepository: ahujasid/ableton-mcp
Length of output: 132
🏁 Script executed:
# Let's check the git commit more to see if there are any clues about implementation
git show 834ca5a:AbletonMCP_Remote_Script/__init__.py 2>/dev/null | sed -n '509,530p'Repository: ahujasid/ableton-mcp
Length of output: 961
🏁 Script executed:
# Search for any reference to Live API documentation or version support
cat README.md | grep -i "live\|version\|11\|10"Repository: ahujasid/ableton-mcp
Length of output: 1809
🏁 Script executed:
# Verify the full data flow: _get_clip_notes result → server.py json.dumps
rg "_get_clip_notes\|get_clip_notes" --type py -B 2 -A 8 | grep -A 20 "get_clip_notes"Repository: ahujasid/ableton-mcp
Length of output: 46
Fix isinstance check to handle tuple return and serialize note objects before returning.
Bug 1: isinstance(all_notes, list) silently discards tuple returns.
When get_all_notes_extended() returns a tuple (the documented return type in Live 11+), the condition isinstance(all_notes, list) evaluates to False, causing execution to fall through to else: notes = []. This silently returns an empty notes list regardless of actual clip content. Verified: isinstance((obj,), list) is always False.
Bug 2: Note objects are not JSON-serializable.
The result dict containing notes is passed to json.dumps(result, indent=2) in server.py. If notes contains Live API objects (tuples of tuples or MidiNoteSpecification objects), the serialization fails with TypeError: Object of type X is not JSON serializable. The list(notes) conversion on line 516 only wraps the container—individual note objects are not converted to JSON-compatible dicts.
🐛 Proposed fix
all_notes = clip.get_all_notes_extended()
if isinstance(all_notes, dict):
- notes = all_notes.get("notes", [])
+ raw_notes = all_notes.get("notes", [])
- elif isinstance(all_notes, list):
- notes = all_notes
else:
- notes = []
- notes = list(notes) # Convert to list if it's a tuple or other iterable
+ raw_notes = all_notes if all_notes is not None else []
+
+ # Serialize note objects to dicts (handle both tuple and object formats)
+ notes = []
+ for note in raw_notes:
+ if isinstance(note, (tuple, list)):
+ notes.append({
+ "pitch": note[0],
+ "start_time": note[1],
+ "duration": note[2],
+ "velocity": note[3],
+ "mute": bool(note[4])
+ })
+ elif hasattr(note, "pitch"):
+ # MidiNoteSpecification or similar object
+ notes.append({
+ "pitch": int(note.pitch),
+ "start_time": float(note.start_time),
+ "duration": float(note.duration),
+ "velocity": int(note.velocity),
+ "mute": bool(note.mute)
+ })🤖 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 509 - 516, The code
currently treats only list returns from clip.get_all_notes_extended() and drops
tuples, and it leaves Live API note objects un-serialized causing json.dumps to
fail; update the check for all_notes to accept both list and tuple (e.g.,
isinstance(all_notes, (list, tuple))) or treat any iterable, then normalize
notes by mapping each note into a JSON-safe structure: convert nested tuples to
lists and for Live/MidiNoteSpecification objects produce a plain dict using
attribute access (e.g., pitch, start_time, duration, velocity, mute via getattr
with sensible defaults); ensure the final notes variable is a list of only
primitives/ dicts/ lists so server.py’s json.dumps(result, indent=2) will
succeed.
(get_clip_notes) Issue 35 I believe.
Summary by CodeRabbit