feat: add Deepgram v2 (flux) support with wellness analysis

[plutoless]

No description provided.

[claude]

Pull Request Review: Deepgram v2 (flux) support with wellness analysis

Thank you for this substantial contribution! This PR adds significant new functionality including WebSocket-based ASR, wellness analysis, and avatar integrations. Here's my comprehensive review:

---

📋 Summary

Changes: 17,358 additions across 79 files

• New extensions: Deepgram WebSocket ASR, Thymia wellness analyzer, HeyGen avatar, Generic video
• New example app: voice-assistant-advanced
• Server enhancements for dynamic property injection
• Comprehensive documentation updates

---

✅ Strengths

1. Excellent Documentation: The AI_working_with_ten.md (2715 lines) provides comprehensive onboarding guidance
2. Proper Base Class Usage: Extensions correctly inherit from framework base classes (AsyncASRBaseExtension, AsyncLLMToolBaseExtension)
3. Good Configuration Management: Pydantic models for validation with environment variable support
4. WebSocket Best Practices: Proper connection management with reconnection logic and cleanup
5. Structured Logging: Good use of log categories and contextual information

---

🔴 Critical Issues

1. Security: API Keys in Logs (HIGH PRIORITY)

Location: deepgram_ws_asr_python/extension.py:169

headers = {"Authorization": f"Token {self.config.api_key}"}

Issue: API keys could be exposed if headers are logged during debugging.

Recommendation:

• Implement key encryption in logs (see CLAUDE.md line 376-388)
• Add to_str() method with sensitive_handling parameter
• Use utils.encrypt() for API keys before logging

Example from guidelines:

def to_str(self, sensitive_handling: bool = True) -> str:
    if not sensitive_handling:
        return f"{self}"
    config = copy.deepcopy(self)
    if config.params and "api_key" in config.params:
        config.params["api_key"] = utils.encrypt(config.params["api_key"])
    return f"{config}"

2. Resource Cleanup: Session Timeout in Exception Path

Location: thymia_analyzer_python/extension.py:474-480

async with self.session.close(), timeout=5.0:

Issue: If session close times out, session object is not set to None, potentially causing issues in subsequent cleanup.

Location: deepgram_ws_asr_python/extension.py:474-480

try:
    await asyncio.wait_for(self.session.close(), timeout=5.0)
except asyncio.TimeoutError:
    self.ten_env.log_warn("[DEEPGRAM-WS] Session close timed out")
self.session = None  # Good - always sets to None

Recommendation: Follow Deepgram pattern - always set session/websocket to None even after timeout.

3. Race Condition: Concurrent WebSocket Operations

Location: heygen_avatar_python/extension.py:99-106

async with self.speaking_lock:
    if not self.is_speaking:
        # ...
        if self.recorder and self.recorder.ws_connected():
            await self.recorder.interrupt()

Issue: ws_connected() check is outside lock, but WebSocket operations happen inside. Connection could close between check and use.

Recommendation:

async with self.speaking_lock:
    if not self.is_speaking and self.recorder:
        if self.recorder.ws_connected():
            try:
                await self.recorder.interrupt()
            except Exception as e:
                self.ten_env.log_warn(f"Interrupt failed: {e}")

4. Subprocess Usage in Async Context (SECURITY & RELIABILITY)

Location: thymia_analyzer_python/extension.py:363-405

# Use curl subprocess to make the request
curl_cmd = ["curl", "-X", "POST", ...]
process = await asyncio.create_subprocess_exec(*curl_cmd, ...)

Issues:

1. Command injection risk: If any config values contain shell metacharacters
2. Unnecessary complexity: Using subprocess for HTTP when aiohttp is already imported
3. Error handling: stderr is captured but never logged if subprocess fails

Recommendation: Use aiohttp consistently (like upload_audio() and get_results() methods):

async with self.session.post(
    f"{self.base_url}/v1/models/mental-wellness",
    json=payload
) as response:
    if response.status not in (200, 201):
        error_text = await response.text()
        raise ThymiaAPIError(f"Failed: {response.status} - {error_text}")
    return await response.json()

---

⚠️ High Priority Issues

5. Error Propagation: Silent Failures

Location: heygen_avatar_python/extension.py:171-173

except Exception as e:
    self.ten_env.log_error(f"Error processing audio frame: {e}")
    continue  # Silently drops frame

Issue: Audio frames are silently dropped on errors without notifying upstream.

Recommendation: Track consecutive failures and send error notification:

self.consecutive_failures = 0
MAX_CONSECUTIVE_FAILURES = 10

except Exception as e:
    self.ten_env.log_error(f"Error processing audio: {e}")
    self.consecutive_failures += 1
    if self.consecutive_failures > MAX_CONSECUTIVE_FAILURES:
        await self.send_error_notification()
    continue

6. Buffer Overflow Risk

