Add rack device introspection: get_rack_device_info tool + chain summaries in get_track_info#67
Add rack device introspection: get_rack_device_info tool + chain summaries in get_track_info#67BenCello wants to merge 1 commit intoahujasid:mainfrom
Conversation
…aries in get_track_info get_track_info now reports chain names and device counts for rack devices. New get_rack_device_info endpoint drills into a specific rack to list all nested devices per chain, with recursive serialization for nested racks. Co-Authored-By: Claude Opus 4.6
Review Summary by QodoAdd rack device introspection with chain details
WalkthroughsDescription• Add get_rack_device_info tool for detailed rack device introspection • Enhance get_track_info with chain summaries for rack devices • Implement recursive device serialization for nested racks • Support drilling into specific rack devices to list all chains Diagramflowchart LR
A["get_track_info"] -->|enhanced| B["Device Info with Chains"]
C["get_rack_device_info"] -->|new tool| D["Rack Device Details"]
D -->|recursive| E["Nested Devices per Chain"]
B -->|chains summary| F["Chain Names & Device Counts"]
File Changes1. AbletonMCP_Remote_Script/__init__.py
|
📝 WalkthroughWalkthroughThis pull request adds support for querying detailed rack device information in Ableton Live. It introduces a new command Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP_Server as MCP Server
participant RemoteScript as Remote Script
participant Ableton as Ableton Live
Client->>MCP_Server: get_rack_device_info(track_index, device_index)
MCP_Server->>RemoteScript: _process_command(get_rack_device_info, ...)
RemoteScript->>RemoteScript: _get_rack_device_info(track_index, device_index)
RemoteScript->>Ableton: Query track and device objects
Ableton-->>RemoteScript: Device and chain data
RemoteScript->>RemoteScript: _serialize_device() recursively
RemoteScript-->>MCP_Server: Serialized device structure
MCP_Server->>MCP_Server: Format as JSON
MCP_Server-->>Client: Pretty-printed JSON response
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Note: While the remote script additions are straightforward, the duplicate Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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. IndexError breaks client loop
|
| if track_index < 0 or track_index >= len(self._song.tracks): | ||
| raise IndexError("Track index out of range") | ||
|
|
||
| track = self._song.tracks[track_index] | ||
|
|
||
| if device_index < 0 or device_index >= len(track.devices): | ||
| raise IndexError("Device index out of range") | ||
|
|
There was a problem hiding this comment.
1. indexerror breaks client loop 📘 Rule violation ⛯ Reliability
The new rack introspection command raises IndexError for invalid indices, and the client handler treats non-ValueError exceptions as serious and breaks the connection. This makes simple bad input an availability/reliability issue instead of a handled edge case.
Agent Prompt
## Issue description
Out-of-range indices currently raise `IndexError`, which leads the client handler to break the loop and disconnect.
## Issue Context
The client handler only treats `ValueError` as non-fatal; other exceptions trigger a disconnect.
## Fix Focus Areas
- AbletonMCP_Remote_Script/__init__.py[450-457]
- AbletonMCP_Remote_Script/__init__.py[198-200]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if not device.can_have_chains: | ||
| raise ValueError("Device '{}' is not a rack and does not have chains".format(device.name)) | ||
|
|
There was a problem hiding this comment.
2. Exception exposes device.name 📘 Rule violation ⛨ Security
The new rack endpoint includes device.name in an exception message, and error handling sends str(e) back to the client. This can leak internal session/device details to the caller.
Agent Prompt
## Issue description
The error message includes `device.name` and is propagated to clients via `str(e)`.
## Issue Context
User-facing error responses should be generic and not reveal internal session/device details.
## Fix Focus Areas
- AbletonMCP_Remote_Script/__init__.py[460-462]
- AbletonMCP_Remote_Script/__init__.py[184-187]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| def _serialize_device(self, device): | ||
| """Serialize a device, recursively including chains for rack devices""" | ||
| device_info = { | ||
| "name": device.name, | ||
| "class_name": device.class_name, | ||
| "type": self._get_device_type(device) | ||
| } | ||
| if device.can_have_chains: | ||
| chains = [] | ||
| for chain_index, chain in enumerate(device.chains): | ||
| chain_devices = [self._serialize_device(dev) for dev in chain.devices] | ||
| chains.append({ | ||
| "index": chain_index, | ||
| "name": chain.name, | ||
| "devices": chain_devices | ||
| }) | ||
| device_info["chains"] = chains | ||
| return device_info |
There was a problem hiding this comment.
3. Unbounded rack serialization 🐞 Bug ⛯ Reliability
New rack serialization recursively walks all nested racks/chains with no depth/size limits, so deep/large racks can cause very slow responses, huge payloads, or a Python RecursionError. Large payloads also increase the chance the Remote Script’s blocking sendall (no timeout) will stall a client handler thread if the client is slow/disconnected.
Agent Prompt
### Issue description
`get_rack_device_info` recursively serializes nested racks without any depth/size limits. Deep or large racks can produce huge responses, hit Python recursion limits, and/or cause long blocking socket sends.
### Issue Context
This endpoint is designed to “drill in” and may be called on complex Ableton projects where nested racks/chains can be large.
### Fix Focus Areas
- AbletonMCP_Remote_Script/__init__.py[428-469]
- AbletonMCP_Remote_Script/__init__.py[133-177]
### Suggested changes
- Update `_serialize_device(device, depth=0, max_depth=..., budget=...)`:
- Stop recursing past `max_depth` and/or when a `max_total_devices` budget is exceeded.
- Include a field like `"truncated": true` or `"truncation_reason": "max_depth"` when limits are reached.
- Optionally include `index` values for devices within each chain to make truncated results still navigable.
- Update `_get_rack_device_info` to accept optional params (e.g., `max_depth`, `max_total_devices`) from `params` and pass them through.
- Consider making the client handler safer for large responses:
- Set a reasonable socket timeout for sends, or
- Send in chunks and abort cleanly if the client disconnects / blocks too long.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
get_track_info now reports chain names and device counts for rack devices. New get_rack_device_info endpoint drills into a specific rack to list all nested devices per chain, with recursive serialization for nested racks.
Co-Authored-By: Claude Opus 4.6
Summary by CodeRabbit