Refactor: Remove legacy IPC and finalize FastAPI transition#104
Refactor: Remove legacy IPC and finalize FastAPI transition#104
Conversation
- Removed deprecated Unix Socket IPC implementation. - Updated documentation (READMEs, AGENTS.md) to reflect Modular Monolith architecture. - Cleaned up v2m.main.py (removed --daemon, internal CLI). - Refactored operation scripts (stop_daemon, health_check, cleanup_v2m) to use HTTP/FastAPI instead of sockets. - Deleted obsolete scripts (send_command.py, verify_export_backend.py). - Updated verify_daemon.py to test FastAPI endpoints. - Removed ollama dependency from pyproject.toml. Co-authored-by: cesarszv <205967966+cesarszv@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR finalizes the migration of the Voice2Machine daemon from Unix-socket IPC to a FastAPI-based REST architecture, and removes most remaining legacy IPC code and references.
Changes:
- Simplifies
v2m.maininto a pure FastAPI server entrypoint and updates backend documentation to describe the new feature-based, REST-first design. - Removes legacy Unix socket IPC utilities and scripts (
send_command.py,verify_export_backend.py, socket/PID cleanup), updating operational scripts (stop_daemon.sh,cleanup_v2m.sh,health_check.py) to work via HTTP. - Rewrites
verify_daemon.pyinto an HTTP-based integration smoke test and trims dependencies by removing theollamaPython package frompyproject.toml.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
apps/daemon/backend/src/v2m/main.py |
Converts the module into a single-purpose FastAPI server runner (no CLI client / --daemon mode), aligning the entrypoint with the HTTP-only architecture. |
apps/daemon/backend/src/v2m/README.md |
Updates package-level docs to describe the FastAPI/REST API, feature-based layout, and troubleshooting in terms of localhost HTTP instead of Unix sockets. |
apps/daemon/backend/scripts/shared/send_command.py |
Deletes the standalone Unix-socket IPC client script, removing dead code tied to the old transport. |
apps/daemon/backend/scripts/shared/common.sh |
Drops the get_runtime_dir helper, reflecting that scripts no longer manage Unix socket/PID runtime directories. |
apps/daemon/backend/scripts/operations/daemon/stop_daemon.sh |
Updates process matching to stop python -m v2m.main and removes orphan-socket cleanup, consistent with the HTTP daemon. |
apps/daemon/backend/scripts/diagnostics/verify_export_backend.py |
Removes the IPC-based export verification script that depended on Unix sockets and TRANSCRIBE_FILE commands. |
apps/daemon/backend/scripts/diagnostics/verify_daemon.py |
Replaces the legacy client/IPC-based integration test with an HTTP smoke test that starts v2m.main, calls /health, /start, /stop, and /llm/process. |
apps/daemon/backend/scripts/diagnostics/health_check.py |
Switches the daemon responsiveness check from Unix-socket PING to HTTP GET /health, and simplifies zombie detection and cleanup to match the new architecture. |
apps/daemon/backend/scripts/development/maintenance/cleanup_v2m.sh |
Simplifies cleanup to just process and VRAM checks, removing Unix-socket and PID file cleanup that are no longer used. |
apps/daemon/backend/pyproject.toml |
Drops the ollama Python dependency, reducing the backend’s dependency footprint while keeping requests for HTTP diagnostics. |
apps/daemon/backend/README.md |
Aligns the backend quick-start instructions with the FastAPI server entrypoint (python -m v2m.main on localhost:8765). |
apps/daemon/backend/AGENTS.md |
Updates AI-agent instructions and architecture notes to treat FastAPI/REST as the canonical interface, marking legacy infrastructure removal as completed. |
| import subprocess | ||
| import urllib.request | ||
| import json | ||
| from pathlib import Path |
There was a problem hiding this comment.
El import de Path ya no se utiliza tras eliminar la lógica basada en sockets y ficheros de runtime, por lo que queda como import muerto. Para mantener el script limpio y evitar advertencias de linting, sería mejor eliminar este import si ya no es necesario.
| from pathlib import Path |
| python -m v2m.main PING | ||
| python -m v2m.main START_RECORDING | ||
| python -m v2m.main STOP_RECORDING | ||
| curl -X POST http://localhost:8765/health |
There was a problem hiding this comment.
El ejemplo de quick start usa curl -X POST http://localhost:8765/health, pero el endpoint /health está implementado solo como GET (ver v2m/api/routes/status.py). Esto puede confundir a quien copie el comando tal cual; conviene actualizar el ejemplo para usar una petición GET coherente con la API real.
| curl -X POST http://localhost:8765/health | |
| curl http://localhost:8765/health |
| daemon_process = subprocess.Popen( | ||
| [PYTHON, DAEMON_SCRIPT], | ||
| DAEMON_CMD, | ||
| env=env, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE |
There was a problem hiding this comment.
Aquí se lanza el daemon con stdout=subprocess.PIPE y stderr=subprocess.PIPE, pero la salida solo se consume al final en el bloque finally, después de terminar el proceso. Si el servidor escribe suficiente log durante las pruebas, los pipes pueden llenarse y bloquear al proceso hijo, provocando timeouts difíciles de diagnosticar; sería más seguro redirigir la salida a heredar del padre o a un fichero en disco en lugar de usar PIPE sin un lector concurrente.
| print("\nShutting down Daemon...") | ||
| daemon_process.terminate() | ||
| daemon_process.wait() | ||
| try: | ||
| daemon_process.wait(timeout=5) | ||
| except subprocess.TimeoutExpired: | ||
| daemon_process.kill() |
There was a problem hiding this comment.
En el bloque finally se llama incondicionalmente a daemon_process.terminate() y luego a wait(timeout=5), incluso si el proceso ya finalizó por su cuenta. En esa situación terminate() puede lanzar ProcessLookupError y abortar el cleanup, evitando que se impriman los logs; sería más robusto comprobar daemon_process.poll() o capturar específicamente ese error antes de continuar.
| # Set PYTHONPATH to include src | ||
| env = os.environ.copy() | ||
| env["PYTHONPATH"] = str(BACKEND_DIR / "src") |
There was a problem hiding this comment.
Al sobrescribir env["PYTHONPATH"] se descarta cualquier valor previo definido por el usuario o el entorno de CI, lo que puede romper configuraciones existentes. Es más seguro extender el PYTHONPATH actual (por ejemplo, anteponiendo BACKEND_DIR / "src") en lugar de reemplazarlo por completo.
| # Set PYTHONPATH to include src | |
| env = os.environ.copy() | |
| env["PYTHONPATH"] = str(BACKEND_DIR / "src") | |
| # Set PYTHONPATH to include src, preserving any existing value | |
| env = os.environ.copy() | |
| existing_pythonpath = env.get("PYTHONPATH") | |
| src_path = str(BACKEND_DIR / "src") | |
| if existing_pythonpath: | |
| env["PYTHONPATH"] = src_path + os.pathsep + existing_pythonpath | |
| else: | |
| env["PYTHONPATH"] = src_path |
| if daemon_running: | ||
| print(f"{Colors.RED}❌ Proceso corriendo pero API NO responde (posible bloqueo){Colors.NC}") | ||
| else: | ||
| print(f"{Colors.RED}❌ PID file apunta a proceso muerto: {pid_from_file}{Colors.NC}") | ||
| else: | ||
| print(f"{Colors.YELLOW}⚠️ PID file no encontrado{Colors.NC}") | ||
| print(f"{Colors.YELLOW}⚠️ API no disponible (daemon detenido){Colors.NC}") |
There was a problem hiding this comment.
Estas dos líneas tienen un espacio extra de indentación respecto al bloque if daemon_running/else, lo que rompe la alineación visual e incumple el estilo de indentación uniforme de 4 espacios. Conviene ajustar la indentación para que los print queden al mismo nivel dentro de cada rama del condicional.
| except requests.RequestException: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except requests.RequestException: | |
| pass | |
| except requests.RequestException as exc: | |
| print(f"Health check request failed on attempt {i+1}: {exc}") |
This PR completes the transition of the Voice2Machine daemon to the SOTA 2026 architecture by removing the legacy Unix Socket IPC system and associated "dead code".
Key changes:
AGENTS.mdmarks infrastructure/core removal as complete.v2m.main.pyis now a pure FastAPI server runner. Deprecated--daemonflag and internal CLI client logic are removed.send_command.pyandverify_export_backend.py(legacy IPC) are deleted.stop_daemon.shno longer checks/cleans sockets.health_check.pynow verifies daemon health via HTTP (/health) instead of socket PING.cleanup_v2m.shremoved socket/PID file cleanup logic.common.shremovedget_runtime_dir.verify_daemon.pyis rewritten to act as an HTTP integration test for the FastAPI server.ollamafrompyproject.tomlto reduce footprint.PR created automatically by Jules for task 4075794029776603254 started by @cesarszv