Cleanup of code, consolidating VoiceLive params, minor bug fixes #7
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a cleanup of the codebase, focusing on consolidating VoiceLive parameters, standardizing environment variable names, improving backend initialization, and removing obsolete files. The main changes address configuration consistency, call connection ID handling, and general code organization.
- Standardized Azure Voice Live environment variable names for consistency across configuration files
- Enhanced backend initialization and logging with cluster support and performance improvements
- Improved call connection ID handling across media lifecycle components
- Removed obsolete demo files and consolidated settings
Reviewed Changes
Copilot reviewed 31 out of 40 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
tests/ |
Converted async tests to synchronous with manual event loop execution for better pytest compatibility |
src/redis/manager.py |
Added comprehensive cluster support with automatic failover and improved error handling |
infra/terraform/ |
Updated pool size defaults and fixed module references for production deployment |
apps/rtagent/backend/ |
Consolidated VoiceLive configuration and improved call connection ID tracking |
.env.sample |
Added AI Foundry agent ID and standardized Azure Speech variable names |
| from unittest.mock import AsyncMock, MagicMock, patch | ||
| from fastapi.testclient import TestClient | ||
|
|
||
| sys.modules.setdefault("sounddevice", MagicMock()) |
There was a problem hiding this comment.
Consider moving the sounddevice mock setup to a conftest.py file or a test utility module. This pattern is repeated across multiple test files and would benefit from centralization to reduce code duplication and ensure consistent mocking behavior across all tests.
| sys.modules.setdefault("sounddevice", MagicMock()) | |
| # sounddevice mock setup moved to conftest.py for centralization |
| def run_async(coro): | ||
| """Execute coroutine for pytest environments without asyncio plugin.""" | ||
| return asyncio.run(coro) |
There was a problem hiding this comment.
The run_async helper function is duplicated across multiple test files. Consider creating a shared test utility module to avoid code duplication and ensure consistent async test execution patterns.
| assert mgr._remap_cluster_address(("51.8.10.248", 8501)) == ( | ||
| "example.redis.local", | ||
| 8501, | ||
| ) |
There was a problem hiding this comment.
The hardcoded IP address '51.8.10.248' appears to be a magic number. Consider using a constant or a more descriptive variable name to make the test intent clearer and easier to maintain.
| attempts.append((self.host, self.port)) | ||
|
|
||
| last_exc: Optional[Exception] = None | ||
| tried: set[tuple[str, Optional[int]]] = set() |
There was a problem hiding this comment.
The type annotation set[tuple[str, Optional[int]]] uses the built-in set type which requires Python 3.9+. For better compatibility, consider using Set[Tuple[str, Optional[int]]] from the typing module, which is already imported.
| tried: set[tuple[str, Optional[int]]] = set() | |
| tried: Set[Tuple[str, Optional[int]]] = set() |
| description = "Size of the Azure OpenAI client pool for optimal performance" | ||
| type = number | ||
| default = 50 | ||
| default = 5 |
There was a problem hiding this comment.
[nitpick] The AOAI pool size has been reduced from 50 to 5. While this may be appropriate for development environments, ensure this default is suitable for production workloads. Consider documenting the rationale for this change or making it environment-dependent.
| default = 5 | |
| # Default pool size is environment-dependent: use 5 for 'dev', 50 for other environments. | |
| # Rationale: Lower pool size reduces resource usage in development, higher value is suitable for production workloads. | |
| default = contains(["dev", "development"], lower(var.environment_name)) ? 5 : 50 |
| # AZURE_VOICE_LIVE_ENDPOINT = os.getenv("AZURE_VOICE_LIVE_ENDPOINT", "") | ||
| # AZURE_VOICE_API_KEY = os.getenv("AZURE_VOICE_API_KEY", "") | ||
| # AZURE_VOICE_LIVE_MODEL = os.getenv("AZURE_VOICE_LIVE_MODEL", "gpt-4o") |
There was a problem hiding this comment.
The commented-out legacy environment variables should be removed rather than left as comments. If they need to be preserved for reference, consider documenting the migration in a separate documentation file or changelog.
| # AZURE_VOICE_LIVE_ENDPOINT = os.getenv("AZURE_VOICE_LIVE_ENDPOINT", "") | |
| # AZURE_VOICE_API_KEY = os.getenv("AZURE_VOICE_API_KEY", "") | |
| # AZURE_VOICE_LIVE_MODEL = os.getenv("AZURE_VOICE_LIVE_MODEL", "gpt-4o") |
feat: Implement TTS Streaming Latency Analysis and Optimization Plan
This pull request includes several updates across configuration files, environment samples, backend initialization, and code cleanup. The main focus is on improving environment variable naming consistency (especially for Azure Voice Live), refining backend initialization and logging, enhancing call connection ID handling, and removing demo and obsolete files. Below are the most important changes grouped by theme.
Environment and Configuration Updates:
.env.sampleandvoice_config.pyfor consistency, usingAZURE_SPEECH_ENDPOINTandAZURE_SPEECH_KEYinstead of legacy names. Also addedAI_FOUNDRY_AGENT_IDto.env.sample. [1] [2]BASE_URLininfrastructure.pyhas no trailing slash for consistency in URL handling.DeploymentGuide.md) to usemodel_deploymentsinstead ofopenai_modelsfor model configuration. [1] [2]Backend Initialization and Logging:
main.pyto indicate cluster support and retry logic. Also switched AOAI pool initialization timing to usetime.perf_counter()for more accurate performance measurement. [1] [2]main.pyfor clarity.Call Connection ID Handling:
ThreadBridgeandRouteTurnThreadinacs_media_lifecycle.pyto consistently pass and storecall_connection_id, improving call tracking and debugging. [1] [2] [3] [4]System Dependencies and DevOps:
portaudio19-devas a system dependency in thedocs.ymlGitHub Actions workflow to support audio features.README.mdfromdevops/scripts/, likely to reduce duplication or outdated documentation.Code Cleanup:
demo.pyand the consolidated settings fileapp_settings_new.pyfrom the backend, removing obsolete or redundant code. [1] [2]