Location: thymia_analyzer_python/extension.py:61-62

self.max_speech_duration = 300.0  # 5 minutes safety limit

Location: Line 104-108

if self.speech_duration < self.max_speech_duration:
    self.speech_buffer.append(buffered_frame)

Issue: After reaching max duration, frames are still accumulated in circular_buffer but checks stop. Buffer could grow unbounded.

Recommendation: Add memory-based limit or clear circular buffer when max is reached:

if self.speech_duration >= self.max_speech_duration:
    self.circular_buffer.clear()  # Stop accumulating
    return self.speech_duration

7. Type Inconsistency

Location: heygen_avatar_python/extension.py:44

self.audio_queue = asyncio.Queue[bytes]()  # Type hint

Location: Line 268-269

frame_buf = audio_frame.get_buf()  # Returns bytearray
self.input_audio_queue.put_nowait(frame_buf)  # Puts bytearray, not bytes

Issue: Type hint says bytes, but bytearray is actually used. Could cause issues with type checkers.

Recommendation: Update type hint or convert: bytes(frame_buf)

---

🟡 Medium Priority Issues

8. Hardcoded Configuration Values

Location: heygen_avatar_python/heygen.py:14

SESSION_CACHE_PATH = "/tmp/heygen_session_id.txt"

Issue: Hardcoded path could cause issues in containerized environments or with multiple instances.

Recommendation: Use instance-based cache path:

self.session_cache_path = f"/tmp/heygen_session_{self.channel_name}.txt"

9. Magic Numbers

Multiple locations have unexplained constants:

• thymia_analyzer_python/extension.py:50: silence_threshold=0.02 - No comment explaining why 0.02
• extension.py:74: silence_threshold_duration = 0.5 - Why 0.5 seconds?
• deepgram_ws_asr_python/extension.py:496: byte_limit=1024 * 1024 * 10 - Why 10MB?

Recommendation: Add comments or make configurable via property.json

10. Inconsistent Error Handling

Location: server/internal/worker.go:162-166

