Feat/ha plugin #248
Feat/ha plugin #248
Conversation
- Updated wizard.py to read Obsidian/Neo4j configuration from config.yml, enhancing flexibility and error handling. - Refactored ChronicleSetup to utilize ConfigManager for loading and verifying config.yml, ensuring a single source of truth. - Improved user feedback for missing configuration files and streamlined the setup process for memory and transcription providers.
- Updated `services.py` to allow service restart with an option to recreate containers, addressing WSL2 bind mount issues. - Added new chat configuration management functions in `system_controller.py` for loading, saving, and validating chat prompts. - Introduced `ChatSettings` component in the web UI for admin users to manage chat configurations easily. - Updated API service methods in `api.ts` to support chat configuration endpoints. - Integrated chat settings into the system management page for better accessibility.
…ion logging - Updated `start.sh` to improve shutdown handling by explicitly killing the backend process if running. - Modified `chat_service.py` to enhance logging for loading chat system prompts, providing clearer feedback on configuration usage. - Added a new `chat` field in `model_registry.py` for better chat service configuration management. - Updated vector store query parameters in `vector_stores.py` for improved clarity and functionality. - Enhanced the chat component in the web UI to conditionally auto-scroll based on message sending status.
…management - Introduced a new plugin architecture to allow for extensibility in the Chronicle application. - Added Home Assistant plugin for controlling devices via natural language commands triggered by wake words. - Implemented plugin configuration management endpoints in the API for loading, saving, and validating plugin settings. - Enhanced the web UI with a dedicated Plugins page for managing plugin configurations. - Updated Docker Compose files to include Tailscale integration for remote service access. - Refactored existing services to support plugin interactions during conversation and memory processing. - Improved error handling and logging for plugin initialization and execution processes.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughIntroduces a comprehensive plugin system enabling extensible transcript and conversation processing with a Home Assistant integration, integrates Tailscale for secure networking, and adds YAML-based configuration management via new admin endpoints and frontend UI components. Changes
Sequence DiagramssequenceDiagram
actor User
participant App as App Lifespan
participant PluginService as Plugin Service
participant PluginRouter as Plugin Router
participant Plugin as Plugin Instance
Note over User,Plugin: Plugin Initialization (Startup)
User->>App: Start Application
App->>PluginService: init_plugin_router()
activate PluginService
PluginService->>PluginService: Load /app/plugins.yml
loop for each enabled plugin
PluginService->>PluginRouter: register_plugin(id, instance)
PluginRouter->>Plugin: __init__(config)
end
PluginService->>PluginRouter: Store in app.state
App->>Plugin: initialize() [async in lifespan]
activate Plugin
Plugin->>Plugin: Setup (connect to services)
deactivate Plugin
PluginService-->>App: ready
deactivate PluginService
sequenceDiagram
participant Worker as Worker Job
participant PluginRouter as Plugin Router
participant Trigger as Plugin (transcript)
participant Service as External Service
Note over Worker,Service: Transcript Plugin Trigger Flow
Worker->>PluginRouter: trigger_plugins(access_level='transcript', user_id, data)
activate PluginRouter
PluginRouter->>PluginRouter: Find plugins for access_level
PluginRouter->>Trigger: _should_trigger(plugin, data)
rect rgb(200, 220, 255)
Note over PluginRouter,Trigger: Trigger condition check
alt wake_word trigger
Trigger-->>PluginRouter: Extract command from transcript
else always trigger
Trigger-->>PluginRouter: return True
end
end
PluginRouter->>Trigger: on_transcript(context)
activate Trigger
alt Plugin processes successfully
Trigger->>Service: Make external calls
Service-->>Trigger: Success
Trigger-->>PluginRouter: PluginResult(success=true, should_continue=true)
else Plugin error
Trigger-->>PluginRouter: PluginResult(success=false, message=error)
end
deactivate Trigger
PluginRouter->>PluginRouter: Check should_continue
deactivate PluginRouter
PluginRouter-->>Worker: [PluginResult, ...]
Worker->>Worker: Log plugin results, continue processing
sequenceDiagram
participant Context as Plugin Context
participant HAPlugin as HA Plugin
participant Cache as Entity Cache
participant LLMParser as LLM Parser
participant MCP as MCP Client
participant HA as Home Assistant
Note over Context,HA: Home Assistant Plugin Transcript Flow
Context->>HAPlugin: on_transcript(PluginContext)
activate HAPlugin
HAPlugin->>Cache: _ensure_cache_initialized()
alt cache_initialized=false
Cache->>MCP: fetch_areas(), fetch_entity_states()
MCP->>HA: Query areas & entities
HA-->>MCP: Return data
MCP-->>Cache: Update EntityCache
end
rect rgb(255, 240, 200)
Note over HAPlugin,LLMParser: Command Parsing (Hybrid)
HAPlugin->>LLMParser: _parse_command_with_llm(transcript)
alt LLM succeeds
LLMParser-->>HAPlugin: ParsedCommand
else LLM timeout/error
HAPlugin->>HAPlugin: _parse_command_fallback(transcript)
HAPlugin-->>HAPlugin: ParsedCommand
end
end
rect rgb(200, 255, 220)
Note over HAPlugin,Cache: Entity Resolution
HAPlugin->>Cache: _resolve_entities(ParsedCommand)
Cache-->>HAPlugin: [entity_ids]
end
rect rgb(255, 220, 220)
Note over HAPlugin,HA: Service Execution
HAPlugin->>MCP: call_service(domain, service, entity_ids)
MCP->>HA: POST /api/services/{domain}/{service}
HA-->>MCP: Success/Error
MCP-->>HAPlugin: Result
end
HAPlugin-->>Context: PluginResult(success, message, should_continue)
deactivate HAPlugin
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Rationale: This PR introduces multiple interconnected subsystems (plugin architecture, Home Assistant integration, configuration management, UI components) across heterogeneous file types (Python backend, TypeScript frontend, Docker, config files). While individual changes within each cohort follow consistent patterns, the breadth of functionality—spanning lifecycle management, event-driven architecture, MCP client interactions, entity caching with fuzzy matching, LLM-based parsing with fallback logic, and dual frontend/API layers—requires careful reasoning about integration points, error handling, and state consistency across the application stack. Possibly Related PRs
Suggested Reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
status.py (1)
78-79: Log JSON decode errors instead of silently ignoring them.Silent error handling violates the coding guideline to "always raise/log errors, never silently ignore." While continuing to process remaining lines is reasonable, malformed JSON should be logged for debugging purposes.
As per coding guidelines and retrieved learnings, use
logging.exception()in except blocks.🔎 Proposed fix
Add logging import at the top of the file:
import json import requests +import logging from pathlib import PathThen update the exception handler:
try: container = json.loads(line) container_name = container.get('Name', 'unknown') # Skip test containers - they're not part of production services if '-test-' in container_name.lower(): continue containers.append({ 'name': container_name, 'state': container.get('State', 'unknown'), 'status': container.get('Status', 'unknown'), 'health': container.get('Health', 'none') }) except json.JSONDecodeError: + logging.warning(f"Failed to parse container JSON line: {line}") continuebackends/advanced/src/advanced_omi_backend/services/memory/providers/vector_stores.py (3)
98-99: Replace bareexceptwith specific exception handling.The bare
except: passsilently suppresses all exceptions, violating the coding guideline to "always raise errors, never silently ignore." This can hide critical errors during collection deletion.🔎 Proposed fix
- except: - pass # Collection might not exist + except Exception as e: + memory_logger.warning(f"Failed to delete collection during recreation: {e}")
247-247: Remove duplicateuuidimport.The
uuidmodule is already imported at the top of the file (line 11). Per coding guidelines, "ALL imports must be at the top of the file after the docstring. NEVER import modules in the middle of functions or files."🔎 Proposed fix
- import uuid try: # Try to parse as UUID first uuid.UUID(memory_id)
328-328: Remove duplicateuuidimport.The
uuidmodule is already imported at the top of the file (line 11). Per coding guidelines, imports must remain at the top level.🔎 Proposed fix
- import uuid try: # Try to parse as UUID first uuid.UUID(memory_id)
🧹 Nitpick comments (31)
services.py (1)
277-278: Update the docstring to document the new parameter.The
recreateparameter should be documented in the function's docstring for clarity and maintainability.🔎 Proposed docstring update
def restart_services(services, recreate=False): - """Restart specified services""" + """Restart specified services + + Args: + services: List of service names to restart + recreate: If True, perform full down + up to recreate containers. + If False, perform quick restart via docker compose restart. + """status.py (2)
66-70: Consider making the test container filter pattern configurable.The
-test-substring check is reasonable but relies on a naming convention. For improved maintainability, consider making this pattern configurable (e.g., via a constant or config).🔎 Suggested improvement
+# Pattern for identifying test containers to exclude from status +TEST_CONTAINER_PATTERN = '-test-' + def get_container_status(service_name: str) -> Dict[str, Any]: """Get Docker container status for a service""" # ... existing code ... container = json.loads(line) container_name = container.get('Name', 'unknown') # Skip test containers - they're not part of production services - if '-test-' in container_name.lower(): + if TEST_CONTAINER_PATTERN.lower() in container_name.lower(): continue
179-185: Clarify or implement the logging mentioned in the comment.The comment states "Unknown status - log it for debugging" but no logging occurs. Either add logging for unknown statuses or update the comment to reflect that a default icon is used.
🔎 Proposed fix (option 1: add logging)
+ elif status['container_status'] in ['error', 'timeout']: + container_icon = "⚫" else: # Unknown status - log it for debugging + logging.warning(f"Unknown container status for {service_name}: {status['container_status']}") container_icon = "⚫"🔎 Proposed fix (option 2: clarify comment)
+ elif status['container_status'] in ['error', 'timeout']: + container_icon = "⚫" else: - # Unknown status - log it for debugging + # Unknown or unhandled status - default to error icon container_icon = "⚫"backends/advanced/src/advanced_omi_backend/services/memory/providers/vector_stores.py (1)
204-206: Uselogging.exception()for better stack traces.The except block uses
memory_logger.error()which doesn't automatically capture the stack trace. Per project learnings, preferlogging.exception()inside except blocks to improve debuggability.Based on learnings from PR #178.
🔎 Proposed fix
except Exception as e: - memory_logger.error(f"Qdrant search failed: {e}") + memory_logger.exception("Qdrant search failed") return []backends/advanced/src/advanced_omi_backend/plugins/homeassistant/command_parser.py (1)
86-87: Consider clarifying the "all" target semantic.The example for "turn off all lights" uses
target_type: "entity"withtarget: "all". This creates a special case where "all" is used as a magic value rather than an actual entity name.Consider either:
- Documenting this in the TARGET_TYPE section (e.g.,
entity: ... or "all" for all entities of a type)- Or using a distinct target_type like
"all_of_type"for consistencyThis is optional since the current approach will work if the consuming code handles it.
wizard.py (2)
379-396: Improve exception handling and add specific exception type.The
read_plugin_config_valuefunction catches a bareExceptionwhich can hide issues. Per coding guidelines, use explicit error handling with proper exceptions.🔎 Proposed fix
def read_plugin_config_value(plugin_id, config_key): """Read a value from existing plugins.yml file""" plugins_yml_path = Path('config/plugins.yml') if not plugins_yml_path.exists(): return None try: with open(plugins_yml_path, 'r') as f: plugins_data = yaml.safe_load(f) if not plugins_data or 'plugins' not in plugins_data: return None plugin_config = plugins_data['plugins'].get(plugin_id, {}) return plugin_config.get(config_key) - except Exception: + except (yaml.YAMLError, IOError, TypeError) as e: + console.print(f"[dim]Warning: Could not read plugin config: {e}[/dim]") return None
369-377: Edge case:mask_valuemay expose more than intended for short values.When
len(value) <= show_chars * 2(i.e., ≤10 chars with defaultshow_chars=5), the function returns the value unmasked. For sensitive tokens that happen to be short, this could expose the entire value.Consider always masking if the value is meant to be sensitive:
🔎 Proposed fix
def mask_value(value, show_chars=5): """Mask a value showing only first and last few characters""" - if not value or len(value) <= show_chars * 2: - return value + if not value: + return value + + # Remove quotes if present + value_clean = value.strip("'\"") + + # For very short values, just show asterisks + if len(value_clean) <= show_chars * 2: + return "*" * len(value_clean) - # Remove quotes if present - value_clean = value.strip("'\"") - return f"{value_clean[:show_chars]}{'*' * min(15, len(value_clean) - show_chars * 2)}{value_clean[-show_chars:]}"backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py (2)
403-437: Streaming transcript plugin integration looks solid, with minor improvements possible.The plugin trigger is well-guarded with null checks and try/except to prevent plugin failures from crashing the main conversation flow. A few suggestions:
- Line 434: The f-string has no placeholders - remove the
fprefix.- Line 436-437: Per project conventions, use
logging.exception()instead oflogger.warning()to capture the full stack trace for debugging.🔎 Suggested improvements
- if not result.should_continue: - logger.info(f" Plugin stopped normal processing") + if not result.should_continue: + logger.info(" Plugin stopped normal processing") except Exception as e: - logger.warning(f"⚠️ Error triggering transcript-level plugins: {e}") + logger.exception("⚠️ Error triggering transcript-level plugins")Based on learnings, prefer
logging.exception()inside except blocks to automatically log the full stack trace.
537-572: Conversation-level plugin integration follows established pattern.The plugin trigger after post-conversation jobs is well-structured. The
end_reasonmetadata provides useful context for plugins. Same logging improvement applies here:🔎 Suggested improvement for exception logging
except Exception as e: - logger.warning(f"⚠️ Error triggering conversation-level plugins: {e}") + logger.exception("⚠️ Error triggering conversation-level plugins")Based on learnings, use
logging.exception()to capture full stack traces for plugin errors.backends/advanced/src/advanced_omi_backend/workers/memory_jobs.py (2)
248-258: Duplicateconversation_idkey in plugin_data.
conversation_idappears both nested inside theconversationdict (line 251) and at the top level (line 257). This redundancy may cause confusion for plugin consumers.🔎 Suggested fix
plugin_data = { 'memories': created_memory_ids, 'conversation': { - 'conversation_id': conversation_id, 'client_id': client_id, 'user_id': user_id, 'user_email': user_email, }, 'memory_count': len(created_memory_ids), 'conversation_id': conversation_id, }
276-277: Uselogging.exception()for better stack traces.Per project conventions, prefer
logging.exception()inside except blocks to automatically capture the full stack trace for debugging.🔎 Suggested fix
except Exception as e: - logger.warning(f"⚠️ Error triggering memory-level plugins: {e}") + logger.exception("⚠️ Error triggering memory-level plugins")Based on learnings,
logging.exception()automatically logs the full stack trace, improving debuggability.backends/advanced/src/advanced_omi_backend/workers/transcription_jobs.py (4)
171-173: Avoid defensivehasattr()check.Per coding guidelines, research and understand the class structure instead of using defensive
hasattr()checks. TheConversationmodel should haveclient_iddefined.🔎 Suggested fix
# Extract user_id and client_id for plugin context user_id = str(conversation.user_id) if conversation.user_id else None - client_id = conversation.client_id if hasattr(conversation, 'client_id') else None + client_id = conversation.client_idAs per coding guidelines, avoid defensive
hasattr()checks.
210-261: Remove DEBUG log statements before merging.Lines 212, 232, 235, 244, 251, and 261 contain verbose DEBUG statements (e.g.,
"🔍 DEBUG: About to trigger plugins...") that appear to be development artifacts. Consider removing or converting to properlogger.debug()calls without the "DEBUG:" prefix for production.Also, Line 261 has an f-string without placeholders - remove the
fprefix:🔎 Suggested fix for line 261
- logger.info(f"🔍 DEBUG: Plugin processing complete, moving to speech validation") + logger.debug("Plugin processing complete, moving to speech validation")
229-230: Remove redundant exception object fromlogging.exception().
logging.exception()automatically includes the exception info; passingein the message is redundant.🔎 Suggested fix
- logger.exception(f"Failed to initialize plugin '{plugin_id}' in worker: {e}") + logger.exception(f"Failed to initialize plugin '{plugin_id}' in worker")
258-259: Remove redundant exception object fromlogging.exception().Same issue -
logging.exception()automatically captures the exception.🔎 Suggested fix
- logger.exception(f"⚠️ Error triggering transcript plugins in batch mode: {e}") + logger.exception("⚠️ Error triggering transcript plugins in batch mode")backends/advanced/src/advanced_omi_backend/app_factory.py (1)
195-201: Uselogging.exception()for shutdown errors.Per project conventions and static analysis, prefer
logging.exception()to automatically capture the stack trace.🔎 Suggested fix
# Shutdown plugins try: from advanced_omi_backend.services.plugin_service import cleanup_plugin_router await cleanup_plugin_router() application_logger.info("Plugins shut down") except Exception as e: - application_logger.error(f"Error shutting down plugins: {e}") + application_logger.exception("Error shutting down plugins")Based on learnings,
logging.exception()automatically logs the full stack trace, improving debuggability.backends/advanced/src/advanced_omi_backend/plugins/homeassistant/entity_cache.py (1)
28-29: Consider using timezone-aware datetime.
datetime.now()returns a naive datetime. Iflast_refreshis ever compared with timezone-aware datetimes elsewhere, this could cause issues. Consider usingdatetime.now(UTC)for consistency (already imported asUTCin other files in this PR).🔎 Suggested fix
from dataclasses import dataclass, field -from datetime import datetime +from datetime import datetime, UTC from typing import Dict, List, Optional import loggingAnd update line 28:
- last_refresh: datetime = field(default_factory=datetime.now) + last_refresh: datetime = field(default_factory=lambda: datetime.now(UTC))And line 121:
- return (datetime.now() - self.last_refresh).total_seconds() + return (datetime.now(UTC) - self.last_refresh).total_seconds()backends/advanced/webui/src/components/ChatSettings.tsx (2)
28-31: Response data handling may be incorrect for plain text response.The backend endpoint
GET /api/admin/chat/configreturns plain text (Response(content=yaml_content, media_type="text/plain")), not JSON. However, this code attempts to accessresponse.data.config_yamlwhich would be undefined for a plain text response. The fallback|| response.datashould work, but the primary accessor is misleading.Consider simplifying to just use
response.datadirectly since the backend returns plain text:🔎 Suggested fix
try { const response = await systemApi.getChatConfigRaw() - setConfigYaml(response.data.config_yaml || response.data) + setConfigYaml(response.data) setMessage('Configuration loaded successfully')
91-95: Message gets overwritten by async callback.
resetConfigsets a message immediately after callingloadChatConfig(), butloadChatConfigis async and will set its own success message when it completes, overwriting the "Configuration reset to file version" message. The user will only ever see "Configuration loaded successfully".Either remove the redundant message in
resetConfigor updateloadChatConfigto optionally suppress its success message:🔎 Suggested fix
const resetConfig = () => { loadChatConfig() - setMessage('Configuration reset to file version') - setTimeout(() => setMessage(''), 3000) }backends/advanced/webui/src/components/PluginSettings.tsx (2)
28-31: Same response data handling issue as ChatSettings.The backend returns plain text, so
response.data.config_yamlwill be undefined. The fallback works but consider simplifying:🔎 Suggested fix
try { const response = await systemApi.getPluginsConfigRaw() - setConfigYaml(response.data.config_yaml || response.data) + setConfigYaml(response.data) setMessage('Configuration loaded successfully')
1-195: Consider extracting shared config editor component.
PluginSettingsandChatSettingsshare nearly identical structure, state management, and UI patterns. Consider extracting a reusableConfigEditorcomponent that accepts props for the title, icon, API methods, and help content. This would reduce duplication and simplify future maintenance.backends/advanced/src/advanced_omi_backend/services/plugin_service.py (1)
101-108: Uselogging.exception()for full stack trace.Based on project learnings, use
logging.exception()instead oflogging.error()in except blocks to automatically capture the full stack trace:🔎 Suggested fix
if _plugin_router: try: await _plugin_router.cleanup_all() logger.info("Plugin router cleanup complete") except Exception as e: - logger.error(f"Error during plugin router cleanup: {e}") + logger.exception("Error during plugin router cleanup") finally: _plugin_router = Nonebackends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py (1)
140-142: Uselogging.exception()and chain exceptions properly.Per project learnings, prefer
logging.exception()to capture full stack traces and chain exceptions withfrom e:🔎 Suggested fix
except Exception as e: - logger.error(f"Failed to get chat config: {e}") - raise HTTPException(status_code=500, detail=str(e)) + logger.exception("Failed to get chat config") + raise HTTPException(status_code=500, detail=str(e)) from eThis pattern should be applied to all similar exception handlers in lines 158-160, 174-176, 187-189, 205-207, and 221-223.
backends/advanced/src/advanced_omi_backend/controllers/system_controller.py (2)
486-488: Uselogging.exception()instead oflogging.error()in except blocks.Per project learnings, use
logging.exception()to automatically capture full stack traces:🔎 Suggested fix for affected locations
# Line 487 - logger.error(f"Error loading chat config: {e}") + logger.exception("Error loading chat config") # Line 537 - logger.error(f"Error saving chat config: {e}") + logger.exception("Error saving chat config") # Line 591 - logger.error(f"Error loading plugins config: {e}") + logger.exception("Error loading plugins config") # Line 643 - logger.error(f"Error saving plugins config: {e}") + logger.exception("Error saving plugins config") # Line 693 - logger.error(f"Error validating plugins config: {e}") + logger.exception("Error validating plugins config")Based on learnings from previous reviews in this project.
Also applies to: 536-538, 590-592, 642-644, 692-694
610-611: Chain exception to preserve original context.Per project coding conventions, use
raise ... from eto preserve the original exception context for better debugging:🔎 Suggested fix
except yaml.YAMLError as e: - raise ValueError(f"Invalid YAML syntax: {e}") + raise ValueError(f"Invalid YAML syntax: {e}") from ebackends/advanced/src/advanced_omi_backend/plugins/base.py (1)
48-49: Annotate mutable class attribute withClassVar.
SUPPORTED_ACCESS_LEVELSis a mutable class attribute that should be annotated withClassVarto indicate it's shared across instances and shouldn't be modified per-instance.🔎 Proposed fix
+from typing import Optional, Dict, Any, List, ClassVar -from typing import Optional, Dict, Any, List # ... in BasePlugin class: - SUPPORTED_ACCESS_LEVELS: List[str] = [] + SUPPORTED_ACCESS_LEVELS: ClassVar[List[str]] = []backends/advanced/src/advanced_omi_backend/plugins/router.py (1)
163-170: Uselogging.exception()for automatic stack trace.Per project conventions, use
logging.exception()instead oflogging.error()in except blocks to automatically log the full stack trace.🔎 Proposed fix
async def cleanup_all(self): """Clean up all registered plugins""" for plugin_id, plugin in self.plugins.items(): try: await plugin.cleanup() logger.info(f"Cleaned up plugin '{plugin_id}'") except Exception as e: - logger.error(f"Error cleaning up plugin '{plugin_id}': {e}") + logger.exception(f"Error cleaning up plugin '{plugin_id}': {e}")Based on learnings, prefer
logging.exception()inside except blocks to automatically log the full stack trace.backends/advanced/src/advanced_omi_backend/plugins/homeassistant/mcp_client.py (2)
102-110: Chain exceptions and uselogging.exception()for better debuggability.Per project conventions: (1) use
logging.exception()instead oflogger.error()in except blocks to automatically capture stack traces, and (2) chain re-raised exceptions withraise ... from eto preserve original context.This pattern appears in multiple places throughout this file (lines 102-110, 232-237, 335-340, 392-397).
🔎 Proposed fix for lines 102-110
except httpx.HTTPStatusError as e: - logger.error(f"HTTP error calling MCP endpoint: {e.response.status_code}") - raise MCPError(f"HTTP {e.response.status_code}: {e.response.text}") + logger.exception(f"HTTP error calling MCP endpoint: {e.response.status_code}") + raise MCPError(f"HTTP {e.response.status_code}: {e.response.text}") from e except httpx.RequestError as e: - logger.error(f"Request error calling MCP endpoint: {e}") - raise MCPError(f"Request failed: {e}") + logger.exception(f"Request error calling MCP endpoint: {e}") + raise MCPError(f"Request failed: {e}") from e except Exception as e: - logger.error(f"Unexpected error calling MCP endpoint: {e}") - raise MCPError(f"Unexpected error: {e}") + logger.exception(f"Unexpected error calling MCP endpoint: {e}") + raise MCPError(f"Unexpected error: {e}") from eBased on learnings, prefer
logging.exception()inside except blocks and chain withraise ... from eto preserve original context.
284-341: N+1 query performance concern infetch_entity_states.The method fetches all entity states, then makes a separate template API call for each entity to get its
area_id. For a Home Assistant instance with hundreds of entities, this results in hundreds of sequential API calls.Consider batching the area lookups or fetching all entity-to-area mappings in a single template call if possible.
backends/advanced/src/advanced_omi_backend/plugins/homeassistant/plugin.py (2)
31-31: Annotate mutable class attribute withClassVar.Same as in
base.py, theSUPPORTED_ACCESS_LEVELSclass attribute should useClassVar.🔎 Proposed fix
+from typing import Any, Dict, List, Optional, ClassVar - SUPPORTED_ACCESS_LEVELS: List[str] = ['transcript'] + SUPPORTED_ACCESS_LEVELS: ClassVar[List[str]] = ['transcript']
93-94: Chain exception withraise ... from e.Per project conventions, chain re-raised exceptions to preserve the original context.
🔎 Proposed fix
except Exception as e: - raise MCPError(f"Failed to connect to Home Assistant API: {e}") + raise MCPError(f"Failed to connect to Home Assistant API: {e}") from eBased on learnings, always chain with
raise ... from eto preserve the original context.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
backends/advanced/.env.templatebackends/advanced/docker-compose-test.ymlbackends/advanced/docker-compose.ymlbackends/advanced/init.pybackends/advanced/src/advanced_omi_backend/app_factory.pybackends/advanced/src/advanced_omi_backend/chat_service.pybackends/advanced/src/advanced_omi_backend/controllers/system_controller.pybackends/advanced/src/advanced_omi_backend/model_registry.pybackends/advanced/src/advanced_omi_backend/plugins/__init__.pybackends/advanced/src/advanced_omi_backend/plugins/base.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/__init__.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/command_parser.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/entity_cache.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/mcp_client.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/plugin.pybackends/advanced/src/advanced_omi_backend/plugins/router.pybackends/advanced/src/advanced_omi_backend/routers/modules/system_routes.pybackends/advanced/src/advanced_omi_backend/services/memory/providers/vector_stores.pybackends/advanced/src/advanced_omi_backend/services/plugin_service.pybackends/advanced/src/advanced_omi_backend/workers/conversation_jobs.pybackends/advanced/src/advanced_omi_backend/workers/memory_jobs.pybackends/advanced/src/advanced_omi_backend/workers/transcription_jobs.pybackends/advanced/start.shbackends/advanced/webui/src/App.tsxbackends/advanced/webui/src/components/ChatSettings.tsxbackends/advanced/webui/src/components/PluginSettings.tsxbackends/advanced/webui/src/components/layout/Layout.tsxbackends/advanced/webui/src/pages/Chat.tsxbackends/advanced/webui/src/pages/Plugins.tsxbackends/advanced/webui/src/pages/System.tsxbackends/advanced/webui/src/services/api.tsconfig/plugins.ymlservices.pystatus.pytests/endpoints/system_admin_tests.robotwizard.py
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
TypeScript code should follow standard React Native conventions.
Files:
backends/advanced/webui/src/components/PluginSettings.tsxbackends/advanced/webui/src/App.tsxbackends/advanced/webui/src/pages/Plugins.tsxbackends/advanced/webui/src/pages/Chat.tsxbackends/advanced/webui/src/services/api.tsbackends/advanced/webui/src/components/layout/Layout.tsxbackends/advanced/webui/src/pages/System.tsxbackends/advanced/webui/src/components/ChatSettings.tsx
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: In Python backend, useuv run python3for running Python commands instead ofpython3directly, as the project usesuvpackage manager.
Python code formatting: Use Black formatter with 100-character line length and isort for import organization.
Python import guidelines: ALL imports must be at the top of the file after the docstring. NEVER import modules in the middle of functions or files. Group imports into standard library, third-party, and local imports. Use lazy imports sparingly and only when absolutely necessary for circular import issues.
Python error handling: Always raise errors, never silently ignore. Use explicit error handling with proper exceptions rather than silent failures. Do not add defensivehasattr()checks—research and understand input/response structures or class structure instead.
Files:
backends/advanced/src/advanced_omi_backend/plugins/__init__.pyservices.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/command_parser.pybackends/advanced/src/advanced_omi_backend/services/plugin_service.pybackends/advanced/src/advanced_omi_backend/app_factory.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/entity_cache.pybackends/advanced/src/advanced_omi_backend/workers/transcription_jobs.pybackends/advanced/src/advanced_omi_backend/services/memory/providers/vector_stores.pybackends/advanced/src/advanced_omi_backend/chat_service.pybackends/advanced/src/advanced_omi_backend/workers/conversation_jobs.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/__init__.pybackends/advanced/src/advanced_omi_backend/model_registry.pybackends/advanced/src/advanced_omi_backend/plugins/base.pybackends/advanced/src/advanced_omi_backend/plugins/router.pystatus.pywizard.pybackends/advanced/src/advanced_omi_backend/workers/memory_jobs.pybackends/advanced/src/advanced_omi_backend/routers/modules/system_routes.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/plugin.pybackends/advanced/init.pybackends/advanced/src/advanced_omi_backend/controllers/system_controller.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/mcp_client.py
**/docker-compose*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
**/docker-compose*.yml: Docker build cache management: Usedocker compose buildwithout--no-cacheby default for faster builds. Only use--no-cachewhen explicitly needed (e.g., package updates, dependency issues, or troubleshooting build problems).
When src/ directory is volume mounted in Docker, code changes are reflected automatically. If not volume mounted, usedocker compose buildto rebuild the image so code changes are reflected. Do not simply rundocker compose restartas it will not rebuild the image.
Files:
backends/advanced/docker-compose-test.ymlbackends/advanced/docker-compose.yml
**/*.robot
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.robot: When writing or modifying Robot Framework tests, read @tests/TESTING_GUIDELINES.md for comprehensive testing patterns and standards before implementation.
Robot Framework tests: Check @tests/tags.md for approved tags. ONLY use the 11 approved tags (tab-separated). Do not create custom tags.
Robot Framework tests: Before writing ANY test code, scan relevant resource files for existing keywords. NEVER write code that duplicates existing keywords.
Robot Framework tests: Write assertions directly in tests using inline verifications, not abstracted to keywords. Only create keywords for reusable setup/action operations.
Robot Framework tests: Use descriptive keyword and test names that explain business purpose, not technical implementation.
Files:
tests/endpoints/system_admin_tests.robot
🧠 Learnings (2)
📚 Learning: 2025-12-08T23:52:34.959Z
Learnt from: AnkushMalaker
Repo: chronicler-ai/chronicle PR: 178
File: backends/advanced/src/advanced_omi_backend/services/memory/providers/mycelia.py:218-223
Timestamp: 2025-12-08T23:52:34.959Z
Learning: In Python code (chronicle project), prefer logging.exception() inside except blocks to automatically log the full stack trace. When re-raising exceptions, always chain with 'raise ... from e' to preserve the original context; use 'raise ... from None' only if you explicitly want to suppress the context. This improves debuggability across Python files.
Applied to files:
backends/advanced/src/advanced_omi_backend/plugins/__init__.pyservices.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/command_parser.pybackends/advanced/src/advanced_omi_backend/services/plugin_service.pybackends/advanced/src/advanced_omi_backend/app_factory.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/entity_cache.pybackends/advanced/src/advanced_omi_backend/workers/transcription_jobs.pybackends/advanced/src/advanced_omi_backend/services/memory/providers/vector_stores.pybackends/advanced/src/advanced_omi_backend/chat_service.pybackends/advanced/src/advanced_omi_backend/workers/conversation_jobs.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/__init__.pybackends/advanced/src/advanced_omi_backend/model_registry.pybackends/advanced/src/advanced_omi_backend/plugins/base.pybackends/advanced/src/advanced_omi_backend/plugins/router.pystatus.pywizard.pybackends/advanced/src/advanced_omi_backend/workers/memory_jobs.pybackends/advanced/src/advanced_omi_backend/routers/modules/system_routes.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/plugin.pybackends/advanced/init.pybackends/advanced/src/advanced_omi_backend/controllers/system_controller.pybackends/advanced/src/advanced_omi_backend/plugins/homeassistant/mcp_client.py
📚 Learning: 2026-01-02T23:28:37.958Z
Learnt from: CR
Repo: chronicler-ai/chronicle PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:28:37.958Z
Learning: Applies to **/docker-compose*.yml : When src/ directory is volume mounted in Docker, code changes are reflected automatically. If not volume mounted, use `docker compose build` to rebuild the image so code changes are reflected. Do not simply run `docker compose restart` as it will not rebuild the image.
Applied to files:
backends/advanced/docker-compose-test.yml
🧬 Code graph analysis (16)
backends/advanced/webui/src/App.tsx (2)
backends/advanced/webui/src/components/ErrorBoundary.tsx (1)
PageErrorBoundary(116-138)backends/advanced/webui/src/pages/Plugins.tsx (1)
Plugins(3-9)
backends/advanced/webui/src/pages/Plugins.tsx (1)
backends/advanced/webui/src/components/PluginSettings.tsx (1)
PluginSettings(10-195)
backends/advanced/src/advanced_omi_backend/plugins/__init__.py (2)
backends/advanced/src/advanced_omi_backend/plugins/base.py (3)
BasePlugin(32-131)PluginContext(15-20)PluginResult(24-29)backends/advanced/src/advanced_omi_backend/plugins/router.py (1)
PluginRouter(15-170)
backends/advanced/src/advanced_omi_backend/app_factory.py (1)
backends/advanced/src/advanced_omi_backend/services/plugin_service.py (3)
init_plugin_router(43-94)set_plugin_router(30-40)cleanup_plugin_router(97-108)
backends/advanced/src/advanced_omi_backend/workers/transcription_jobs.py (4)
backends/advanced/src/advanced_omi_backend/services/plugin_service.py (2)
get_plugin_router(20-27)init_plugin_router(43-94)backends/advanced/src/advanced_omi_backend/models/user.py (1)
user_id(71-73)backends/advanced/src/advanced_omi_backend/models/conversation.py (1)
segments(197-199)backends/advanced/src/advanced_omi_backend/plugins/router.py (1)
trigger_plugins(39-106)
backends/advanced/src/advanced_omi_backend/chat_service.py (1)
backends/advanced/src/advanced_omi_backend/model_registry.py (1)
get_models_registry(350-365)
backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py (3)
backends/advanced/src/advanced_omi_backend/services/plugin_service.py (1)
get_plugin_router(20-27)backends/advanced/src/advanced_omi_backend/plugins/router.py (1)
trigger_plugins(39-106)backends/advanced/src/advanced_omi_backend/models/conversation.py (2)
Conversation(17-314)transcript(191-193)
backends/advanced/src/advanced_omi_backend/plugins/homeassistant/__init__.py (1)
backends/advanced/src/advanced_omi_backend/plugins/homeassistant/plugin.py (1)
HomeAssistantPlugin(19-598)
backends/advanced/webui/src/pages/System.tsx (1)
backends/advanced/webui/src/components/ChatSettings.tsx (1)
ChatSettings(10-195)
backends/advanced/src/advanced_omi_backend/plugins/base.py (2)
backends/advanced/src/advanced_omi_backend/models/user.py (1)
user_id(71-73)backends/advanced/src/advanced_omi_backend/plugins/homeassistant/plugin.py (2)
initialize(61-96)on_transcript(98-250)
backends/advanced/src/advanced_omi_backend/plugins/router.py (1)
backends/advanced/src/advanced_omi_backend/plugins/base.py (7)
BasePlugin(32-131)PluginContext(15-20)PluginResult(24-29)on_transcript(85-101)on_conversation_complete(103-116)on_memory_processed(118-131)cleanup(74-81)
backends/advanced/src/advanced_omi_backend/workers/memory_jobs.py (2)
backends/advanced/src/advanced_omi_backend/services/plugin_service.py (1)
get_plugin_router(20-27)backends/advanced/src/advanced_omi_backend/plugins/router.py (1)
trigger_plugins(39-106)
backends/advanced/src/advanced_omi_backend/plugins/homeassistant/plugin.py (4)
backends/advanced/src/advanced_omi_backend/plugins/base.py (2)
PluginContext(15-20)PluginResult(24-29)backends/advanced/src/advanced_omi_backend/plugins/homeassistant/entity_cache.py (3)
EntityCache(16-133)get_entities_in_area(73-117)find_entity_by_name(31-71)backends/advanced/src/advanced_omi_backend/plugins/homeassistant/mcp_client.py (2)
MCPError(17-19)_render_template(187-237)backends/advanced/src/advanced_omi_backend/plugins/homeassistant/command_parser.py (1)
ParsedCommand(13-29)
backends/advanced/init.py (2)
wizard.py (2)
prompt_with_existing_masked(397-436)prompt_value(349-360)extras/speaker-recognition/init.py (3)
read_existing_env_value(68-77)mask_api_key(79-87)prompt_value(48-54)
backends/advanced/src/advanced_omi_backend/controllers/system_controller.py (2)
backends/advanced/src/advanced_omi_backend/model_registry.py (2)
_find_config_path(252-286)load_models_config(289-347)backends/advanced/src/advanced_omi_backend/services/plugin_service.py (1)
get_plugin_router(20-27)
backends/advanced/webui/src/components/ChatSettings.tsx (2)
backends/advanced/webui/src/contexts/AuthContext.tsx (1)
useAuth(115-121)backends/advanced/webui/src/services/api.ts (1)
systemApi(150-197)
🪛 dotenv-linter (4.0.0)
backends/advanced/.env.template
[warning] 219-219: [UnorderedKey] The LANGFUSE_ENABLE_TELEMETRY key should go before the LANGFUSE_HOST key
(UnorderedKey)
[warning] 234-234: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🪛 Ruff (0.14.10)
backends/advanced/src/advanced_omi_backend/services/plugin_service.py
89-89: Consider moving this statement to an else block
(TRY300)
105-105: Do not catch blind exception: Exception
(BLE001)
106-106: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backends/advanced/src/advanced_omi_backend/app_factory.py
200-200: Do not catch blind exception: Exception
(BLE001)
201-201: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backends/advanced/src/advanced_omi_backend/workers/transcription_jobs.py
230-230: Redundant exception object included in logging.exception call
(TRY401)
259-259: Redundant exception object included in logging.exception call
(TRY401)
261-261: f-string without any placeholders
Remove extraneous f prefix
(F541)
backends/advanced/src/advanced_omi_backend/chat_service.py
162-162: Do not catch blind exception: Exception
(BLE001)
backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py
434-434: f-string without any placeholders
Remove extraneous f prefix
(F541)
436-436: Do not catch blind exception: Exception
(BLE001)
571-571: Do not catch blind exception: Exception
(BLE001)
backends/advanced/src/advanced_omi_backend/plugins/base.py
49-49: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
74-81: BasePlugin.cleanup is an empty method in an abstract base class, but has no abstract decorator
(B027)
85-101: BasePlugin.on_transcript is an empty method in an abstract base class, but has no abstract decorator
(B027)
103-116: BasePlugin.on_conversation_complete is an empty method in an abstract base class, but has no abstract decorator
(B027)
118-131: BasePlugin.on_memory_processed is an empty method in an abstract base class, but has no abstract decorator
(B027)
backends/advanced/src/advanced_omi_backend/plugins/router.py
169-169: Do not catch blind exception: Exception
(BLE001)
170-170: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
wizard.py
358-358: Consider moving this statement to an else block
(TRY300)
394-394: Do not catch blind exception: Exception
(BLE001)
backends/advanced/src/advanced_omi_backend/workers/memory_jobs.py
276-276: Do not catch blind exception: Exception
(BLE001)
backends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py
135-135: Unused function argument: current_user
(ARG001)
135-135: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
140-140: Do not catch blind exception: Exception
(BLE001)
141-141: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
142-142: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
148-148: Unused function argument: current_user
(ARG001)
148-148: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
157-157: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
158-158: Do not catch blind exception: Exception
(BLE001)
159-159: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
160-160: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
166-166: Unused function argument: current_user
(ARG001)
166-166: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
174-174: Do not catch blind exception: Exception
(BLE001)
175-175: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
176-176: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
182-182: Unused function argument: current_user
(ARG001)
182-182: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
187-187: Do not catch blind exception: Exception
(BLE001)
188-188: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
189-189: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
195-195: Unused function argument: current_user
(ARG001)
195-195: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
204-204: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
205-205: Do not catch blind exception: Exception
(BLE001)
206-206: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
207-207: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
213-213: Unused function argument: current_user
(ARG001)
213-213: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
221-221: Do not catch blind exception: Exception
(BLE001)
222-222: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
223-223: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
backends/advanced/src/advanced_omi_backend/plugins/homeassistant/plugin.py
31-31: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
75-75: Avoid specifying long messages outside the exception class
(TRY003)
91-91: Abstract raise to an inner function
(TRY301)
91-91: Avoid specifying long messages outside the exception class
(TRY003)
93-93: Do not catch blind exception: Exception
(BLE001)
94-94: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
94-94: Avoid specifying long messages outside the exception class
(TRY003)
314-314: Undefined name ParsedCommand
(F821)
385-385: Consider moving this statement to an else block
(TRY300)
388-388: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
394-394: Undefined name ParsedCommand
(F821)
422-422: Avoid specifying long messages outside the exception class
(TRY003)
433-436: Avoid specifying long messages outside the exception class
(TRY003)
452-455: Avoid specifying long messages outside the exception class
(TRY003)
465-468: Avoid specifying long messages outside the exception class
(TRY003)
474-474: Avoid specifying long messages outside the exception class
(TRY003)
531-531: Undefined name ParsedCommand
(F821)
568-568: Do not catch blind exception: Exception
(BLE001)
594-594: Consider moving this statement to an else block
(TRY300)
backends/advanced/init.py
386-386: f-string without any placeholders
Remove extraneous f prefix
(F541)
backends/advanced/src/advanced_omi_backend/controllers/system_controller.py
484-484: Consider moving this statement to an else block
(TRY300)
487-487: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
498-498: Abstract raise to an inner function
(TRY301)
498-498: Avoid specifying long messages outside the exception class
(TRY003)
502-502: Abstract raise to an inner function
(TRY301)
502-502: Avoid specifying long messages outside the exception class
(TRY003)
504-504: Abstract raise to an inner function
(TRY301)
504-504: Avoid specifying long messages outside the exception class
(TRY003)
534-534: Consider moving this statement to an else block
(TRY300)
537-537: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
554-554: Consider moving this statement to an else block
(TRY300)
556-556: Do not catch blind exception: Exception
(BLE001)
557-557: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
558-558: Use explicit conversion flag
Replace with conversion flag
(RUF010)
588-588: Consider moving this statement to an else block
(TRY300)
591-591: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
604-604: Prefer TypeError exception for invalid type
(TRY004)
604-604: Avoid specifying long messages outside the exception class
(TRY003)
608-608: Avoid specifying long messages outside the exception class
(TRY003)
611-611: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
611-611: Avoid specifying long messages outside the exception class
(TRY003)
632-632: Do not catch blind exception: Exception
(BLE001)
637-640: Consider moving this statement to an else block
(TRY300)
643-643: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
690-690: Consider moving this statement to an else block
(TRY300)
692-692: Do not catch blind exception: Exception
(BLE001)
693-693: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
694-694: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backends/advanced/src/advanced_omi_backend/plugins/homeassistant/mcp_client.py
98-98: Abstract raise to an inner function
(TRY301)
98-98: Avoid specifying long messages outside the exception class
(TRY003)
103-103: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
104-104: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
104-104: Avoid specifying long messages outside the exception class
(TRY003)
106-106: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
107-107: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
107-107: Avoid specifying long messages outside the exception class
(TRY003)
108-108: Do not catch blind exception: Exception
(BLE001)
109-109: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
110-110: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
110-110: Avoid specifying long messages outside the exception class
(TRY003)
182-182: Consider moving this statement to an else block
(TRY300)
183-183: Do not catch blind exception: Exception
(BLE001)
184-184: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
230-230: Consider moving this statement to an else block
(TRY300)
233-233: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
234-234: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
234-234: Avoid specifying long messages outside the exception class
(TRY003)
236-236: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
237-237: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
237-237: Avoid specifying long messages outside the exception class
(TRY003)
326-326: Do not catch blind exception: Exception
(BLE001)
333-333: Consider moving this statement to an else block
(TRY300)
336-336: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
337-337: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
337-337: Avoid specifying long messages outside the exception class
(TRY003)
339-339: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
340-340: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
340-340: Avoid specifying long messages outside the exception class
(TRY003)
390-390: Consider moving this statement to an else block
(TRY300)
393-393: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
394-394: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
394-394: Avoid specifying long messages outside the exception class
(TRY003)
396-396: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
397-397: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
397-397: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: robot-tests
| tailscale: | ||
| image: tailscale/tailscale:latest | ||
| container_name: advanced-tailscale | ||
| hostname: chronicle-tailscale | ||
| environment: | ||
| - TS_AUTHKEY=${TS_AUTHKEY} | ||
| - TS_STATE_DIR=/var/lib/tailscale | ||
| - TS_USERSPACE=false | ||
| - TS_ACCEPT_DNS=true | ||
| volumes: | ||
| - tailscale-state:/var/lib/tailscale | ||
| devices: | ||
| - /dev/net/tun:/dev/net/tun | ||
| cap_add: | ||
| - NET_ADMIN | ||
| restart: unless-stopped | ||
| profiles: | ||
| - tailscale # Optional profile | ||
| ports: | ||
| - "18123:18123" # HA proxy port | ||
| command: > | ||
| sh -c "tailscaled & | ||
| tailscale up --authkey=$${TS_AUTHKEY} --accept-dns=true && | ||
| apk add --no-cache socat 2>/dev/null || true && | ||
| socat TCP-LISTEN:18123,fork,reuseaddr TCP:100.99.62.5:8123" |
There was a problem hiding this comment.
Hardcoded Tailscale IP address will not work for other users.
Line 26 contains a hardcoded IP address 100.99.62.5:8123 which appears to be a specific Tailscale node. This will break for any other deployment. Consider making this configurable via an environment variable:
🔎 Suggested fix
command: >
sh -c "tailscaled &
tailscale up --authkey=$${TS_AUTHKEY} --accept-dns=true &&
apk add --no-cache socat 2>/dev/null || true &&
- socat TCP-LISTEN:18123,fork,reuseaddr TCP:100.99.62.5:8123"
+ socat TCP-LISTEN:18123,fork,reuseaddr TCP:${HA_TAILSCALE_HOST:-100.99.62.5}:${HA_PORT:-8123}"Also, using tailscale/tailscale:latest may cause unexpected behavior after image updates. Consider pinning to a specific version. Per coding guidelines for docker-compose files, this is a maintainability concern.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tailscale: | |
| image: tailscale/tailscale:latest | |
| container_name: advanced-tailscale | |
| hostname: chronicle-tailscale | |
| environment: | |
| - TS_AUTHKEY=${TS_AUTHKEY} | |
| - TS_STATE_DIR=/var/lib/tailscale | |
| - TS_USERSPACE=false | |
| - TS_ACCEPT_DNS=true | |
| volumes: | |
| - tailscale-state:/var/lib/tailscale | |
| devices: | |
| - /dev/net/tun:/dev/net/tun | |
| cap_add: | |
| - NET_ADMIN | |
| restart: unless-stopped | |
| profiles: | |
| - tailscale # Optional profile | |
| ports: | |
| - "18123:18123" # HA proxy port | |
| command: > | |
| sh -c "tailscaled & | |
| tailscale up --authkey=$${TS_AUTHKEY} --accept-dns=true && | |
| apk add --no-cache socat 2>/dev/null || true && | |
| socat TCP-LISTEN:18123,fork,reuseaddr TCP:100.99.62.5:8123" | |
| tailscale: | |
| image: tailscale/tailscale:latest | |
| container_name: advanced-tailscale | |
| hostname: chronicle-tailscale | |
| environment: | |
| - TS_AUTHKEY=${TS_AUTHKEY} | |
| - TS_STATE_DIR=/var/lib/tailscale | |
| - TS_USERSPACE=false | |
| - TS_ACCEPT_DNS=true | |
| volumes: | |
| - tailscale-state:/var/lib/tailscale | |
| devices: | |
| - /dev/net/tun:/dev/net/tun | |
| cap_add: | |
| - NET_ADMIN | |
| restart: unless-stopped | |
| profiles: | |
| - tailscale # Optional profile | |
| ports: | |
| - "18123:18123" # HA proxy port | |
| command: > | |
| sh -c "tailscaled & | |
| tailscale up --authkey=$${TS_AUTHKEY} --accept-dns=true && | |
| apk add --no-cache socat 2>/dev/null || true && | |
| socat TCP-LISTEN:18123,fork,reuseaddr TCP:${HA_TAILSCALE_HOST:-100.99.62.5}:${HA_PORT:-8123}" |
🤖 Prompt for AI Agents
In backends/advanced/docker-compose.yml around lines 2 to 26 the tailscale
service hardcodes a Tailscale node address (100.99.62.5:8123) and uses the image
tag :latest; change the command to read the target host:port from an environment
variable (e.g., TAILSCALE_TARGET_HOST and TAILSCALE_TARGET_PORT or a single
TAILSCALE_TARGET) instead of the hardcoded IP, update the environment section to
include that variable with a sensible default or document it in .env, and pin
the image to a specific version (replace tailscale/tailscale:latest with a
concrete semver tag) to avoid unexpected upgrades.
backends/advanced/init.py
Outdated
| # Check if Tailscale auth key provided via args | ||
| if hasattr(self.args, 'ts_authkey') and self.args.ts_authkey: | ||
| self.config["TS_AUTHKEY"] = self.args.ts_authkey | ||
| self.console.print(f"[green][SUCCESS][/green] Tailscale auth key configured (Docker integration enabled)") | ||
| self.console.print("[blue][INFO][/blue] Start Tailscale with: docker compose --profile tailscale up -d") |
There was a problem hiding this comment.
Remove unnecessary f-string prefix.
Line 386 uses an f-string without any placeholders.
🔎 Proposed fix
if hasattr(self.args, 'ts_authkey') and self.args.ts_authkey:
self.config["TS_AUTHKEY"] = self.args.ts_authkey
- self.console.print(f"[green][SUCCESS][/green] Tailscale auth key configured (Docker integration enabled)")
+ self.console.print("[green][SUCCESS][/green] Tailscale auth key configured (Docker integration enabled)")
self.console.print("[blue][INFO][/blue] Start Tailscale with: docker compose --profile tailscale up -d")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Check if Tailscale auth key provided via args | |
| if hasattr(self.args, 'ts_authkey') and self.args.ts_authkey: | |
| self.config["TS_AUTHKEY"] = self.args.ts_authkey | |
| self.console.print(f"[green][SUCCESS][/green] Tailscale auth key configured (Docker integration enabled)") | |
| self.console.print("[blue][INFO][/blue] Start Tailscale with: docker compose --profile tailscale up -d") | |
| # Check if Tailscale auth key provided via args | |
| if hasattr(self.args, 'ts_authkey') and self.args.ts_authkey: | |
| self.config["TS_AUTHKEY"] = self.args.ts_authkey | |
| self.console.print("[green][SUCCESS][/green] Tailscale auth key configured (Docker integration enabled)") | |
| self.console.print("[blue][INFO][/blue] Start Tailscale with: docker compose --profile tailscale up -d") |
🧰 Tools
🪛 Ruff (0.14.10)
386-386: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In backends/advanced/init.py around lines 383 to 387, the console.print call on
line 386 uses an unnecessary f-string prefix for a static message; change the
f-string to a regular string literal (remove the leading f) so the printed
message remains identical but without the redundant f-string syntax.
| def _get_system_prompt(self) -> str: | ||
| """ | ||
| Get system prompt from config with fallback to default. | ||
| Returns: | ||
| str: System prompt for chat interactions | ||
| """ | ||
| try: | ||
| reg = get_models_registry() | ||
| if reg and hasattr(reg, 'chat'): | ||
| chat_config = reg.chat | ||
| prompt = chat_config.get('system_prompt') | ||
| if prompt: | ||
| logger.info(f"✅ Loaded chat system prompt from config (length: {len(prompt)} chars)") | ||
| logger.debug(f"System prompt: {prompt[:100]}...") | ||
| return prompt | ||
| except Exception as e: | ||
| logger.warning(f"Failed to load chat system prompt from config: {e}") | ||
|
|
||
| # Fallback to default | ||
| logger.info("⚠️ Using default chat system prompt (config not found)") | ||
| return """You are a helpful AI assistant with access to the user's personal memories and conversation history. | ||
| Use the provided memories and conversation context to give personalized, contextual responses. If memories are relevant, reference them naturally in your response. Be conversational and helpful. | ||
| If no relevant memories are available, respond normally based on the conversation context.""" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unnecessary hasattr check and improve exception handling.
Two issues:
-
Per coding guidelines: The
hasattr(reg, 'chat')check is unnecessary sincechatis a defined field onAppModelswithdefault_factory=dict. The field will always exist. -
Per learnings: Use
logging.exception()inside except blocks to capture the full stack trace for better debuggability.
🔎 Proposed fix
def _get_system_prompt(self) -> str:
"""
Get system prompt from config with fallback to default.
Returns:
str: System prompt for chat interactions
"""
try:
reg = get_models_registry()
- if reg and hasattr(reg, 'chat'):
- chat_config = reg.chat
+ if reg:
+ chat_config = reg.chat # Always exists with default_factory=dict
prompt = chat_config.get('system_prompt')
if prompt:
logger.info(f"✅ Loaded chat system prompt from config (length: {len(prompt)} chars)")
logger.debug(f"System prompt: {prompt[:100]}...")
return prompt
except Exception as e:
- logger.warning(f"Failed to load chat system prompt from config: {e}")
+ logger.exception("Failed to load chat system prompt from config")
# Fallback to default
logger.info("⚠️ Using default chat system prompt (config not found)")Based on coding guidelines (no defensive hasattr() checks) and learnings (use logging.exception() in except blocks).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _get_system_prompt(self) -> str: | |
| """ | |
| Get system prompt from config with fallback to default. | |
| Returns: | |
| str: System prompt for chat interactions | |
| """ | |
| try: | |
| reg = get_models_registry() | |
| if reg and hasattr(reg, 'chat'): | |
| chat_config = reg.chat | |
| prompt = chat_config.get('system_prompt') | |
| if prompt: | |
| logger.info(f"✅ Loaded chat system prompt from config (length: {len(prompt)} chars)") | |
| logger.debug(f"System prompt: {prompt[:100]}...") | |
| return prompt | |
| except Exception as e: | |
| logger.warning(f"Failed to load chat system prompt from config: {e}") | |
| # Fallback to default | |
| logger.info("⚠️ Using default chat system prompt (config not found)") | |
| return """You are a helpful AI assistant with access to the user's personal memories and conversation history. | |
| Use the provided memories and conversation context to give personalized, contextual responses. If memories are relevant, reference them naturally in your response. Be conversational and helpful. | |
| If no relevant memories are available, respond normally based on the conversation context.""" | |
| def _get_system_prompt(self) -> str: | |
| """ | |
| Get system prompt from config with fallback to default. | |
| Returns: | |
| str: System prompt for chat interactions | |
| """ | |
| try: | |
| reg = get_models_registry() | |
| if reg: | |
| chat_config = reg.chat # Always exists with default_factory=dict | |
| prompt = chat_config.get('system_prompt') | |
| if prompt: | |
| logger.info(f"✅ Loaded chat system prompt from config (length: {len(prompt)} chars)") | |
| logger.debug(f"System prompt: {prompt[:100]}...") | |
| return prompt | |
| except Exception as e: | |
| logger.exception("Failed to load chat system prompt from config") | |
| # Fallback to default | |
| logger.info("⚠️ Using default chat system prompt (config not found)") | |
| return """You are a helpful AI assistant with access to the user's personal memories and conversation history. | |
| Use the provided memories and conversation context to give personalized, contextual responses. If memories are relevant, reference them naturally in your response. Be conversational and helpful. | |
| If no relevant memories are available, respond normally based on the conversation context.""" |
🧰 Tools
🪛 Ruff (0.14.10)
162-162: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In backends/advanced/src/advanced_omi_backend/chat_service.py around lines 146
to 172, remove the unnecessary hasattr(reg, 'chat') guard (since reg.chat always
exists) and directly access reg.chat to retrieve 'system_prompt'; if prompt is
present return it as before. In the except block replace logger.warning(...)
with logger.exception(...) so the full stack trace is logged for debugging.
Ensure the logic still falls back to the default prompt when reg or prompt are
missing.
| logger.debug(f"Fetched {len(entity_details)} entity states") | ||
|
|
||
| # Create cache | ||
| from datetime import datetime |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move imports to the top of the file.
Per coding guidelines, ALL imports must be at the top of the file. The following mid-function imports violate this:
- Line 297:
from datetime import datetime - Lines 335-336:
from advanced_omi_backend.llm_client import get_llm_clientandfrom .command_parser import ... - Line 416:
from .command_parser import ParsedCommand - Lines 549-550:
import asyncioandfrom .command_parser import ParsedCommand
🔎 Proposed fix - add to top of file
import json
import logging
+import asyncio
from typing import Any, Dict, List, Optional
+from datetime import datetime
from ..base import BasePlugin, PluginContext, PluginResult
from .entity_cache import EntityCache
from .mcp_client import HAMCPClient, MCPError
+from .command_parser import COMMAND_PARSER_SYSTEM_PROMPT, ParsedCommand
+from advanced_omi_backend.llm_client import get_llm_clientAs per coding guidelines, imports must be at the top of the file after the docstring.
Also applies to: 335-336, 416-416, 549-550
🤖 Prompt for AI Agents
In backends/advanced/src/advanced_omi_backend/plugins/homeassistant/plugin.py
around lines 297, 335-336, 416, and 549-550, several imports are performed
mid-file; move the following imports to the top of the file (directly after the
module docstring): from datetime import datetime; import asyncio; from
advanced_omi_backend.llm_client import get_llm_client; and all usages of from
.command_parser import ParsedCommand (or the full names used). Consolidate
duplicate imports into a single top-level import, order them with other imports
per project style, and only keep mid-function imports if a circular-import
problem forces it (in that case add a brief inline comment explaining the
exception).
|
| Metric | Count |
|---|---|
| ✅ Passed | 94 |
| ❌ Failed | 1 |
| 📊 Total | 95 |
📊 View Reports
GitHub Pages (Live Reports):
Download Artifacts:
- robot-test-reports-html - HTML reports
- robot-test-results-xml - XML output
- Updated .gitignore to include plugins.yml for security reasons. - Modified start.sh to allow passing additional arguments during service startup. - Refactored wizard.py to support new HF_TOKEN configuration prompts and improved handling of wake words in plugin settings. - Introduced a new setup_hf_token_if_needed function to streamline Hugging Face token management. - Enhanced the GitHub Actions workflow to create plugins.yml from a template, ensuring proper configuration setup. - Added detailed comments and documentation in the plugins.yml.template for better user guidance on Home Assistant integration.
…word processing - Added asynchronous Redis support in ClientManager for tracking client-user relationships. - Introduced `initialize_redis_for_client_manager` to set up Redis for cross-container mapping. - Updated `create_client_state` to use asynchronous tracking for client-user relationships. - Enhanced wake word processing in PluginRouter with normalization and command extraction. - Refactored DeepgramStreamingConsumer to utilize async Redis lookups for user ID retrieval. - Set TTL on Redis streams during client state cleanup for better resource management.
|
| Metric | Count |
|---|---|
| ✅ Passed | 93 |
| ❌ Failed | 2 |
| 📊 Total | 95 |
📊 View Reports
GitHub Pages (Live Reports):
Download Artifacts:
- robot-test-reports-html - HTML reports
- robot-test-results-xml - XML output
|
| Metric | Count |
|---|---|
| ✅ Passed | 94 |
| ❌ Failed | 1 |
| 📊 Total | 95 |
📊 View Reports
GitHub Pages (Live Reports):
Download Artifacts:
- robot-test-reports-html - HTML reports
- robot-test-results-xml - XML output
- Disabled the batch Deepgram worker in favor of the streaming worker to prevent race conditions. - Updated text normalization in wake word processing to replace punctuation with spaces, preserving word boundaries. - Enhanced regex pattern for wake word matching to allow optional punctuation and whitespace after the last part. - Improved logging in DeepgramStreamingConsumer for better visibility of message processing and error handling.
|
| Metric | Count |
|---|---|
| ✅ Passed | 89 |
| ❌ Failed | 6 |
| 📊 Total | 95 |
📊 View Reports
GitHub Pages (Live Reports):
Download Artifacts:
- robot-test-reports-html - HTML reports
- robot-test-results-xml - XML output
|
This PR actually introduces plugin system, as well as introduces one plugin (home assistant). It also changes how the workers consume redis data streams and switches to fan-out pattern |
- Implemented retrieval of the original chat prompt before saving a custom prompt to ensure test isolation. - Added restoration of the original prompt after the test to prevent interference with subsequent tests. - Enhanced the test documentation for clarity on the purpose of these changes.
|
| Metric | Count |
|---|---|
| ✅ Passed | 90 |
| ❌ Failed | 5 |
| 📊 Total | 95 |
📊 View Reports
GitHub Pages (Live Reports):
Download Artifacts:
- robot-test-reports-html - HTML reports
- robot-test-results-xml - XML output
- Simplified test execution commands in CLAUDE.md and quickstart.md for better usability. - Added instructions for running tests from the project root and clarified the process for executing the complete Robot Framework test suite. - Introduced a new Docker service for the Deepgram streaming worker in docker-compose-test.yml to improve testing capabilities. - Updated system_admin_tests.robot to use a defined default prompt for restoration, enhancing test reliability and clarity.
|
| Metric | Count |
|---|---|
| ✅ Passed | 90 |
| ❌ Failed | 5 |
| 📊 Total | 95 |
📊 View Reports
GitHub Pages (Live Reports):
Download Artifacts:
- robot-test-reports-html - HTML reports
- robot-test-results-xml - XML output
01PrathamS
left a comment
01PrathamS
left a comment
There was a problem hiding this comment.
got the idea, LGTM!!
will try each of the pieces and contribute for improvements.
- Updated `run-test.sh` and `run-robot-tests.sh` to improve cleanup processes, including handling permission issues with Docker. - Introduced a new function `mark_session_complete` in `session_controller.py` to ensure atomic updates for session completion status. - Refactored WebSocket and conversation job handling to utilize the new session completion function, enhancing reliability. - Updated `start-workers.sh` to enable the batch Deepgram worker alongside the streaming worker for improved transcription capabilities. - Enhanced test scripts to verify the status of Deepgram workers and ensure proper cleanup of test containers.
|
| Metric | Count |
|---|---|
| ✅ Passed | 94 |
| ❌ Failed | 1 |
| 📊 Total | 95 |
📊 View Reports
GitHub Pages (Live Reports):
Download Artifacts:
- robot-test-reports-html - HTML reports
- robot-test-results-xml - XML output
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.