From a8a91972ba1b7393b300b96575d52511f16c594b Mon Sep 17 00:00:00 2001 From: SMKRV Date: Thu, 12 Mar 2026 02:24:09 +0300 Subject: [PATCH] =?UTF-8?q?fix:=20Comprehensive=20v2.4.0=20quality=20pass?= =?UTF-8?q?=20=E2=80=94=2024=20review=20findings=20+=20review=20agent=20fi?= =?UTF-8?q?xes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase A — Critical: - Remove dual timeout stacking in coordinator._send_to_api - Add Gemini-specific asyncio.timeout (sync SDK via to_thread) - Store full text in history for context; cap per-field at 32KB on disk - Fix instance lookup to match by normalized_name - Add Bearer/sk-/x-api-key credential sanitization patterns Phase B — Dead code removal: - Merge async_ask_question/async_process_question into single method - Remove dead is_anthropic flag from coordinator and __init__ - Remove unused DEFAULT_TIMEOUT and API_TIMEOUT constants - Remove redundant _create_history_dir calls - Consolidate async_check_api to use provider registry Phase C — Config flow correctness: - Truncate name before uniqueness check (prevent post-truncation collisions) - Add async_set_unique_id + _abort_if_unique_id_configured - Extract shared _build_parameter_schema for ConfigFlow/OptionsFlow dedup Phase D — UX improvements: - Optimize history write: serialize from memory, single file write - Show last 5 history entries in sensor attributes (was 1) - Return actual error type in ask_question service response - Add dedicated api_key_required error for provider/endpoint changes - Pass config_entry to DataUpdateCoordinator (HA 2024.8+) Phase E — Cleanup: - Extract _apply_structured_output for OpenAI/DeepSeek dedup - Reduce ABSOLUTE_MAX_HISTORY_SIZE to 200 with Final annotation - Remove dead translation keys (queued, invalid_characters) - Migrate to _attr_has_entity_name = True - Add from __future__ import annotations to all modules - Remove redundant api_status sensor attribute - Add missing translation keys (last_model, last_timestamp, etc.) Review agent fixes: - Add archive file cleanup (max 3 archives) to prevent disk exhaustion - Per-entry storage cap (32KB per field) for history on disk --- custom_components/ha_text_ai/__init__.py | 52 +++--- custom_components/ha_text_ai/api_client.py | 71 ++++---- custom_components/ha_text_ai/config_flow.py | 158 +++++++----------- custom_components/ha_text_ai/const.py | 7 +- custom_components/ha_text_ai/coordinator.py | 47 ++---- custom_components/ha_text_ai/history.py | 125 +++++++------- custom_components/ha_text_ai/metrics.py | 5 +- custom_components/ha_text_ai/providers.py | 2 + custom_components/ha_text_ai/sensor.py | 9 +- custom_components/ha_text_ai/strings.json | 23 ++- .../ha_text_ai/translations/en.json | 23 ++- .../ha_text_ai/translations/ru.json | 23 ++- custom_components/ha_text_ai/utils.py | 2 + 13 files changed, 272 insertions(+), 275 deletions(-) diff --git a/custom_components/ha_text_ai/__init__.py b/custom_components/ha_text_ai/__init__.py index ac2f104..9ddd4f9 100644 --- a/custom_components/ha_text_ai/__init__.py +++ b/custom_components/ha_text_ai/__init__.py @@ -25,7 +25,7 @@ from homeassistant.util import dt as dt_util from .coordinator import HATextAICoordinator from .api_client import APIClient -from .utils import safe_log_data, validate_endpoint +from .utils import normalize_name, safe_log_data, validate_endpoint from .providers import get_default_endpoint, get_default_model, build_auth_headers from .const import ( DOMAIN, @@ -38,9 +38,6 @@ from .const import ( CONF_API_TIMEOUT, CONF_API_PROVIDER, CONF_CONTEXT_MESSAGES, - API_PROVIDER_ANTHROPIC, - API_PROVIDER_DEEPSEEK, - API_PROVIDER_GEMINI, DEFAULT_TEMPERATURE, DEFAULT_MAX_TOKENS, DEFAULT_REQUEST_INTERVAL, @@ -87,12 +84,22 @@ SERVICE_SCHEMA_GET_HISTORY = vol.Schema({ }) def get_coordinator_by_instance(hass: HomeAssistant, instance: str) -> HATextAICoordinator: - """Get coordinator by instance name.""" + """Get coordinator by instance name or normalized name. + + Accepts instance_name, normalized_name, or sensor entity_id. + """ if instance.startswith("sensor."): instance = instance.replace("sensor.ha_text_ai_", "", 1) + normalized_input = normalize_name(instance) + for entry_id, coord in hass.data[DOMAIN].items(): - if isinstance(coord, HATextAICoordinator) and coord.instance_name.lower() == instance.lower(): + if not isinstance(coord, HATextAICoordinator): + continue + if ( + coord.instance_name.lower() == instance.lower() + or coord.normalized_name == normalized_input + ): return coord raise HomeAssistantError(f"Instance {instance} not found") @@ -142,7 +149,8 @@ async def async_setup(hass: HomeAssistant, config: Dict[str, Any]) -> bool: "question": call.data["question"], "timestamp": dt_util.utcnow().isoformat(), "success": False, - "error": "Service call failed" + "error": str(err), + "error_type": type(err).__name__ } async def async_clear_history(call: ServiceCall) -> None: @@ -212,21 +220,22 @@ async def async_setup(hass: HomeAssistant, config: Dict[str, Any]) -> bool: return True async def async_check_api(session, endpoint: str, headers: dict, provider: str, api_timeout: int = DEFAULT_API_TIMEOUT) -> bool: - """Check API availability for different providers.""" + """Check API availability using provider registry configuration.""" try: - if provider == API_PROVIDER_GEMINI: - # Gemini API does not support GET /models for validation, just check key presence - if headers.get("Authorization", "").replace("Bearer ", ""): + from .providers import get_provider_config + provider_config = get_provider_config(provider) + check_path = provider_config.get("check_path") + + if check_path is None: + # Provider does not support /models check (e.g. Gemini) + auth_header = provider_config["auth_header"] + auth_value = headers.get(auth_header, "").replace(provider_config.get("auth_prefix", ""), "") + if auth_value: return True - else: - _LOGGER.error("Gemini API key is missing or empty") - return False - elif provider == API_PROVIDER_ANTHROPIC: - check_url = f"{endpoint}/v1/models" - elif provider == API_PROVIDER_DEEPSEEK: - check_url = f"{endpoint}/models" - else: # OpenAI - check_url = f"{endpoint}/models" + _LOGGER.error("API key is missing or empty for %s", provider) + return False + + check_url = f"{endpoint}{check_path}" async with asyncio.timeout(api_timeout): async with session.get(check_url, headers=headers) as response: @@ -276,7 +285,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: temperature = config.get(CONF_TEMPERATURE, DEFAULT_TEMPERATURE) max_history_size = config.get(CONF_MAX_HISTORY_SIZE, DEFAULT_MAX_HISTORY) context_messages = config.get(CONF_CONTEXT_MESSAGES, DEFAULT_CONTEXT_MESSAGES) - is_anthropic = api_provider == API_PROVIDER_ANTHROPIC headers = build_auth_headers(api_provider, api_key) @@ -301,11 +309,11 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: model=model, update_interval=request_interval, instance_name=instance_name, + config_entry=entry, max_tokens=max_tokens, temperature=temperature, max_history_size=max_history_size, context_messages=context_messages, - is_anthropic=is_anthropic, api_timeout=api_timeout, ) diff --git a/custom_components/ha_text_ai/api_client.py b/custom_components/ha_text_ai/api_client.py index e744d45..8f88360 100644 --- a/custom_components/ha_text_ai/api_client.py +++ b/custom_components/ha_text_ai/api_client.py @@ -6,6 +6,8 @@ API Client for HA Text AI. @github: https://github.com/smkrv/ha-text-ai @source: https://github.com/smkrv/ha-text-ai """ +from __future__ import annotations + import logging import asyncio from typing import Any, Dict, List, Optional @@ -182,6 +184,30 @@ class APIClient: _LOGGER.error("API request failed: %s", str(e)) raise HomeAssistantError(f"API request failed: {str(e)}") + @staticmethod + def _apply_structured_output( + payload: Dict[str, Any], + structured_output: bool, + json_schema: Optional[str], + ) -> None: + """Apply OpenAI-compatible structured output to payload in-place.""" + if not (structured_output and json_schema): + return + import json + try: + schema = json.loads(json_schema) + payload["response_format"] = { + "type": "json_schema", + "json_schema": { + "name": "structured_response", + "strict": True, + "schema": schema, + }, + } + except json.JSONDecodeError as e: + _LOGGER.warning("Invalid JSON schema: %s. Falling back to json_object.", e) + payload["response_format"] = {"type": "json_object"} + async def _create_deepseek_completion( self, model: str, @@ -198,26 +224,9 @@ class APIClient: "messages": messages, "temperature": temperature, "max_tokens": max_tokens, - "stream": False + "stream": False, } - - # Add structured output format if enabled (DeepSeek is OpenAI-compatible) - if structured_output and json_schema: - try: - import json - schema = json.loads(json_schema) - payload["response_format"] = { - "type": "json_schema", - "json_schema": { - "name": "structured_response", - "strict": True, - "schema": schema - } - } - _LOGGER.debug("DeepSeek structured output enabled with schema") - except json.JSONDecodeError as e: - _LOGGER.warning("Invalid JSON schema provided: %s. Falling back to json_object mode.", e) - payload["response_format"] = {"type": "json_object"} + self._apply_structured_output(payload, structured_output, json_schema) data = await self._make_request(url, payload) return { @@ -250,24 +259,7 @@ class APIClient: "temperature": temperature, "max_tokens": max_tokens, } - - # Add structured output format if enabled - if structured_output and json_schema: - try: - import json - schema = json.loads(json_schema) - payload["response_format"] = { - "type": "json_schema", - "json_schema": { - "name": "structured_response", - "strict": True, - "schema": schema - } - } - _LOGGER.debug("OpenAI structured output enabled with schema") - except json.JSONDecodeError as e: - _LOGGER.warning("Invalid JSON schema provided: %s. Falling back to json_object mode.", e) - payload["response_format"] = {"type": "json_object"} + self._apply_structured_output(payload, structured_output, json_schema) data = await self._make_request(url, payload) return { @@ -469,7 +461,10 @@ class APIClient: ) return chat.send_message(last_user_msg) - response = await asyncio.to_thread(generate_content) + # Gemini uses sync SDK via to_thread, so needs its own timeout + # (aiohttp ClientTimeout doesn't apply here) + async with asyncio.timeout(self.api_timeout): + response = await asyncio.to_thread(generate_content) # Extract response text def extract_response(): diff --git a/custom_components/ha_text_ai/config_flow.py b/custom_components/ha_text_ai/config_flow.py index 99415e9..2999764 100644 --- a/custom_components/ha_text_ai/config_flow.py +++ b/custom_components/ha_text_ai/config_flow.py @@ -6,6 +6,8 @@ Config flow for HA text AI integration. @github: https://github.com/smkrv/ha-text-ai @source: https://github.com/smkrv/ha-text-ai """ +from __future__ import annotations + import logging from typing import Any, Dict, Optional @@ -61,6 +63,36 @@ from .providers import get_default_endpoint, get_default_model, build_auth_heade _LOGGER = logging.getLogger(__name__) +def _build_parameter_schema(data: Dict[str, Any]) -> dict: + """Build shared parameter schema fields used by both ConfigFlow and OptionsFlow.""" + return { + vol.Optional( + CONF_TEMPERATURE, + default=data.get(CONF_TEMPERATURE, DEFAULT_TEMPERATURE), + ): vol.All(vol.Coerce(float), vol.Range(min=MIN_TEMPERATURE, max=MAX_TEMPERATURE)), + vol.Optional( + CONF_MAX_TOKENS, + default=data.get(CONF_MAX_TOKENS, DEFAULT_MAX_TOKENS), + ): vol.All(vol.Coerce(int), vol.Range(min=MIN_MAX_TOKENS, max=MAX_MAX_TOKENS)), + vol.Optional( + CONF_REQUEST_INTERVAL, + default=data.get(CONF_REQUEST_INTERVAL, DEFAULT_REQUEST_INTERVAL), + ): vol.All(vol.Coerce(float), vol.Range(min=MIN_REQUEST_INTERVAL)), + vol.Optional( + CONF_API_TIMEOUT, + default=data.get(CONF_API_TIMEOUT, DEFAULT_API_TIMEOUT), + ): vol.All(vol.Coerce(int), vol.Range(min=MIN_API_TIMEOUT, max=MAX_API_TIMEOUT)), + vol.Optional( + CONF_CONTEXT_MESSAGES, + default=data.get(CONF_CONTEXT_MESSAGES, DEFAULT_CONTEXT_MESSAGES), + ): vol.All(vol.Coerce(int), vol.Range(min=MIN_CONTEXT_MESSAGES, max=MAX_CONTEXT_MESSAGES)), + vol.Optional( + CONF_MAX_HISTORY_SIZE, + default=data.get(CONF_MAX_HISTORY_SIZE, DEFAULT_MAX_HISTORY), + ): vol.All(vol.Coerce(int), vol.Range(min=MIN_HISTORY_SIZE, max=MAX_HISTORY_SIZE)), + } + + class HATextAIConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow for HA text AI.""" @@ -95,42 +127,14 @@ class HATextAIConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): ) -> vol.Schema: """Build provider configuration schema with optional defaults from data.""" defaults = data or {} - return vol.Schema({ + schema_dict = { vol.Required(CONF_NAME, default=defaults.get(CONF_NAME, DEFAULT_INSTANCE_NAME)): str, vol.Required(CONF_API_KEY): str, vol.Required(CONF_MODEL, default=defaults.get(CONF_MODEL, get_default_model(self._provider))): str, vol.Required(CONF_API_ENDPOINT, default=defaults.get(CONF_API_ENDPOINT, get_default_endpoint(self._provider))): str, - vol.Optional(CONF_TEMPERATURE, default=defaults.get(CONF_TEMPERATURE, DEFAULT_TEMPERATURE)): vol.All( - vol.Coerce(float), - vol.Range(min=MIN_TEMPERATURE, max=MAX_TEMPERATURE) - ), - vol.Optional(CONF_MAX_TOKENS, default=defaults.get(CONF_MAX_TOKENS, DEFAULT_MAX_TOKENS)): vol.All( - vol.Coerce(int), - vol.Range(min=MIN_MAX_TOKENS, max=MAX_MAX_TOKENS) - ), - vol.Optional(CONF_REQUEST_INTERVAL, default=defaults.get(CONF_REQUEST_INTERVAL, DEFAULT_REQUEST_INTERVAL)): vol.All( - vol.Coerce(float), - vol.Range(min=MIN_REQUEST_INTERVAL) - ), - vol.Optional(CONF_API_TIMEOUT, default=defaults.get(CONF_API_TIMEOUT, DEFAULT_API_TIMEOUT)): vol.All( - vol.Coerce(int), - vol.Range(min=MIN_API_TIMEOUT, max=MAX_API_TIMEOUT) - ), - vol.Optional( - CONF_CONTEXT_MESSAGES, - default=defaults.get(CONF_CONTEXT_MESSAGES, DEFAULT_CONTEXT_MESSAGES) - ): vol.All( - vol.Coerce(int), - vol.Range(min=MIN_CONTEXT_MESSAGES, max=MAX_CONTEXT_MESSAGES) - ), - vol.Optional( - CONF_MAX_HISTORY_SIZE, - default=defaults.get(CONF_MAX_HISTORY_SIZE, DEFAULT_MAX_HISTORY) - ): vol.All( - vol.Coerce(int), - vol.Range(min=MIN_HISTORY_SIZE, max=MAX_HISTORY_SIZE) - ), - }) + } + schema_dict.update(_build_parameter_schema(defaults)) + return vol.Schema(schema_dict) async def async_step_provider(self, user_input: Optional[Dict[str, Any]] = None) -> ConfigFlowResult: """Handle provider configuration step.""" @@ -190,35 +194,25 @@ class HATextAIConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): return await self._create_entry(input_copy) def _validate_and_normalize_name(self, name: str) -> str: - """ - Validate and normalize name with detailed error handling. + """Validate and normalize name. + + Truncates before uniqueness check to prevent collisions. Raises: - ValueError: If name is invalid - - Returns: - Normalized name + ValueError: If name is invalid or already exists. """ - if not name: + if not name or not name.strip(): raise ValueError("empty") - name = name.strip() - normalized = ''.join( - c if c.isalnum() or c in ' _' else '_' # Only allow underscores - for c in name - ) + normalized = normalize_name(name.strip())[:50] - normalized = normalized.replace(' ', '_').lower() + if not normalized: + raise ValueError("empty") for entry in self._async_current_entries(): if entry.data.get(CONF_NAME, "") == normalized: raise ValueError("name_exists") - normalized = normalized[:50] - - if not normalized: - raise ValueError("empty") - return normalized async def _async_validate_api(self, user_input: Dict[str, Any]) -> bool: @@ -264,21 +258,21 @@ class HATextAIConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): return False async def _create_entry(self, user_input: Dict[str, Any]) -> ConfigFlowResult: - """Create the config entry with comprehensive data preservation.""" + """Create the config entry with unique_id deduplication.""" instance_name = user_input[CONF_NAME] normalized_name = normalize_name(instance_name) - unique_id = f"{DOMAIN}_{normalized_name}_{self._provider}".lower() + unique_id = f"{DOMAIN}_{normalized_name}_{self._provider}" + await self.async_set_unique_id(unique_id) + self._abort_if_unique_id_configured() default_model = get_default_model(self._provider) entry_data = { CONF_API_PROVIDER: self._provider, CONF_NAME: instance_name, - "normalized_name": normalized_name, CONF_API_KEY: user_input.get(CONF_API_KEY), CONF_API_ENDPOINT: user_input.get(CONF_API_ENDPOINT), - "unique_id": unique_id, CONF_MODEL: user_input.get(CONF_MODEL, default_model), CONF_TEMPERATURE: user_input.get(CONF_TEMPERATURE, DEFAULT_TEMPERATURE), CONF_MAX_TOKENS: user_input.get(CONF_MAX_TOKENS, DEFAULT_MAX_TOKENS), @@ -398,7 +392,7 @@ class OptionsFlowHandler(config_entries.OptionsFlow): stored_endpoint = current_data.get(CONF_API_ENDPOINT, "") endpoint_changed = endpoint != stored_endpoint if not api_key and (provider_changed or endpoint_changed): - self._errors["base"] = "invalid_auth" + self._errors["base"] = "api_key_required" return self.async_show_form( step_id="settings", data_schema=self._get_settings_schema( @@ -464,59 +458,19 @@ class OptionsFlowHandler(config_entries.OptionsFlow): default_endpoint: str, default_model: str, ) -> vol.Schema: - """Build settings schema.""" + """Build settings schema using shared parameter definitions.""" data = user_input or current_data - return vol.Schema({ + schema_dict = { vol.Optional(CONF_API_KEY, default=""): str, vol.Required( CONF_API_ENDPOINT, - default=data.get(CONF_API_ENDPOINT, default_endpoint) + default=data.get(CONF_API_ENDPOINT, default_endpoint), ): str, vol.Required( CONF_MODEL, - default=data.get(CONF_MODEL, default_model) + default=data.get(CONF_MODEL, default_model), ): str, - vol.Optional( - CONF_TEMPERATURE, - default=data.get(CONF_TEMPERATURE, DEFAULT_TEMPERATURE) - ): vol.All( - vol.Coerce(float), - vol.Range(min=MIN_TEMPERATURE, max=MAX_TEMPERATURE) - ), - vol.Optional( - CONF_MAX_TOKENS, - default=data.get(CONF_MAX_TOKENS, DEFAULT_MAX_TOKENS) - ): vol.All( - vol.Coerce(int), - vol.Range(min=MIN_MAX_TOKENS, max=MAX_MAX_TOKENS) - ), - vol.Optional( - CONF_REQUEST_INTERVAL, - default=data.get(CONF_REQUEST_INTERVAL, DEFAULT_REQUEST_INTERVAL) - ): vol.All( - vol.Coerce(float), - vol.Range(min=MIN_REQUEST_INTERVAL) - ), - vol.Optional( - CONF_API_TIMEOUT, - default=data.get(CONF_API_TIMEOUT, DEFAULT_API_TIMEOUT) - ): vol.All( - vol.Coerce(int), - vol.Range(min=MIN_API_TIMEOUT, max=MAX_API_TIMEOUT) - ), - vol.Optional( - CONF_CONTEXT_MESSAGES, - default=data.get(CONF_CONTEXT_MESSAGES, DEFAULT_CONTEXT_MESSAGES) - ): vol.All( - vol.Coerce(int), - vol.Range(min=MIN_CONTEXT_MESSAGES, max=MAX_CONTEXT_MESSAGES) - ), - vol.Optional( - CONF_MAX_HISTORY_SIZE, - default=data.get(CONF_MAX_HISTORY_SIZE, DEFAULT_MAX_HISTORY) - ): vol.All( - vol.Coerce(int), - vol.Range(min=MIN_HISTORY_SIZE, max=MAX_HISTORY_SIZE) - ), - }) + } + schema_dict.update(_build_parameter_schema(data)) + return vol.Schema(schema_dict) diff --git a/custom_components/ha_text_ai/const.py b/custom_components/ha_text_ai/const.py index 7c6ec9e..135d45c 100644 --- a/custom_components/ha_text_ai/const.py +++ b/custom_components/ha_text_ai/const.py @@ -6,6 +6,8 @@ Constants for the HA text AI integration. @github: https://github.com/smkrv/ha-text-ai @source: https://github.com/smkrv/ha-text-ai """ +from __future__ import annotations + from typing import Final from homeassistant.const import Platform @@ -44,12 +46,11 @@ CONF_REQUEST_INTERVAL: Final = "request_interval" CONF_API_TIMEOUT: Final = "api_timeout" CONF_INSTANCE: Final = "instance" CONF_MAX_HISTORY_SIZE: Final = "max_history_size" # Correct constant name -CONF_IS_ANTHROPIC: Final = "is_anthropic" CONF_CONTEXT_MESSAGES: Final = "context_messages" CONF_STRUCTURED_OUTPUT: Final = "structured_output" CONF_JSON_SCHEMA: Final = "json_schema" -ABSOLUTE_MAX_HISTORY_SIZE = 500 +ABSOLUTE_MAX_HISTORY_SIZE: Final = 200 # Hard cap; UI allows max MAX_HISTORY_SIZE (100) MAX_ATTRIBUTE_SIZE = 4 * 1024 MAX_HISTORY_FILE_SIZE = 1 * 1024 * 1024 # Default values @@ -60,7 +61,6 @@ DEFAULT_GEMINI_MODEL: Final = "gemini-2.0-flash" DEFAULT_TEMPERATURE: Final = 0.1 DEFAULT_MAX_TOKENS: Final = 1000 DEFAULT_REQUEST_INTERVAL: Final = 1.0 -DEFAULT_TIMEOUT: Final = 30 DEFAULT_API_TIMEOUT: Final = 30 DEFAULT_MAX_HISTORY: Final = 50 DEFAULT_NAME: Final = "HA Text AI" @@ -85,7 +85,6 @@ MIN_API_TIMEOUT: Final = 5 MAX_API_TIMEOUT: Final = 600 # API constants -API_TIMEOUT: Final = 30 # Legacy constant, use CONF_API_TIMEOUT from config API_RETRY_COUNT: Final = 3 # Service names diff --git a/custom_components/ha_text_ai/coordinator.py b/custom_components/ha_text_ai/coordinator.py index b0670b0..2122331 100644 --- a/custom_components/ha_text_ai/coordinator.py +++ b/custom_components/ha_text_ai/coordinator.py @@ -14,6 +14,7 @@ import os from datetime import timedelta from typing import Any, Dict, List, Optional +from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.update_coordinator import DataUpdateCoordinator @@ -49,11 +50,11 @@ class HATextAICoordinator(DataUpdateCoordinator): model: str, update_interval: int, instance_name: str, + config_entry: ConfigEntry, max_tokens: int = DEFAULT_MAX_TOKENS, temperature: float = DEFAULT_TEMPERATURE, max_history_size: int = DEFAULT_MAX_HISTORY, context_messages: int = DEFAULT_CONTEXT_MESSAGES, - is_anthropic: bool = False, api_timeout: int = DEFAULT_API_TIMEOUT, ) -> None: """Initialize coordinator.""" @@ -87,7 +88,6 @@ class HATextAICoordinator(DataUpdateCoordinator): self.model = model self.temperature = temperature self.max_tokens = max_tokens - self.is_anthropic = is_anthropic self.api_timeout = api_timeout # Concurrency control @@ -115,6 +115,7 @@ class HATextAICoordinator(DataUpdateCoordinator): _LOGGER, name=instance_name, update_interval=timedelta(seconds=update_interval), + config_entry=config_entry, ) self.available = True @@ -212,23 +213,6 @@ class HATextAICoordinator(DataUpdateCoordinator): context_messages: Optional[int] = None, structured_output: bool = False, json_schema: Optional[str] = None, - ) -> dict: - """Process a question with optional parameters.""" - return await self.async_process_question( - question, model, temperature, max_tokens, system_prompt, - context_messages, structured_output, json_schema, - ) - - async def async_process_question( - self, - question: str, - model: Optional[str] = None, - temperature: Optional[float] = None, - max_tokens: Optional[int] = None, - system_prompt: Optional[str] = None, - context_messages: Optional[int] = None, - structured_output: bool = False, - json_schema: Optional[str] = None, ) -> dict: """Process question with context management.""" if self.client is None: @@ -295,17 +279,20 @@ class HATextAICoordinator(DataUpdateCoordinator): structured_output: bool = False, json_schema: Optional[str] = None, ) -> dict: - """Send request to AI provider and return structured response.""" + """Send request to AI provider and return structured response. + + Note: timeout is handled by APIClient via aiohttp ClientTimeout. + No additional asyncio.timeout wrapper to avoid dual timeout stacking. + """ try: - async with asyncio.timeout(self.api_timeout): - response = await self.client.create( - model=model, - messages=messages, - temperature=temperature, - max_tokens=max_tokens, - structured_output=structured_output, - json_schema=json_schema, - ) + response = await self.client.create( + model=model, + messages=messages, + temperature=temperature, + max_tokens=max_tokens, + structured_output=structured_output, + json_schema=json_schema, + ) # Reset error state on success self._is_rate_limited = False @@ -339,8 +326,6 @@ class HATextAICoordinator(DataUpdateCoordinator): "success": True, } - except asyncio.TimeoutError: - raise HomeAssistantError("Request timed out") except Exception as err: _LOGGER.error("Error in API call: %s", err) raise diff --git a/custom_components/ha_text_ai/history.py b/custom_components/ha_text_ai/history.py index f2d9ce8..51f55fa 100644 --- a/custom_components/ha_text_ai/history.py +++ b/custom_components/ha_text_ai/history.py @@ -28,6 +28,10 @@ from .const import ( TRUNCATION_INDICATOR, ) +# Per-entry storage cap (32KB per field) to prevent disk exhaustion +MAX_STORED_FIELD_SIZE = 32 * 1024 +MAX_ARCHIVE_FILES = 3 + _LOGGER = logging.getLogger(__name__) @@ -107,7 +111,6 @@ class HistoryManager: async def _check_history_directory(self) -> None: """Check history directory permissions and writability.""" try: - await self._create_history_dir() test_file_path = os.path.join(self._history_dir, ".write_test") await self.hass.async_add_executor_job( self._sync_test_directory_write, test_file_path @@ -129,8 +132,6 @@ class HistoryManager: async def _initialize_history_file(self) -> None: """Initialize history file and load existing history.""" try: - await self._create_history_dir() - if await self._file_exists(self._history_file): async with AsyncFileHandler(self._history_file, "r") as f: content = await f.read() @@ -155,71 +156,43 @@ class HistoryManager: _LOGGER.debug(traceback.format_exc()) async def update_history(self, question: str, response: dict) -> None: - """Update conversation history with size validation.""" + """Update conversation history. + + In-memory history stores full text for context retrieval. + On-disk storage caps per-field size to prevent disk exhaustion. + Display truncation is handled by get_limited_history(). + """ try: + content = response.get("content", "") history_entry = { "timestamp": dt_util.utcnow().isoformat(), - "question": self._truncate_text(question, MAX_ATTRIBUTE_SIZE), - "response": self._truncate_text( - response.get("content", ""), MAX_ATTRIBUTE_SIZE - ), + "question": question[:MAX_STORED_FIELD_SIZE], + "response": content[:MAX_STORED_FIELD_SIZE], } - entry_size = len(json.dumps(history_entry).encode("utf-8")) - current_size = await self._check_file_size(self._history_file) - - if current_size + entry_size > MAX_HISTORY_FILE_SIZE: - _LOGGER.warning( - "History size limit approaching. Current: %d, Entry: %d, Max: %d", - current_size, entry_size, MAX_HISTORY_FILE_SIZE, - ) - await self._rotate_history() - self._conversation_history.append(history_entry) while len(self._conversation_history) > self.max_history_size: self._conversation_history.pop(0) - await self._write_history_entry(history_entry) + await self._save_history_to_file() except Exception as e: _LOGGER.error("Error updating history: %s", e) _LOGGER.debug(traceback.format_exc()) - async def _write_history_entry(self, entry: dict) -> None: - """Write history entry with file size checks.""" + async def _save_history_to_file(self) -> None: + """Serialize in-memory history to file with rotation if needed.""" try: - if not await self._file_exists(self._history_dir): - await self._create_history_dir() + data = json.dumps(self._conversation_history, indent=2) + data_size = len(data.encode("utf-8")) - current_size = 0 - if await self._file_exists(self._history_file): - current_size = await self.hass.async_add_executor_job( - os.path.getsize, self._history_file - ) - - entry_size = len(json.dumps(entry).encode("utf-8")) - if current_size + entry_size > MAX_HISTORY_FILE_SIZE: - _LOGGER.warning( - "History file size limit reached. Current: %d, Entry: %d, Max: %d", - current_size, entry_size, MAX_HISTORY_FILE_SIZE, - ) + if data_size > MAX_HISTORY_FILE_SIZE: await self._rotate_history() - history = [] - if await self._file_exists(self._history_file): - async with AsyncFileHandler(self._history_file, "r") as f: - content = await f.read() - if content: - history = json.loads(content) - - history.append(entry) - if len(history) > self.max_history_size: - history = history[-self.max_history_size :] - async with AsyncFileHandler(self._history_file, "w") as f: - await f.write(json.dumps(history, indent=2)) + await f.write(data) except Exception as e: - _LOGGER.error("Error writing history entry: %s", e) + _LOGGER.error("Error writing history file: %s", e) _LOGGER.debug(traceback.format_exc()) async def _check_history_size(self) -> None: @@ -283,10 +256,34 @@ class HistoryManager: ) _LOGGER.info("History file rotated to: %s", archive_file) + + # Clean up old archive files, keep only MAX_ARCHIVE_FILES + await self._cleanup_archives() except Exception as e: _LOGGER.error("History rotation failed: %s", e) _LOGGER.debug(traceback.format_exc()) + async def _cleanup_archives(self) -> None: + """Remove old archive files beyond MAX_ARCHIVE_FILES.""" + try: + prefix = f"{self.normalized_name}_history_" + + def find_archives(): + archives = [] + for f in os.listdir(self._history_dir): + if f.startswith(prefix) and f.endswith(".json") and f != os.path.basename(self._history_file): + archives.append(os.path.join(self._history_dir, f)) + archives.sort() + return archives + + archives = await self.hass.async_add_executor_job(find_archives) + if len(archives) > MAX_ARCHIVE_FILES: + for old_file in archives[:-MAX_ARCHIVE_FILES]: + await self.hass.async_add_executor_job(os.remove, old_file) + _LOGGER.debug("Removed old archive: %s", old_file) + except Exception as e: + _LOGGER.warning("Archive cleanup error: %s", e) + async def _migrate_history_from_txt_to_json(self) -> None: """Migrate old .txt history to .json format.""" try: @@ -424,27 +421,27 @@ class HistoryManager: _LOGGER.error("Error getting history: %s", e) return [] - def get_limited_history(self) -> Dict[str, Any]: - """Get limited conversation history showing only last Q&A.""" - limited_history = [] + def get_limited_history(self, max_display: int = 5) -> Dict[str, Any]: + """Get limited conversation history for sensor attributes. - if self._conversation_history: - last_entry = self._conversation_history[-1] - limited_entry = { - "timestamp": last_entry["timestamp"], - "question": self._truncate_text(last_entry["question"], 4096), - "response": self._truncate_text(last_entry["response"], 4096), + Returns last `max_display` entries with truncated text for HA state. + """ + recent = self._conversation_history[-max_display:] + limited_history = [ + { + "timestamp": entry["timestamp"], + "question": self._truncate_text(entry["question"], 4096), + "response": self._truncate_text(entry["response"], 4096), } - limited_history.append(limited_entry) - - history_info = { - "total_entries": len(self._conversation_history), - "displayed_entries": len(limited_history), - } + for entry in recent + ] return { "entries": limited_history, - "info": history_info, + "info": { + "total_entries": len(self._conversation_history), + "displayed_entries": len(limited_history), + }, } @staticmethod diff --git a/custom_components/ha_text_ai/metrics.py b/custom_components/ha_text_ai/metrics.py index 2b88bce..5f9795e 100644 --- a/custom_components/ha_text_ai/metrics.py +++ b/custom_components/ha_text_ai/metrics.py @@ -123,10 +123,13 @@ class MetricsManager: await self._save_metrics() error_msg = str(error) - # Strip URLs, API keys, and query parameters from error messages + # Strip URLs, API keys, tokens, and query parameters from error messages error_msg = re.sub(r'https?://\S+', '[URL]', error_msg) error_msg = re.sub(r'[?&]key=[^\s&]+', '?key=***', error_msg) error_msg = re.sub(r'AIza[A-Za-z0-9_-]+', '***', error_msg) + error_msg = re.sub(r'Bearer\s+\S+', 'Bearer ***', error_msg) + error_msg = re.sub(r'sk-[A-Za-z0-9_-]{20,}', '***', error_msg) + error_msg = re.sub(r'x-api-key:\s*\S+', 'x-api-key: ***', error_msg, flags=re.IGNORECASE) if len(error_msg) > 256: error_msg = error_msg[:256] + "..." diff --git a/custom_components/ha_text_ai/providers.py b/custom_components/ha_text_ai/providers.py index e6e70aa..d8f80eb 100644 --- a/custom_components/ha_text_ai/providers.py +++ b/custom_components/ha_text_ai/providers.py @@ -9,6 +9,8 @@ across __init__.py, config_flow.py, and api_client.py. @github: https://github.com/smkrv/ha-text-ai @source: https://github.com/smkrv/ha-text-ai """ +from __future__ import annotations + from typing import Any from .const import ( diff --git a/custom_components/ha_text_ai/sensor.py b/custom_components/ha_text_ai/sensor.py index 1222b89..d15e897 100644 --- a/custom_components/ha_text_ai/sensor.py +++ b/custom_components/ha_text_ai/sensor.py @@ -6,6 +6,8 @@ Sensor platform for HA Text AI. @github: https://github.com/smkrv/ha-text-ai @source: https://github.com/smkrv/ha-text-ai """ +from __future__ import annotations + import logging import math from typing import Any, Dict @@ -42,7 +44,6 @@ from .const import ( ATTR_API_PROVIDER, ATTR_MODEL, ATTR_SYSTEM_PROMPT, - ATTR_API_STATUS, ATTR_RESPONSE, ATTR_QUESTION, ATTR_CONVERSATION_HISTORY, @@ -126,9 +127,10 @@ class HATextAISensor(CoordinatorEntity, SensorEntity): self._conversation_history = [] self._system_prompt = None - self._attr_name = f"HA Text AI {self._instance_name}" + self._attr_has_entity_name = True + self._attr_name = self._instance_name self.entity_id = f"sensor.ha_text_ai_{self._normalized_name}" - self._attr_unique_id = f"{config_entry.entry_id}" + self._attr_unique_id = config_entry.entry_id _LOGGER.debug("Created sensor with entity_id: %s", self.entity_id) _LOGGER.debug("Sensor name: %s", self._attr_name) @@ -238,7 +240,6 @@ class HATextAISensor(CoordinatorEntity, SensorEntity): attributes = { ATTR_MODEL: self._config_entry.data.get(CONF_MODEL, "Unknown"), ATTR_API_PROVIDER: self._config_entry.data.get(CONF_API_PROVIDER, "Unknown"), - ATTR_API_STATUS: self._current_state, ATTR_TOTAL_ERRORS: metrics.get("total_errors", 0), "instance_name": self._instance_name, "normalized_name": self._normalized_name, diff --git a/custom_components/ha_text_ai/strings.json b/custom_components/ha_text_ai/strings.json index ff62875..a5e06ff 100644 --- a/custom_components/ha_text_ai/strings.json +++ b/custom_components/ha_text_ai/strings.json @@ -42,6 +42,7 @@ "name_exists": "An instance with this name already exists", "invalid_name": "Invalid instance name", "invalid_auth": "Authentication failed - check your API key", + "api_key_required": "API key is required when changing provider or endpoint", "invalid_api_key": "Invalid API key - please verify your credentials", "cannot_connect": "Failed to connect to API service", "invalid_model": "Selected model is not available", @@ -55,7 +56,6 @@ "invalid_instance": "Invalid instance specified", "unknown": "Unexpected error occurred", "empty": "Name cannot be empty", - "invalid_characters": "Name can only contain letters, numbers, spaces, underscores and hyphens", "name_too_long": "Name must be 50 characters or less" }, "abort": { @@ -208,8 +208,7 @@ "rate_limited": "Rate Limited", "maintenance": "Maintenance", "initializing": "Initializing", - "retrying": "Retrying", - "queued": "Queued" + "retrying": "Retrying" }, "state_attributes": { "question": { @@ -301,6 +300,24 @@ }, "min_latency": { "name": "Minimum Latency" + }, + "last_model": { + "name": "Last Used Model" + }, + "last_timestamp": { + "name": "Last Response Time" + }, + "instance_name": { + "name": "Instance Name" + }, + "normalized_name": { + "name": "Normalized Name" + }, + "last_error": { + "name": "Last Error" + }, + "conversation_history": { + "name": "Conversation History" } } } diff --git a/custom_components/ha_text_ai/translations/en.json b/custom_components/ha_text_ai/translations/en.json index ff62875..a5e06ff 100644 --- a/custom_components/ha_text_ai/translations/en.json +++ b/custom_components/ha_text_ai/translations/en.json @@ -42,6 +42,7 @@ "name_exists": "An instance with this name already exists", "invalid_name": "Invalid instance name", "invalid_auth": "Authentication failed - check your API key", + "api_key_required": "API key is required when changing provider or endpoint", "invalid_api_key": "Invalid API key - please verify your credentials", "cannot_connect": "Failed to connect to API service", "invalid_model": "Selected model is not available", @@ -55,7 +56,6 @@ "invalid_instance": "Invalid instance specified", "unknown": "Unexpected error occurred", "empty": "Name cannot be empty", - "invalid_characters": "Name can only contain letters, numbers, spaces, underscores and hyphens", "name_too_long": "Name must be 50 characters or less" }, "abort": { @@ -208,8 +208,7 @@ "rate_limited": "Rate Limited", "maintenance": "Maintenance", "initializing": "Initializing", - "retrying": "Retrying", - "queued": "Queued" + "retrying": "Retrying" }, "state_attributes": { "question": { @@ -301,6 +300,24 @@ }, "min_latency": { "name": "Minimum Latency" + }, + "last_model": { + "name": "Last Used Model" + }, + "last_timestamp": { + "name": "Last Response Time" + }, + "instance_name": { + "name": "Instance Name" + }, + "normalized_name": { + "name": "Normalized Name" + }, + "last_error": { + "name": "Last Error" + }, + "conversation_history": { + "name": "Conversation History" } } } diff --git a/custom_components/ha_text_ai/translations/ru.json b/custom_components/ha_text_ai/translations/ru.json index aa48c64..c57b18a 100644 --- a/custom_components/ha_text_ai/translations/ru.json +++ b/custom_components/ha_text_ai/translations/ru.json @@ -42,6 +42,7 @@ "name_exists": "Экземпляр с таким именем уже существует", "invalid_name": "Недопустимое имя экземпляра", "invalid_auth": "Ошибка аутентификации - проверьте API-ключ", + "api_key_required": "Необходимо ввести API-ключ при смене провайдера или эндпоинта", "invalid_api_key": "Недопустимый API-ключ - пожалуйста, проверьте учетные данные", "cannot_connect": "Не удалось подключиться к сервису API", "invalid_model": "Выбранная модель недоступна", @@ -55,7 +56,6 @@ "invalid_instance": "Указан некорректный экземпляр", "unknown": "Произошла непредвиденная ошибка", "empty": "Имя не может быть пустым", - "invalid_characters": "Имя может содержать только буквы, цифры, пробелы, подчеркивания и дефисы", "name_too_long": "Имя должно быть не длиннее 50 символов" }, "abort": { @@ -208,8 +208,7 @@ "rate_limited": "Лимит запросов", "maintenance": "Техническое обслуживание", "initializing": "Инициализация", - "retrying": "Повторная попытка", - "queued": "В очереди" + "retrying": "Повторная попытка" }, "state_attributes": { "question": { @@ -301,6 +300,24 @@ }, "min_latency": { "name": "Минимальная задержка" + }, + "last_model": { + "name": "Последняя использованная модель" + }, + "last_timestamp": { + "name": "Время последнего ответа" + }, + "instance_name": { + "name": "Имя экземпляра" + }, + "normalized_name": { + "name": "Нормализованное имя" + }, + "last_error": { + "name": "Последняя ошибка" + }, + "conversation_history": { + "name": "История разговоров" } } } diff --git a/custom_components/ha_text_ai/utils.py b/custom_components/ha_text_ai/utils.py index 93cca7d..cdb38c4 100644 --- a/custom_components/ha_text_ai/utils.py +++ b/custom_components/ha_text_ai/utils.py @@ -6,6 +6,8 @@ Utility functions for HA Text AI integration. @github: https://github.com/smkrv/ha-text-ai @source: https://github.com/smkrv/ha-text-ai """ +from __future__ import annotations + import ipaddress import socket from typing import Any