logFile, err := os.OpenFile(w.LogFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
if err != nil {
    slog.Error("Failed to open log file", "err", err, "requestId", req.RequestId, logTag)
    // return err  <-- Commented out, continues despite error
}

Issue: Log file failure is ignored, potentially causing lost logs.

Recommendation: Either fallback to stdout or return error:

if err != nil {
    slog.Error("Failed to open log file, falling back to stdout", "err", err)
    stdoutWriter = os.Stdout
    stderrWriter = os.Stderr
}

11. Potential Deadlock in Queue Operations

Location: heygen_avatar_python/extension.py:192-199

async def _clear_audio_queue(self) -> None:
    queue_size = self.input_audio_queue.qsize()
    for _ in range(queue_size):
        try:
            self.input_audio_queue.get_nowait()

Issue: qsize() is unreliable in async context - size could change between check and get.

Recommendation:

while not self.input_audio_queue.empty():
    try:
        self.input_audio_queue.get_nowait()
        cleared_count += 1
    except asyncio.QueueEmpty:
        break

---

📝 Code Quality Issues

12. Commented-Out Code

Multiple locations have large blocks of commented code:

• heygen_avatar_python/extension.py:24: # from .heygen_bak import HeyGenRecorder
• extension.py:64-70: Entire alternative recorder implementation
• heygen.py:283-285: Comment about removed debounce hack

Recommendation: Remove commented code before merging. Use git history if needed.

13. Print Statements Instead of Logging

Location: thymia_analyzer_python/extension.py:198-201

print(
    "[THYMIA_BUFFER_CLEAR] Speech buffer cleared",
    flush=True,
)

Issue: Multiple print() statements instead of using ten_env.log_*().

Impact: Logs don't follow framework conventions, harder to filter/categorize.

Recommendation: Replace all print() with self.ten_env.log_info()

14. Missing Docstrings

Classes and methods in heygen.py lack docstrings:

• AgoraHeygenRecorder class (line 13)
• _generate_token() method (line 58)
• ws_connected() method

Recommendation: Add docstrings for public methods and classes.

15. Overly Verbose Logging

Location: thymia_analyzer_python/extension.py:214-269

30+ log statements in get_wav_data() method, including multiple flush=True prints.

Issue: Excessive logging in performance-critical audio processing path.

Recommendation: Use debug level or remove after testing:

self.ten_env.log_debug(f"WAV conversion: {len(wav_data)} bytes")

---

🧪 Test Coverage

Current State:

• ✅ HeyGen extension has basic tests (heygen_avatar_python/tests/test_basic.py)
• ❌ Deepgram WS ASR: No tests found
• ❌ Thymia analyzer: No tests found
• ❌ Generic video: No tests found

Recommendations:

1. Add ASR tests (follow framework pattern):

ai_agents/agents/ten_packages/extension/deepgram_ws_asr_python/tests/
├── bin/start
├── conftest.py
└── test_basic.py

2. Add wellness analyzer tests:

• Mock Thymia API responses
• Test audio buffer edge cases (silence detection, max duration)
• Test RMS calculation with various audio levels

3. Integration tests: Consider adding to ASR guarder framework (see CLAUDE.md lines 463-476)

---

🔒 Security Considerations

Current State:

• ✅ Environment variable usage for API keys
• ✅ API keys not hardcoded
• ⚠️ Potential log exposure (see issue Update README.md #1)
• ⚠️ Command injection risk (see issue Updating texts and adding gif #4)

Additional Recommendations:

1. Input Validation: Add validation for channel_name to prevent injection

import re
if not re.match(r'^[a-zA-Z0-9_-]+$', channel_name):
    raise ValueError("Invalid channel name")

2. Rate Limiting: Consider adding rate limiting for API calls to Thymia/HeyGen

3. Error Messages: Avoid exposing internal paths in error messages sent to clients

---

🚀 Performance Considerations

Observed Issues:

1. Synchronous HTTP in Async Code: heygen.py uses requests library (blocking) in async context

   • Lines 134-139, 173-178, 196-202, 215-221
   • Impact: Blocks event loop during API calls
   • Fix: Replace with aiohttp (already used in Thymia extension)

2. Inefficient String Concatenation: thymia_analyzer_python/extension.py:252

pcm_data = b"".join(frames_list)  # Could be slow for 5 min audio

• Impact: Memory spike for large buffers
• Optimization: Use BytesIO or stream processing

3. Unnecessary Copies: heygen_avatar_python/extension.py:124

audio_data = np.frombuffer(audio_frame, dtype=np.int16)  # Creates copy

• Consider zero-copy alternatives if performance becomes an issue

Performance Test Recommendations:

1. Test Deepgram WS extension with 60+ minute sessions (max_speech_duration edge case)
2. Load test HeyGen avatar with concurrent TTS streams
3. Measure Thymia buffer memory usage with max duration audio

---

📚 Documentation Review

The new documentation (AI_working_with_ten.md) is excellent and comprehensive. Minor suggestions:

1. Line 482: Add link to environment variable reference
2. Line 685: Clarify when PYTHONPATH needs manual setting vs. automatic
3. Consider adding troubleshooting section for new extensions

---

✅ Approval Recommendation

Status: Approve with required changes

Required before merge:

1. ✅ Fix security issue Update README.md #1 (API key logging)
2. ✅ Fix security issue Updating texts and adding gif #4 (subprocess command injection)
3. ✅ Fix critical race condition Fix speech sdk #3
4. ✅ Add tests for new extensions

Recommended before merge:

• Address resource cleanup issues (modify playground default settings，fix voice type error #2)
• Fix error propagation (Dev/zhangtao #5)
• Remove commented code ([Doc] README Overhaul - adding the M1 with Rosetta section #12)
• Replace print() with logging ([Doc] README redesign diagrams light and dark themes. #13)

Can be addressed in follow-up PR:

• Performance optimizations
• Additional test coverage
• Documentation enhancements

---

🎯 Priority Summary

Priority	Count	Must Fix Before Merge
Critical	4	✅ Yes
High	7	⚠️ Recommended
Medium	4	❌ No
Quality	4	❌ No

---

Final Notes

This is a substantial and well-structured PR that adds important functionality. The code quality is generally good, following framework patterns correctly. The main concerns are around security (API key logging, command injection) and resource management (race conditions, cleanup). Once the critical issues are addressed, this will be a solid contribution to the project.

Great work on the comprehensive documentation - it will significantly help future developers working with the TEN Framework!

Reviewed by: Claude Code
Review Date: 2025-11-11

[claude]

Code Review: PR #1691 - Add Deepgram Flux v2 & Wellness Analysis Features

Thank you for this comprehensive PR! I've completed a thorough review covering code quality, potential bugs, performance, security, and test coverage. Overall, this is a well-structured addition with good async patterns and proper configuration handling.

---

📊 Summary

What's Being Added:

• 4 new extensions: deepgram_ws_asr_python, thymia_analyzer_python, heygen_avatar_python, generic_video_python
• New voice-assistant-advanced example with 7 graph variants
• Comprehensive documentation (AI_working_with_ten.md, AI_working_with_ten_compact.md)
• Playground UI enhancements for video support

Scale: 79 files changed, +17,358 lines

---

✅ Strengths

1. Excellent Documentation: The AI_working_with_ten.md files are comprehensive and well-organized
2. Proper Configuration: All API keys correctly use ${env:VAR_NAME} syntax
3. Good Async Patterns: Proper use of async/await, asyncio.Queue, and connection management
4. Multi-Version Support: Deepgram extension cleanly supports both v1 (Nova) and v2 (Flux) APIs
5. Integration Tests: voice-assistant-advanced includes good graph validation tests

---

🔴 Critical Issues

None found that would block merging.

---

🟡 High Priority Issues

1. Subprocess Usage in thymia_analyzer_python (Security)

File: ai_agents/agents/ten_packages/extension/thymia_analyzer_python/extension.py:362-406

Currently uses subprocess curl for API calls instead of aiohttp:

curl_cmd = ["curl", "-X", "POST", f"{self.base_url}/v1/models/mental-wellness", ...]
process = await asyncio.create_subprocess_exec(*curl_cmd, ...)

Issue: Introduces unnecessary security risk and is inconsistent with the aiohttp pattern used elsewhere (apollo_api.py).

Recommendation: Replace with aiohttp (pattern already demonstrated in apollo_api.py):

async with self.session.post(
    f"{self.base_url}/v1/models/mental-wellness",
    json=payload,
    headers={"x-api-key": self.api_key}
) as response:
    if response.status not in (200, 201):
        raise ThymiaAPIError(f"Failed: {response.status}")
    return await response.json()

2. Bug in heygen_avatar_python

File: ai_agents/agents/ten_packages/extension/heygen_avatar_python/extension.py:177

def _dump_audio_if_need(self, buf: bytearray) -> None:
    with open("{}_{}.pcm".format("tts", self.config.agora_channel_name), "ab") as dump_file:

Issue: References undefined field self.config.agora_channel_name (should be self.config.channel). This function appears to be dead code as it's never called.

Recommendation: Remove this dead code entirely.

3. Large File Needs Refactoring

File: thymia_analyzer_python/extension.py (2,794 lines)

Issue: Single file with 123KB of code makes maintenance difficult.

Recommendation: Split into modules:

• audio_buffer.py - AudioBuffer class
• wellness_metrics.py - WellnessMetrics and analysis logic
• api_client.py - ThymiaAPIClient
• extension.py - Main extension logic

---

🟠 Medium Priority Issues

1. Fire-and-Forget Tasks Without Exception Handling

Found in all extensions:

• deepgram_ws_asr_python/extension.py:184
• thymia_analyzer_python/extension.py:222
• heygen_avatar_python/extension.py:84
• generic_video_python/extension.py:128

Pattern:

asyncio.create_task(self._some_background_task())  # Not stored or awaited

Issue: Exceptions in background tasks will be silently swallowed.

Recommendation:

self.background_task = asyncio.create_task(self._some_background_task())

# In on_stop():
if self.background_task:
    self.background_task.cancel()
    try:
        await self.background_task
    except asyncio.CancelledError:
        pass

2. Infinite Loop Without Timeout

File: generic_video_python/extension.py:170-172

async def _wait_for_recorder_ready(self):
    while self.recorder.connected is False:
        await asyncio.sleep(0.5)

Issue: If recorder never connects, loop runs forever.

Recommendation: Add timeout:

timeout = 30  # seconds
start_time = asyncio.get_event_loop().time()
while self.recorder.connected is False:
    if asyncio.get_event_loop().time() - start_time > timeout:
        raise TimeoutError("Recorder failed to connect")
    await asyncio.sleep(0.5)

3. Silent Exception Suppression

File: thymia_analyzer_python/extension.py:563-564, 669-670

except Exception:
    pass

Issue: Silently hides initialization errors.

Recommendation: At minimum, log the errors:

except Exception as e:
    ten_env.log_error(f"Failed to load property: {e}")

4. Test Coverage Gaps

Status: Only 1 of 4 extensions has tests (heygen_avatar_python).

Missing tests:

• deepgram_ws_asr_python (20KB of code, 0 tests)
• thymia_analyzer_python (123KB of code, 0 tests) ⚠️ Highest risk
• generic_video_python (35KB of code, 0 tests)

Recommendation: Add unit tests following the heygen_avatar_python pattern:

• tests/conftest.py with pytest fixtures
• tests/test_basic.py with ExtensionTester
• tests/bin/start executable script

---

🟢 Low Priority / Nice-to-Haves

1. Add explicit SSL/TLS contexts to WebSocket connections for defense-in-depth
2. Remove debug artifacts: Print statements and commented code in production files
3. Add circuit breaker for JSON decode errors in deepgram (if too many errors, reconnect)
4. File permissions: Add os.chmod(path, 0o600) to session cache files
5. Logging consistency: Some extensions mix print() and ten_env.log_*() calls

---

📈 Performance Considerations

Positive:

• Good use of asyncio for concurrent operations
• Proper WebSocket connection reuse
• Audio buffering with circular buffers prevents memory bloat

Potential Improvements:

1. thymia_analyzer_python: Consider using streaming API if available instead of uploading large audio files
2. Audio resampling: heygen uses manual resampling (lines 135-154) - consider scipy for better quality
3. Connection pooling: Ensure aiohttp sessions are reused across requests

---

🔒 Security Assessment

✅ Good Practices:

• All API keys use environment variables (no hardcoded secrets)
• Proper timeout configuration on connections
• No sensitive data exposed in error messages
• Audio buffer limits prevent memory exhaustion

⚠️ Concerns Addressed Above:

• Subprocess curl usage (HIGH priority)
• WebSocket SSL contexts (MEDIUM priority)
• File permissions on cache files (LOW priority)

---

🧪 Test Coverage Summary

Extension	Tests?	Coverage Level
deepgram_ws_asr_python	❌ No	0%
thymia_analyzer_python	❌ No	0% (highest risk - 123KB code)
heygen_avatar_python	✅ Yes	Basic unit test
generic_video_python	❌ No	0%
voice-assistant-advanced	✅ Yes	Integration tests (config validation)

Overall: 25% extension test coverage (1/4), plus integration tests for the example.

---

📝 Specific File References

Bugs:

• heygen_avatar_python/extension.py:177 - Wrong field name in dead code

Security:

• thymia_analyzer_python/extension.py:362-406 - Subprocess curl usage

Code Quality:

• thymia_analyzer_python/extension.py - 2794 lines, needs refactoring
• All extensions - Fire-and-forget tasks need exception handling
• generic_video_python/extension.py:170-172 - Infinite loop without timeout

Tests:

• Add tests to: deepgram_ws_asr_python/, thymia_analyzer_python/, generic_video_python/

---

🎯 Recommendations Priority

Before Merge:

1. Fix heygen bug (wrong field name) - 5 min fix
2. Remove dead code (_dump_audio_if_need) - 5 min fix

For Next Iteration:

1. Replace subprocess curl with aiohttp in thymia (security/consistency)
2. Add exception handling to fire-and-forget tasks
3. Add timeout to _wait_for_recorder_ready loop
4. Add tests for the 3 untested extensions

Long Term:

1. Refactor thymia_analyzer_python into modules
2. Improve logging consistency across extensions
3. Add SSL contexts to WebSocket connections

---

✅ Conclusion

This is a solid PR that adds significant functionality with good async patterns and proper configuration management. The code is generally well-structured and follows framework conventions.

Recommendation: Approve with minor fixes - The two small bugs (heygen field name, dead code removal) should be fixed before merge. The other issues can be addressed in follow-up PRs as they don't block functionality.

Great work on the comprehensive documentation and integration tests! The voice-assistant-advanced example provides excellent validation of the graph configurations.

---

Review generated using Claude Code with analysis of all 79 changed files, focusing on code quality, security, performance, and test coverage per repository CLAUDE.md guidelines.

[claude]

Comprehensive Code Review: PR #1691

Executive Summary

This PR adds 81 files with 16,313 additions including 4 new extensions. While showing solid async understanding, there are 5 critical and 7 high-priority issues requiring fixes before merge.

🔴 Critical Issues (Must Fix)

1. Untracked Background Tasks

Files: thymia_analyzer_python/extension.py (lines 884, 1075, 1149, 1153, 2702)
Issue: Multiple asyncio.create_task() calls without storing references leads to resource leaks and zombie tasks.
Fix: Track tasks and cancel in on_stop().

2. subprocess curl Usage

File: thymia_analyzer_python/extension.py:363-406
Issue: API keys exposed in command-line args (visible in ps), depends on external binary.
Fix: Replace with aiohttp.

3. Race Condition in Connection

File: deepgram_ws_asr_python/extension.py:156-197
Issue: start_connection() may deadlock calling stop_connection() within same lock.

4. API Keys in Command Args

File: thymia_analyzer_python/extension.py:363-377
Issue: API keys visible in ps output - security violation.

5. Blocking HTTP Calls

Files: heygen_avatar_python/heygen.py:134-216, generic_video_python/generic.py:240-306
Issue: Synchronous requests.post() with 30s timeouts blocks event loop, drops audio frames.
Fix: Replace with async aiohttp.

🟠 High Priority

1. Wrong requirements.txt

File: thymia_analyzer_python/requirements.txt
Lists 'asyncio' as dependency - it's stdlib! Remove it.

2. Missing requirements.txt in Manifest

File: thymia_analyzer_python/manifest.json
Won't install dependencies when packaged.

3. Missing Version Pins

Files: heygen/generic requirements.txt
scipy, numpy lack versions.

4-7. Other High Issues

• Session cleanup timeout handling
• Buffer memory growth
• API keys in logs
• Missing tests for 3 extensions

🟡 Medium Priority

12 issues including: debug logging, magic numbers, inefficient resampling, god object pattern (2,794 lines).

✅ Strengths

• Comprehensive features
• Good async patterns mostly
• Proper TEN integration
• Excellent documentation

🎯 Recommendation

Do NOT merge until fixing:

• All 5 Critical issues
• At least 5/7 High issues

Effort: 8-16 hours

---

Claude Code Review | 2025-11-11 | Commit f94189c

[claude]

PR Review: Deepgram v2 (Flux) Support with Wellness Analysis

Summary

This is a substantial PR adding 85 files with 16,397 additions that introduces:

• Deepgram v2 (Flux) ASR support with WebSocket implementation
• New voice-assistant-advanced example application
• Three new video/avatar extensions (generic_video, heygen_avatar, thymia_analyzer)
• Comprehensive documentation (2,715+ lines)
• Playground UI enhancements
• Server-side property injection improvements

---

✅ Strengths

1. Excellent Documentation ⭐

• AI_working_with_ten.md (2,715 lines) provides comprehensive onboarding with detailed explanations
• AI_working_with_ten_compact.md (828 lines) offers quick reference commands
• Clear separation between detailed learning docs and quick reference
• Includes troubleshooting, architecture details, and best practices

2. Well-Structured Deepgram WebSocket Extension

File: ai_agents/agents/ten_packages/extension/deepgram_ws_asr_python/extension.py

Strengths:

• Proper async/await patterns with asyncio
• Connection lifecycle management with locks (_connection_lock)
• Clean separation of v1 (Nova) vs v2 (Flux) API handling
• Good error handling with try/except blocks
• Voice activity detection with confidence filtering

Good practices observed:

# Line 161: Proper locking for connection state
async with self._connection_lock:
    await self.stop_connection()
    
# Line 286: Smart filtering for false positives
if word_count == 1 and elapsed_time < 5.0:
    # Drop single-word results during echo cancel settling

3. Robust Audio Processing in Thymia Analyzer

File: ai_agents/agents/ten_packages/extension/thymia_analyzer_python/extension.py

• Sophisticated audio buffering with VAD (Voice Activity Detection)
• Pre-speech capture using circular buffer (deque for O(1) performance)
• Natural onset/offset detection with 0.5s silence threshold
• Memory safety with max duration limits (5 minutes)
• Efficient RMS volume calculation

Performance consideration:

# Line 64: Using deque for efficient circular buffer
self.circular_buffer = deque()  # O(1) popleft() vs list O(n)

4. Server-Side Property Auto-Injection ⭐

File: ai_agents/server/internal/http_server.go (lines 646-690)

Excellent architectural decision to implement property-based auto-injection:

• Automatically injects channel_name to all nodes with a "channel" property
• Future-proof: new extensions automatically receive dynamic values
• No hardcoding of extension names required
• Type-safe configuration mapping

Benefits:

• Adding new avatar/video extensions requires no server code changes
• Clean separation of concerns
• Reduces configuration boilerplate

5. Good Test Coverage

• Test files for voice-assistant-advanced example
• Test utilities (test_start_api.py, test_graphs_api.py, test_properties_override.py)
• Basic extension tests for heygen_avatar_python
• Property override validation scripts

---

⚠️ Issues & Recommendations

1. Security: API Key Handling 🔒

Issue: API keys visible in configuration logs
Location: deepgram_ws_asr_python/extension.py:169

headers = {"Authorization": f"Token {self.config.api_key}"}

Recommendation:

• Implement API key masking in logs (see existing pattern in rime_http_tts)
• Add to_str() method with encrypt() utility
• Never log full API keys in debug/info statements

Example from CLAUDE.md (line 380):

def to_str(self, sensitive_handling: bool = True) -> str:
    if not sensitive_handling:
        return f"{self}"
    config = copy.deepcopy(self)
    if config.params and "api_key" in config.params:
        config.params["api_key"] = utils.encrypt(config.params["api_key"])
    return f"{config}"

2. Type Safety: Missing Type Hints

Files affected:

• thymia_analyzer_python/extension.py (lines 78-158)
• generic_video_python/extension.py (lines 140-167)

Current:

def add_frame(self, pcm_data: bytes):  # Missing return type
    # ...
    return self.speech_duration

Recommended:

def add_frame(self, pcm_data: bytes) -> float:
    # ...
    return self.speech_duration

Impact: Reduces IDE autocomplete effectiveness and makes code harder to maintain

3. Error Handling: Missing Graceful Degradation

Location: heygen_avatar_python/extension.py:90-106

async def _loop_input_audio_sender(self, _: AsyncTenEnv):
    while True:  # ⚠️ Infinite loop with no error recovery
        audio_frame = await self.input_audio_queue.get()
        # ... processing ...

Issues:

• No try/except around the entire loop
• Infinite loop with no exit condition
• Single error could crash the entire audio processing

Recommended:

async def _loop_input_audio_sender(self, _: AsyncTenEnv):
    while self._audio_processing_enabled:  # Add flag for clean shutdown
        try:
            audio_frame = await self.input_audio_queue.get()
            # ... processing ...
        except asyncio.CancelledError:
            self.ten_env.log_info("Audio processing task cancelled")
            break
        except Exception as e:
            self.ten_env.log_error(f"Error in audio loop: {e}")
            await asyncio.sleep(0.1)  # Brief pause before retry

4. Performance: Inefficient String Concatenation

Location: thymia_analyzer_python/extension.py:252

pcm_data = b"".join(frames_list)  # ⚠️ Can be slow for large buffers

Issue: For 300 seconds of audio (5 min max), this could join 30,000+ frames

Recommendation:

• Pre-allocate bytearray if total size is known
• Or use io.BytesIO for incremental building

Better approach:

import io
pcm_buffer = io.BytesIO()
for frame in frames_list:
    pcm_buffer.write(frame)
pcm_data = pcm_buffer.getvalue()

5. Code Duplication: Audio Queue Clearing

Locations:

• heygen_avatar_python/extension.py:190-200
• generic_video_python/extension.py:174-188

Both extensions have nearly identical _clear_audio_queue() methods.

Recommendation:

• Extract to shared utility function in ten_ai_base
• Reduces maintenance burden
• Ensures consistent behavior

6. Configuration Validation Inconsistency

Issue: Different validation patterns across extensions

• deepgram_ws_asr: Uses Pydantic model validation ✅
• generic_video: Custom validate_params() method
• heygen_avatar: No explicit validation ❌

Recommendation:

• Standardize on Pydantic model validation across all extensions
• Add model_validator decorators for complex constraints
• Document validation pattern in CLAUDE.md

7. Playground UI: Missing Error States

File: ai_agents/playground/src/components/Agent/View.tsx:18

const subscribedVolumes = useMultibandTrackVolume(audioTrack, 12);

Issue: No null/undefined check before passing audioTrack

Potential TypeScript error:

Type 'IRemoteAudioTrack | undefined' is not assignable to parameter type 'IMicrophoneAudioTrack | MediaStreamTrack | undefined'

Recommendation:

const subscribedVolumes = useMultibandTrackVolume(
  audioTrack as IMicrophoneAudioTrack | undefined, 
  12
);

Or update the hook signature to accept IRemoteAudioTrack.

8. Documentation: Date Inconsistency

File: ai/AI_working_with_ten.md:3

**Last Updated**: 2025-11-05

Issue: Date is in the future (should be 2024-11 or use current date)

9. Worker Management: Potential Race Condition

File: ai_agents/server/internal/worker.go:256-268

Issue: Graceful shutdown timeout check could have race condition

for i := 0; i < gracefulTimeout*10; i++ {
    err = syscall.Kill(-w.Pid, 0)  // Check if process exists
    if err != nil {
        // Process gone
        return nil
    }
    time.Sleep(100 * time.Millisecond)
}

Edge case: Process could respawn with same PID between check and kill

Recommendation:

• Store process start time
• Verify both PID and start time match
• Or use process group session ID for additional validation

10. Missing Environment Variable Documentation

New extensions require new keys:

• HEYGEN_API_KEY
• GENERIC_VIDEO_API_KEY
• THYMIA_API_KEY (implied)

Issue: Not listed in .env.example or PERSISTENT_KEYS_CONFIG.md

Recommendation:

• Add to ai_agents/.env.example
• Update ai_agents/PERSISTENT_KEYS_CONFIG.md
• Document in AI_working_with_ten.md environment section

---

🧪 Test Coverage Assessment

✅ What's Tested:

• Basic extension lifecycle (heygen_avatar_python)
• Graph API endpoint validation
• Property override simulation
• Start API behavior

❌ What's Missing:

• Unit tests for audio buffering logic (thymia_analyzer)
• Integration tests for Deepgram WebSocket reconnection
• Error scenario tests (network failures, invalid API keys)
• Performance tests for large audio buffers
• End-to-end tests for voice-assistant-advanced graphs

Recommendation:

• Add pytest-based unit tests for AudioBuffer class
• Add integration tests using ASR guarder framework
• Test Deepgram Flux-specific features (EOT threshold, eager EOT)

---

📊 Code Quality Metrics

Category	Rating	Notes
Documentation	⭐⭐⭐⭐⭐	Exceptional - comprehensive guides
Architecture	⭐⭐⭐⭐⭐	Property injection pattern is excellent
Error Handling	⭐⭐⭐☆☆	Good but inconsistent across extensions
Type Safety	⭐⭐⭐☆☆	Missing return type hints in places
Security	⭐⭐⭐☆☆	API keys need better masking
Test Coverage	⭐⭐⭐☆☆	Basic tests present, needs more coverage
Performance	⭐⭐⭐⭐☆	Generally good, minor optimizations possible

---

🎯 Recommendations Priority

High Priority (Should fix before merge):

1. ✅ Mask API keys in logs (security)
2. ✅ Fix infinite loops to use cancellable flags
3. ✅ Add missing environment variables to .env.example
4. ✅ Fix future date in documentation

Medium Priority (Fix soon after merge):

5. Add comprehensive unit tests for audio processing
6. Standardize configuration validation across extensions
7. Extract duplicated code to shared utilities
8. Add missing type hints

Low Priority (Nice to have):

9. Optimize audio buffer concatenation for large files
10. Add integration tests for error scenarios

---

📝 Summary

This is a high-quality PR that adds significant value to the TEN Framework:

Major Wins:

• ⭐ Exceptional documentation (2,715 lines of comprehensive guides)
• ⭐ Smart server-side property injection architecture
• ✅ Well-structured Deepgram v2 (Flux) implementation
• ✅ Sophisticated audio processing with VAD
• ✅ Good async/await patterns

Areas for Improvement:

• 🔒 API key security (masking in logs)
• 🛡️ Error handling consistency
• 🧪 Test coverage expansion
• 📦 Type safety improvements

Recommendation: APPROVE with minor changes

The core implementation is solid, and the identified issues are mostly polish items that can be addressed incrementally. The documentation alone makes this PR valuable for the community.

Great work! 🚀

---

Reviewed with: TEN Framework CLAUDE.md conventions
Review Date: 2025-01-12

[claude]

Code Review: PR #1691

Overview

This PR adds 4 new extensions, voice-assistant-advanced example with 7 graph variants, comprehensive documentation, and playground improvements. Scope: 85 files, 16,398 additions.

Assessment: Near production-ready with critical issues that must be addressed

---

Critical Issues (Must Fix)

1. Debug File I/O in Production (thymia_analyzer_python)

Lines 540-564, 655-670: Synchronous file writes to /tmp in async code

• Performance degradation
• Disk usage accumulation
  Fix: Remove or gate behind debug flag

2. Subprocess Security Risk (thymia_analyzer_python)

Lines 363-406: Using curl subprocess instead of aiohttp

• Attack surface expansion
• Performance overhead
  Fix: Replace with aiohttp (already imported)

3. TLS Verification Disabled (server/internal/http_client.go:14)

InsecureSkipVerify: true for ALL HTTP calls

• HIGH SECURITY RISK for production
  Fix: Enable verification or make configurable

---

High Priority Issues

4. Unbounded Queues (heygen/generic_video)

asyncio.Queue() with no maxsize can cause OOM
Fix: Set maxsize=100

5. AudioBuffer Growth (thymia)

No byte-size limit, only 300s duration check
Fix: Add explicit byte limit

6. Goroutine Leak (worker.go:216-231)

No context cancellation in worker monitor
Fix: Add context handling

7. CORS Wildcard (middleware.go)

Access-Control-Allow-Origin: * allows all origins
Fix: Make configurable per environment

8. Frontend Message Cache Leak (rtc.ts:192-289)

Messages with total_parts=-1 never cleaned
Fix: Add timeout cleanup

---

Strengths

• Excellent resource cleanup in all extensions
• Good error propagation patterns
• Proper async/await usage
• Security hardening (path traversal protection)
• Comprehensive documentation (2700-line guide)
• generic_video_python is exemplary code quality

Areas for Improvement

• thymia_analyzer_python: 2000+ lines, needs splitting
• Magic numbers without constants
• Dead code in heygen extension
• API keys in logs need sanitization

---

Test Coverage

Good: deepgram_ws_asr_python, heygen_avatar_python have tests
Missing: thymia_analyzer_python (2000+ lines, NO tests), generic_video_python

Recommendation: Add tests for API errors, buffer edge cases, async cleanup

---

Performance

• Good: Efficient websockets, proper timeouts, async queues
• Bad: Sync file I/O in async code, subprocess overhead

---

Security Summary

Critical: TLS disabled, subprocess risk
Medium: CORS wildcard, API keys in logs
Good: Path traversal protection, config validation

---

Recommendations

Must Fix Before Merge:

• Remove debug file I/O
• Replace curl with aiohttp
• Fix TLS verification
• Add thymia tests

Should Fix:

• Bounded queues
• AudioBuffer limits
• Goroutine context
• CORS config
• Message cache cleanup

Nice to Have:

• React error boundaries
• More tests
• Architecture docs
• Split large files

---

Scorecard

• Code Quality: 7/10
• Security: 4/10 (TLS critical)
• Tests: 6/10
• Docs: 9/10
• Performance: 7/10
• Error Handling: 8/10
  Overall: 6.8/10

---

Verdict

Substantial, valuable contribution with excellent documentation and good practices. However, 3 critical security/stability issues must be fixed before production.

Recommendation: Request changes for critical issues, then approve.

Great work overall!