ableton-mcp-ai/AbletonMCP_AI/CODE_REVIEW_NEXT_STEPS.md

Code Review and Next Steps

Date: 2026-03-29 Repository state reviewed after cleanup commit 5b804db

Scope

This review covers the remaining tracked source in the repository:

• AbletonMCP_AI/MCP_Server/*.py
• AbletonMCP_AI/MCP_Server/tests/*.py
• project docs that define current intent (CHANGELOG.md, IMPLEMENTATION_REPORT.md, PRO_DJ_ROADMAP.md, tofix.md)
• remaining root utility scripts and wrappers (mcp_wrapper.py, start_mcp.bat, validate_script.py, validate_audio_resampler.py, diagnostico_wsl.py, mcp_1429/server.py, etc.)

Checks executed during the review:

• python -m unittest discover .\AbletonMCP_AI\MCP_Server\tests -v
• python -m compileall .\AbletonMCP_AI\MCP_Server
• python .\validate_audio_resampler.py
• python .\validate_script.py
• python .\AbletonMCP_AI\MCP_Server\health_check.py
• import smoke for selected modules (reference_stem_builder, vector_manager, full_integration, server_v2)

Executive summary

The project has a real and useful architecture: server.py is the operational core, song_generator.py and sample_selector.py hold important musical logic, and full_integration.py plus the current tests show that part of the high-level generation pipeline is coherent.

The main problem is not lack of features. The main problem is stability and maintainability drift:

• optional dependencies are treated as hard requirements in several import paths
• machine-specific paths are still embedded across the codebase
• server.py contains duplicated MCP tool definitions
• song_generator.py contains duplicated helper functions
• health/validation tooling gives a more optimistic picture than the code currently supports

Before adding more roadmap features, the next sprint should be a stabilization sprint.

Confirmed bugs and risks

P0 - Confirmed functional bugs

1. AbletonMCP_AI/MCP_Server/server_v2.py:393

   • compileall fails with SyntaxError: name '_ableton_connection' is used prior to global declaration.
   • Current status: the file is in the repo but is not runnable.
   • Recommendation: either fix it immediately or archive/remove server_v2.py until it is valid again.

2. AbletonMCP_AI/MCP_Server/audio_analyzer.py:12

   • numpy is imported at module import-time even when AudioAnalyzer(backend="basic") is requested.
   • This breaks tests/test_sample_selector.py:69 and also breaks validate_audio_resampler.py.
   • Evidence:
     • unittest discover => 20 tests pass, 1 test errors on ModuleNotFoundError: No module named 'numpy'
     • validate_audio_resampler.py => 0/5 checks pass for the same reason
   • Recommendation: move numpy usage behind lazy import/fallback logic so the "basic" backend really works without scientific deps.

3. AbletonMCP_AI/MCP_Server/requirements.txt

   • The file only declares mcp>=1.0.0, but the reviewed code imports or optionally depends on:
     • numpy
     • librosa
     • soundfile
     • sentence_transformers
     • sklearn
     • torch
     • demucs
   • Current status: environment setup is under-specified.
   • Recommendation: split dependencies into base and extras (audio, ml, stems, dev) and keep docs/tests aligned with those extras.

P1 - High priority structural defects

4. AbletonMCP_AI/MCP_Server/server.py

   • Duplicate MCP tool definitions exist.
   • AST count during review:
     • 93 tool definitions
     • 85 unique tool names
   • Confirmed duplicate names:
     • generate_with_human_feel at lines 5453 and 8574
     • apply_clip_fades at lines 5502 and 8623
     • write_volume_automation at lines 5573 and 8694
     • apply_sidechain_pump at lines 5651 and 8772
     • inject_pattern_fills at lines 5698 and 8819
     • humanize_set at lines 5746 and 8867
     • suggest_key_change at lines 5862 and 6945
     • reset_diversity_memory at lines 6272 and 8935
   • Risk:
     • duplicate registration can hide drift between versions
     • tool count in docs/changelog may be inflated
     • future changes can silently modify only one copy
   • Recommendation: dedupe the file and add a test that fails if duplicate tool names exist.

5. AbletonMCP_AI/MCP_Server/song_generator.py

   • Duplicate helper functions exist:
     • _get_pattern_variant_penalty at 1582 and 1939
     • _record_pattern_variant_usage at 1586 and 1946
     • _decay_pattern_variant_memory at 1590 and 1952
     • reset_pattern_variant_memory at 1594 and 1960
   • Risk:
     • shadowed logic
     • future edits hitting the wrong copy
     • unclear source of truth
   • Recommendation: keep one copy, remove the other, and add a regression test for pattern-variant memory behavior.

6. AbletonMCP_AI/MCP_Server/health_check.py

   • Dependency check and test check are misleading:
     • check_dependencies() hard-requires numpy, sklearn, sentence_transformers at lines 75-83
     • check_tests() only runs tests.test_human_feel at lines 135-147
     • the report can say "All tests passing" even while the full suite fails
   • Actual observed behavior:
     • health_check.py reported 5/9 checks passing
     • unit tests were marked as passing
     • the full suite still fails on audio_analyzer.py
   • Recommendation:
     • split "core deps" vs "optional deps"
     • run the real suite, not a single test module
     • expose failed check details in console output more clearly

7. AbletonMCP_AI/MCP_Server/reference_stem_builder.py:13-16

   • Heavy dependencies are imported at module import-time:
     • soundfile
     • torch
     • demucs
   • Import smoke result:
     • reference_stem_builder fails immediately with ModuleNotFoundError: No module named 'soundfile'
   • Recommendation:
     • move these imports inside the stem-building flow
     • gate the feature behind a clear optional dependency check
     • document the extra dependency group

P2 - Portability and packaging issues

8. Hard-coded local paths remain in operational files and demos:

   • AbletonMCP_AI/MCP_Server/server.py
   • AbletonMCP_AI/MCP_Server/server_v2.py:53
   • AbletonMCP_AI/MCP_Server/start_server.py:6-10
   • mcp_wrapper.py:10
   • start_mcp.bat:2-4
   • AbletonMCP_AI/MCP_Server/scan_audio.py:5
   • AbletonMCP_AI/MCP_Server/sample_system_demo.py:25,52
   • AbletonMCP_AI/MCP_Server/reference_stem_builder.py and other modules still assume local library layout
   • Risk:
     • fragile on another machine
     • hard to test in CI
     • requires sys.path hacks and wrapper scripts
   • Recommendation:
     • introduce a settings.py or env-driven config layer
     • use package-relative imports
     • stop hardcoding C:\Users\ren\... and C:\ProgramData\... in runtime modules

9. Package import strategy is still brittle

   • Evidence:
     • server.py mutates sys.path
     • server_v2.py mutates sys.path
     • start_server.py forces os.chdir(...)
     • tests also mutate sys.path
   • Risk:
     • behavior depends on launch location
     • tools may work from one wrapper and fail from another
   • Recommendation:
     • make AbletonMCP_AI a real package boundary
     • prefer from .module import ... or from AbletonMCP_AI.MCP_Server.module import ...
     • keep wrapper scripts thin and path-agnostic

10. Validation/diagnostic scripts are not resilient

• validate_script.py crashes on connection refusal instead of reporting a clean diagnostic result.
• validate_audio_resampler.py assumes import-time availability of the whole scientific stack.
• mcp_1429/server.py looks like an unrelated test server and should not live near production entrypoints.
• Recommendation:
  • move one-off utilities into scripts/dev, scripts/diagnostics, and scripts/manual
  • make each script report actionable failure instead of throwing raw tracebacks

Code review notes by area

What is good

• full_integration.py is coherent and readable for a high-level orchestration layer.
• sample_selector.py has real domain logic for fatigue, palette, coverage and compatibility.
• audio_key_compatibility.py, audio_arrangement.py, audio_mastering.py, and human_feel.py are good candidates for stable domain modules.
• There is already some test coverage around:
  • human feel
  • full integration
  • sample selector behavior
• The roadmap and implementation report are detailed enough to guide future work.

What is weak

• server.py is now a monolith at ~10k lines.
• song_generator.py is ~6k lines and mixes configuration, generation logic, transition logic, and pattern-memory helpers.
• reference_listener.py is ~4.7k lines and likely deserves its own decomposition plan.
• project scripts are still spread between core runtime, demos, wrappers, manual validators and environment-specific helpers.
• docs are drifting from reality:
  • tofix.md says there are no critical runtime issues
  • current code still has confirmed critical issues

Priority roadmap for the next implementation sprint

Sprint 1 - Stabilize the runtime

1. Fix audio_analyzer.py so backend="basic" works without numpy.
2. Decide the fate of server_v2.py: fix or archive.
3. Deduplicate server.py MCP tools.
4. Deduplicate song_generator.py pattern-memory helpers.
5. Make health_check.py honest:
   • real suite
   • real dependency tiers
   • clearer reporting
6. Update requirements.txt into real install profiles.

Sprint 2 - Make the code portable

7. Create a centralized config layer for:
   • sample library paths
   • ProgramData paths
   • host/port values
   • optional feature flags
8. Remove machine-specific paths from wrappers and demos.
9. Replace sys.path/os.chdir hacks with package-safe imports.

Sprint 3 - Reduce maintenance cost

10. Split server.py into smaller modules, for example:
    • tools_session.py
    • tools_generation.py
    • tools_validation.py
    • tools_audio_fx.py
    • tools_learning.py
11. Move manual scripts and demos into scripts/:
    • scan_audio.py
    • sample_system_demo.py
    • validate_script.py
    • validate_audio_resampler.py
    • diagnostico_wsl.py
    • temp_socket_cmd.py
    • place_perc_audio.py
    • set_input_routing.py
    • mcp_1429/server.py

Sprint 4 - Improve confidence

12. Add CI for:
    • import smoke
    • compileall
    • full unit test suite
    • duplicate-tool-name detection
13. Add tests for:
    • audio_analyzer basic fallback
    • health_check correctness
    • server tool registration uniqueness
    • config/path resolution without local machine assumptions

Feature work that should wait until after stabilization

The docs already show partially implemented roadmap items. These should be resumed only after the runtime is stable:

• T017 / T019: brighter and key-aware sample analysis quality
• T044 / T047: dynamic variation and loop variation completion
• T051 / T058 / T059: better tonal and spectral section behavior
• T068 / T069 / T070: section-based kick, hat and bass evolution
• T089: A/B drop testing
• T101 / T103 / T105: real regression coverage, hot reload strategy, CI

Suggested owner checklist for the next pass

• Fix dependency model first
• Remove duplicate definitions in server.py
• Remove duplicate helpers in song_generator.py
• Make health check trustworthy
• Move environment-specific utilities out of the core
• Add CI so the same regressions do not come back

Bottom line

The project is not blocked by lack of ideas. It is blocked by runtime honesty and codebase shape.

If the next work session focuses on stabilization, packaging and test truthfulness, the feature roadmap becomes much safer to continue. If new features are added first, the duplicate logic and environment coupling will keep compounding.