mirror of
https://github.com/smkrv/ha-text-ai.git
synced 2026-07-21 22:54:00 +08:00
docs: Add v2.4.0 comprehensive fix plan spec
Full refactoring plan covering 48 issues from 4 parallel reviews (code, security, architecture, UI/UX). 6 phases + final review + tests.
This commit is contained in:
@@ -0,0 +1,851 @@
|
|||||||
|
# ha-text-ai v2.4.0 — Полный план исправлений
|
||||||
|
|
||||||
|
**Дата**: 2026-03-12
|
||||||
|
**Версия**: 2.3.0 → 2.4.0
|
||||||
|
**Принцип**: Без breaking changes. Один релиз.
|
||||||
|
**Статус**: В работе
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Оглавление
|
||||||
|
|
||||||
|
- [Принципы обратной совместимости](#принципы-обратной-совместимости)
|
||||||
|
- [Workflow разработки](#workflow-разработки)
|
||||||
|
- [Фаза 0: Подготовка инфраструктуры](#фаза-0-подготовка-инфраструктуры)
|
||||||
|
- [Фаза 1: Security Hardening](#фаза-1-security-hardening)
|
||||||
|
- [Фаза 2: Critical Bug Fixes](#фаза-2-critical-bug-fixes)
|
||||||
|
- [Фаза 3: Concurrency & State](#фаза-3-concurrency--state)
|
||||||
|
- [Фаза 4: Dead Code & Deduplication](#фаза-4-dead-code--deduplication)
|
||||||
|
- [Фаза 5: Coordinator Refactoring](#фаза-5-coordinator-refactoring)
|
||||||
|
- [Фаза 6: UI/UX & Translations](#фаза-6-uiux--translations)
|
||||||
|
- [Финал: Full Review + Tests + CI/CD](#финал-full-review--tests--cicd)
|
||||||
|
- [Трекинг прогресса](#трекинг-прогресса)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Принципы обратной совместимости
|
||||||
|
|
||||||
|
| Что | Правило |
|
||||||
|
|-----|---------|
|
||||||
|
| Sensor attributes | Все существующие сохраняются. Новые добавляются. Избыточные остаются для совместимости |
|
||||||
|
| Service response format | Структура ответа `ask_question`, `get_history` не меняется |
|
||||||
|
| Service schema | Поля не удаляются, не переименовываются. Новые — только `vol.Optional` |
|
||||||
|
| Entity ID | Формат `sensor.ha_text_ai_{name}` не меняется |
|
||||||
|
| Config entry data | Добавление новых ключей OK. Существующие не удаляются. Миграция прозрачная |
|
||||||
|
| History file format | JSON-структура сохраняется. Новые поля — опциональные |
|
||||||
|
| `hass.data[DOMAIN]` | Ключи по `entry_id` — не менять |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Workflow разработки
|
||||||
|
|
||||||
|
```
|
||||||
|
Для каждой фазы:
|
||||||
|
1. Реализация фикса
|
||||||
|
2. 4 параллельных review-агента (Code, Security, Architecture, UI/UX)
|
||||||
|
3. Документация
|
||||||
|
4. Исправление замечаний review
|
||||||
|
5. Повторная проверка (4 агента)
|
||||||
|
6. Документация
|
||||||
|
7. Локальный коммит
|
||||||
|
|
||||||
|
После ВСЕХ фаз:
|
||||||
|
1. Полный финальный review (все 4 агента)
|
||||||
|
2. Добавление тестов + CI/CD workflow
|
||||||
|
3. Финальный коммит
|
||||||
|
4. Push
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Фаза 0: Подготовка инфраструктуры
|
||||||
|
|
||||||
|
**Цель**: Создать фундамент для остальных фаз без изменения поведения.
|
||||||
|
|
||||||
|
### 0.1. Создать `utils.py`
|
||||||
|
|
||||||
|
**Файл**: `custom_components/ha_text_ai/utils.py` (новый)
|
||||||
|
|
||||||
|
Извлечь из `config_flow.py`:
|
||||||
|
- Функцию `normalize_name()` (config_flow.py:351-370)
|
||||||
|
- Вспомогательную функцию `get_file_hash()` из `__init__.py:108-114`
|
||||||
|
- Хелпер для безопасного логирования (фильтрация API-ключей)
|
||||||
|
|
||||||
|
```python
|
||||||
|
def safe_log_data(data: dict, sensitive_keys: tuple = (CONF_API_KEY,)) -> dict:
|
||||||
|
"""Filter sensitive keys from data for safe logging."""
|
||||||
|
return {k: "***" if k in sensitive_keys else v for k, v in data.items()}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Обратная совместимость**: `config_flow.py` сохраняет `normalize_name` как re-export:
|
||||||
|
```python
|
||||||
|
from .utils import normalize_name # backward compat
|
||||||
|
```
|
||||||
|
Это позволит `coordinator.py:29` (`from .config_flow import normalize_name`) продолжать работать, пока мы не обновим все импорты.
|
||||||
|
|
||||||
|
### 0.2. Создать `providers.py`
|
||||||
|
|
||||||
|
**Файл**: `custom_components/ha_text_ai/providers.py` (новый)
|
||||||
|
|
||||||
|
Единый реестр провайдеров вместо дублирования dispatch-таблиц в 4+ местах:
|
||||||
|
|
||||||
|
```python
|
||||||
|
PROVIDER_REGISTRY = {
|
||||||
|
API_PROVIDER_OPENAI: {
|
||||||
|
"default_model": DEFAULT_MODEL_OPENAI,
|
||||||
|
"default_endpoint": DEFAULT_ENDPOINT_OPENAI,
|
||||||
|
"auth_header": "Authorization",
|
||||||
|
"auth_prefix": "Bearer ",
|
||||||
|
"check_path": "/models",
|
||||||
|
},
|
||||||
|
API_PROVIDER_ANTHROPIC: { ... },
|
||||||
|
API_PROVIDER_DEEPSEEK: { ... },
|
||||||
|
API_PROVIDER_GEMINI: { ... },
|
||||||
|
}
|
||||||
|
|
||||||
|
def get_provider_config(provider: str) -> dict: ...
|
||||||
|
def get_default_endpoint(provider: str) -> str: ...
|
||||||
|
def get_default_model(provider: str) -> str: ...
|
||||||
|
def build_auth_headers(provider: str, api_key: str) -> dict: ...
|
||||||
|
```
|
||||||
|
|
||||||
|
### 0.3. Обновить импорты
|
||||||
|
|
||||||
|
Во всех файлах заменить:
|
||||||
|
- `from .config_flow import normalize_name` → `from .utils import normalize_name`
|
||||||
|
- Inline dispatch-таблицы → вызовы `providers.py`
|
||||||
|
|
||||||
|
**Коммит**: `refactor: Extract utils.py and providers.py for code deduplication`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Фаза 1: Security Hardening
|
||||||
|
|
||||||
|
**Цель**: Устранить все уязвимости безопасности.
|
||||||
|
|
||||||
|
### 1.1. Убрать логирование API-ключей [CRITICAL]
|
||||||
|
|
||||||
|
**Файлы и строки**:
|
||||||
|
- `__init__.py:312` — `_LOGGER.debug(f"Setting up HA Text AI entry: {entry.data}")`
|
||||||
|
- `config_flow.py:160` — `_LOGGER.debug(f"Provider step input data: {user_input}")`
|
||||||
|
- `config_flow.py:465` — `_LOGGER.debug(f"Creating config entry with data: {entry_data}")`
|
||||||
|
- `sensor.py:116` — `_LOGGER.debug(f"Initializing sensor with config entry: {config_entry.data}")`
|
||||||
|
|
||||||
|
**Исправление**: Заменить на `safe_log_data()` из `utils.py`:
|
||||||
|
```python
|
||||||
|
_LOGGER.debug("Setting up HA Text AI entry: %s", safe_log_data(entry.data))
|
||||||
|
```
|
||||||
|
|
||||||
|
Заодно перевести все `_LOGGER` вызовы с f-string на `%s`-форматирование (lazy evaluation).
|
||||||
|
|
||||||
|
**Масштаб**: Все файлы, все `_LOGGER.debug(f"...")` → `_LOGGER.debug("...", arg)`.
|
||||||
|
|
||||||
|
### 1.2. Валидация endpoint — HTTPS-only + защита от SSRF [HIGH]
|
||||||
|
|
||||||
|
**Файлы**: `utils.py` (новая функция), `config_flow.py`, `__init__.py`
|
||||||
|
|
||||||
|
Добавить в `utils.py`:
|
||||||
|
```python
|
||||||
|
from urllib.parse import urlparse
|
||||||
|
import ipaddress
|
||||||
|
|
||||||
|
def validate_endpoint(url: str) -> tuple[bool, str]:
|
||||||
|
"""Validate API endpoint URL. Returns (is_valid, error_message)."""
|
||||||
|
parsed = urlparse(url)
|
||||||
|
if parsed.scheme not in ("https",):
|
||||||
|
return False, "only_https_supported"
|
||||||
|
hostname = parsed.hostname
|
||||||
|
if not hostname:
|
||||||
|
return False, "invalid_url"
|
||||||
|
try:
|
||||||
|
ip = ipaddress.ip_address(hostname)
|
||||||
|
if ip.is_private or ip.is_loopback or ip.is_link_local:
|
||||||
|
return False, "private_ip_not_allowed"
|
||||||
|
except ValueError:
|
||||||
|
pass # hostname is a domain name, not IP — OK
|
||||||
|
return True, ""
|
||||||
|
```
|
||||||
|
|
||||||
|
Применить в:
|
||||||
|
- `config_flow.py:async_step_provider` — при вводе endpoint
|
||||||
|
- `config_flow.py:OptionsFlowHandler` — при смене endpoint
|
||||||
|
- `__init__.py:async_setup_entry` — перед созданием API client
|
||||||
|
|
||||||
|
**Обратная совместимость**: Существующие записи с HTTP endpoints продолжат работать (валидация только при создании/изменении). Добавить warning в лог при загрузке существующего HTTP endpoint.
|
||||||
|
|
||||||
|
### 1.3. Не показывать API-ключ в формах [HIGH]
|
||||||
|
|
||||||
|
**Файлы**: `config_flow.py:648-650, 224, 286, 329`
|
||||||
|
|
||||||
|
**Исправление**:
|
||||||
|
- В `_get_settings_schema` и во всех error re-display формах: НЕ передавать `default=data.get(CONF_API_KEY)`.
|
||||||
|
- Вместо `vol.Required(CONF_API_KEY, default=...)` использовать `vol.Optional(CONF_API_KEY, description={"suggested_value": ""})`.
|
||||||
|
- Если поле пустое — сохранять существующий ключ.
|
||||||
|
|
||||||
|
### 1.4. Требовать повторный ввод ключа при смене endpoint [HIGH]
|
||||||
|
|
||||||
|
**Файл**: `config_flow.py:OptionsFlowHandler.async_step_settings`
|
||||||
|
|
||||||
|
При обнаружении изменения endpoint:
|
||||||
|
- Очистить поле API key
|
||||||
|
- Показать предупреждение в описании поля
|
||||||
|
- Не отправлять старый ключ на новый endpoint при валидации
|
||||||
|
|
||||||
|
### 1.5. Убрать history_path из атрибутов сенсора [MEDIUM]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:_get_limited_history`
|
||||||
|
|
||||||
|
Удалить `"history_path": self._history_file` из возвращаемого dict. Это внутренний путь FS.
|
||||||
|
|
||||||
|
### 1.6. Санитизировать ошибки в service response [MEDIUM]
|
||||||
|
|
||||||
|
**Файл**: `__init__.py:161`
|
||||||
|
|
||||||
|
Заменить `"error": str(err)` на `"error": "Request failed"`. Полную ошибку логировать через `_LOGGER.error`.
|
||||||
|
|
||||||
|
### 1.7. Добавить лимит длины для question и json_schema [LOW]
|
||||||
|
|
||||||
|
**Файл**: `__init__.py:73,80`
|
||||||
|
|
||||||
|
```python
|
||||||
|
vol.Required("question"): vol.All(cv.string, vol.Length(max=100000)),
|
||||||
|
vol.Optional("json_schema"): vol.All(cv.string, vol.Length(max=50000)),
|
||||||
|
```
|
||||||
|
|
||||||
|
### 1.8. Валидация Gemini API key [LOW]
|
||||||
|
|
||||||
|
**Файл**: `config_flow.py:263-278`, `__init__.py:279-285`
|
||||||
|
|
||||||
|
Для Gemini: выполнять реальный API-вызов `models.list()` вместо проверки "непустая строка".
|
||||||
|
|
||||||
|
**Коммит**: `fix: Security hardening — credential protection, SSRF prevention, input validation`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Фаза 2: Critical Bug Fixes
|
||||||
|
|
||||||
|
**Цель**: Исправить баги, приводящие к потере данных и некорректному поведению.
|
||||||
|
|
||||||
|
### 2.1. Исправить инициализацию координатора [CRITICAL]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:177-186`
|
||||||
|
|
||||||
|
**Было**: 5 fire-and-forget `async_create_task` в `__init__` + `_history_file` назначается после.
|
||||||
|
|
||||||
|
**Станет**:
|
||||||
|
1. Убрать все `async_create_task` из `__init__`
|
||||||
|
2. Переместить `self._history_file` назначение ПЕРЕД задачами
|
||||||
|
3. Создать метод `async_initialize()`:
|
||||||
|
```python
|
||||||
|
async def async_initialize(self) -> None:
|
||||||
|
"""Initialize coordinator: directories, history, metrics. Must be awaited."""
|
||||||
|
await self._create_history_dir()
|
||||||
|
await self._check_history_directory()
|
||||||
|
await self._initialize_metrics()
|
||||||
|
await self.async_initialize_history_file()
|
||||||
|
await self._migrate_history_from_txt_to_json()
|
||||||
|
```
|
||||||
|
4. В `__init__.py:async_setup_entry` вызывать `await coordinator.async_initialize()` после конструктора.
|
||||||
|
|
||||||
|
### 2.2. Удалить деструктивный цикл `async_set` [CRITICAL]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:778-781`
|
||||||
|
|
||||||
|
Удалить полностью цикл:
|
||||||
|
```python
|
||||||
|
for entity_id in self.hass.states.async_entity_ids():
|
||||||
|
if entity_id.startswith(entity_id_base):
|
||||||
|
self.hass.states.async_set(entity_id, self._get_current_state())
|
||||||
|
```
|
||||||
|
|
||||||
|
`async_request_refresh()` на строке 775 уже обновляет состояние через координатор → `_handle_coordinator_update` → `async_write_ha_state()`.
|
||||||
|
|
||||||
|
### 2.3. Исправить `temperature=0` [HIGH]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:870-873`
|
||||||
|
|
||||||
|
Заменить:
|
||||||
|
```python
|
||||||
|
temp_temperature = temperature or self.temperature
|
||||||
|
temp_max_tokens = max_tokens or self.max_tokens
|
||||||
|
```
|
||||||
|
На:
|
||||||
|
```python
|
||||||
|
temp_temperature = temperature if temperature is not None else self.temperature
|
||||||
|
temp_max_tokens = max_tokens if max_tokens is not None else self.max_tokens
|
||||||
|
temp_context = context_messages if context_messages is not None else self.context_messages
|
||||||
|
temp_system = system_prompt if system_prompt is not None else self._system_prompt
|
||||||
|
```
|
||||||
|
|
||||||
|
**Файл**: `__init__.py:76`
|
||||||
|
|
||||||
|
Заменить `cv.positive_float` на `vol.All(vol.Coerce(float), vol.Range(min=0.0, max=2.0))` для temperature.
|
||||||
|
|
||||||
|
### 2.4. Исправить retry loop — не ретраить нетранзиентные ошибки [HIGH]
|
||||||
|
|
||||||
|
**Файл**: `api_client.py:86-122`
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Было: except Exception ловит HomeAssistantError (4xx ответы) и ретраит
|
||||||
|
# Станет:
|
||||||
|
for attempt in range(self.retry_count):
|
||||||
|
try:
|
||||||
|
async with self.session.post(...) as response:
|
||||||
|
if response.ok:
|
||||||
|
return await response.json()
|
||||||
|
error_data = {}
|
||||||
|
try:
|
||||||
|
error_data = await response.json()
|
||||||
|
except Exception:
|
||||||
|
error_data = {"raw": await response.text()}
|
||||||
|
if response.status == 429: # rate limit — retry
|
||||||
|
await asyncio.sleep(attempt + 1)
|
||||||
|
continue
|
||||||
|
if response.status >= 400: # client/server error — don't retry
|
||||||
|
raise HomeAssistantError(f"API error: {response.status}")
|
||||||
|
except (asyncio.TimeoutError, aiohttp.ClientError) as err:
|
||||||
|
if attempt == self.retry_count - 1:
|
||||||
|
raise HomeAssistantError(f"Connection failed: {err}") from err
|
||||||
|
await asyncio.sleep(attempt + 1)
|
||||||
|
```
|
||||||
|
|
||||||
|
### 2.5. Починить ротацию истории [HIGH]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:588-592`
|
||||||
|
|
||||||
|
`_rotate_history_files` — `async def`, но вызывается через `async_add_executor_job` (запускает sync в потоке → получает coroutine, не результат).
|
||||||
|
|
||||||
|
**Исправление**: Заменить:
|
||||||
|
```python
|
||||||
|
await self.hass.async_add_executor_job(self._rotate_history_files)
|
||||||
|
```
|
||||||
|
На:
|
||||||
|
```python
|
||||||
|
await self._rotate_history_files()
|
||||||
|
```
|
||||||
|
|
||||||
|
Или сделать `_rotate_history_files` синхронной (предпочтительнее — она делает файловые операции).
|
||||||
|
|
||||||
|
### 2.6. Исправить `_is_rate_limited` — сбрасывать после успеха [HIGH]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py`
|
||||||
|
|
||||||
|
В `async_process_message` после успешного API-вызова:
|
||||||
|
```python
|
||||||
|
self._is_rate_limited = False
|
||||||
|
self.endpoint_status = "ready"
|
||||||
|
self._error_count = 0
|
||||||
|
```
|
||||||
|
|
||||||
|
### 2.7. `ConnectionError` не должен ставить `is_rate_limited` [HIGH]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:361-364`
|
||||||
|
|
||||||
|
Убрать `"is_rate_limited": True` из маппинга `ConnectionError`.
|
||||||
|
|
||||||
|
### 2.8. Исправить `async_ask_question` — propagation ошибок [MEDIUM]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:815-849`
|
||||||
|
|
||||||
|
`async_ask_question` — бессмысленная обёртка над `async_process_question`. Убрать лишний уровень индирекции, или добавить полезную логику (валидацию входных параметров).
|
||||||
|
|
||||||
|
**Файл**: `__init__.py:148-162`
|
||||||
|
|
||||||
|
Сервис `ask_question` глотает все ошибки. Добавить `raise HomeAssistantError` для критических ошибок, сохраняя `success: False` в response data для совместимости:
|
||||||
|
```python
|
||||||
|
except Exception as err:
|
||||||
|
_LOGGER.error("ask_question failed: %s", err)
|
||||||
|
response = {"success": False, "error": "Request failed", ...}
|
||||||
|
# НЕ raise — для обратной совместимости. Ошибка в response data.
|
||||||
|
return response
|
||||||
|
```
|
||||||
|
|
||||||
|
**Коммит**: `fix: Critical bugs — init race condition, data loss, temperature=0, retry logic, history rotation`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Фаза 3: Concurrency & State
|
||||||
|
|
||||||
|
**Цель**: Устранить race conditions и проблемы управления состоянием.
|
||||||
|
|
||||||
|
### 3.1. Добавить `asyncio.Lock` для конкурентных запросов [MEDIUM]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:__init__`
|
||||||
|
|
||||||
|
```python
|
||||||
|
self._request_lock = asyncio.Lock()
|
||||||
|
```
|
||||||
|
|
||||||
|
В `async_process_question`:
|
||||||
|
```python
|
||||||
|
async with self._request_lock:
|
||||||
|
# ... existing logic
|
||||||
|
```
|
||||||
|
|
||||||
|
### 3.2. Исправить Semaphore — убрать или сделать instance-level [MEDIUM]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:668`
|
||||||
|
|
||||||
|
Убрать `async with asyncio.Semaphore(1):` — `DataUpdateCoordinator` уже сериализует `_async_update_data`.
|
||||||
|
|
||||||
|
### 3.3. Исправить `async_shutdown` — правильный ключ [HIGH]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:1159`
|
||||||
|
|
||||||
|
Заменить `self.hass.data[DOMAIN].pop(self.instance_name, None)` на:
|
||||||
|
```python
|
||||||
|
# Cleanup handled by async_unload_entry in __init__.py
|
||||||
|
# Remove this line entirely — it uses wrong key (instance_name vs entry_id)
|
||||||
|
```
|
||||||
|
|
||||||
|
### 3.4. Исправить double timeout в `_make_request` [MEDIUM]
|
||||||
|
|
||||||
|
**Файл**: `api_client.py:98-103`
|
||||||
|
|
||||||
|
Убрать `async with timeout(self.api_timeout):` — `aiohttp ClientTimeout` достаточно. Убрать `from async_timeout import timeout`.
|
||||||
|
|
||||||
|
### 3.5. Кэшировать Gemini client [MEDIUM]
|
||||||
|
|
||||||
|
**Файл**: `api_client.py`
|
||||||
|
|
||||||
|
```python
|
||||||
|
def __init__(self, ...):
|
||||||
|
...
|
||||||
|
self._gemini_client = None
|
||||||
|
|
||||||
|
def _get_gemini_client(self, api_key):
|
||||||
|
if self._gemini_client is None:
|
||||||
|
self._gemini_client = genai.Client(api_key=api_key, ...)
|
||||||
|
return self._gemini_client
|
||||||
|
```
|
||||||
|
|
||||||
|
### 3.6. Исправить `get_history` — не мутировать оригинальные записи [MEDIUM]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:1118-1125`
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Было: entry["metadata"] = {...} # мутирует оригинал
|
||||||
|
# Станет:
|
||||||
|
import copy
|
||||||
|
enriched = copy.deepcopy(entry)
|
||||||
|
enriched["metadata"] = {...}
|
||||||
|
```
|
||||||
|
|
||||||
|
### 3.7. Персистировать system_prompt [LOW]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py`
|
||||||
|
|
||||||
|
Сохранять `_system_prompt` в файл `.storage/ha_text_ai_system_prompt_{name}.txt`. Загружать при инициализации. При reload/restart — восстанавливать.
|
||||||
|
|
||||||
|
### 3.8. Исправить `os.rename` в async контексте [MEDIUM]
|
||||||
|
|
||||||
|
**Файл**: `coordinator.py:1047`
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Было: os.rename(old_history_file, backup_file)
|
||||||
|
# Станет:
|
||||||
|
await self.hass.async_add_executor_job(os.rename, old_history_file, backup_file)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Коммит**: `fix: Concurrency — request locking, state management, Gemini client caching, system prompt persistence`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Фаза 4: Dead Code & Deduplication
|
||||||
|
|
||||||
|
**Цель**: Удалить мёртвый код и устранить дублирование.
|
||||||
|
|
||||||
|
### 4.1. Удалить мёртвый код
|
||||||
|
|
||||||
|
| Файл | Что удалить | Строки |
|
||||||
|
|------|-------------|--------|
|
||||||
|
| `coordinator.py` | `_check_memory_available()` + `import psutil` | 1138-1154, 17 |
|
||||||
|
| `coordinator.py` | `_sync_write_history_entry()` | 535-586 |
|
||||||
|
| `coordinator.py` | `AsyncFileHandler` class (заменить на прямой `aiofiles.open`) | 51-63 |
|
||||||
|
| `api_client.py` | `check_connection()` (всегда broken) | 326-... |
|
||||||
|
| `api_client.py` | `self._closed` flag (никто не проверяет) | в shutdown() |
|
||||||
|
| `api_client.py` | `import HomeAssistant` (не используется) | 16 |
|
||||||
|
| `api_client.py` | `import datetime, timedelta` (не используются) | 14 |
|
||||||
|
| `coordinator.py` | `import re` (не используется) | 18 |
|
||||||
|
| `__init__.py` | `import TypeVar` + `ConfigType` (не нужен) | 16, 69 |
|
||||||
|
| `__init__.py` | `import timedelta` (не используется) | 15 |
|
||||||
|
|
||||||
|
### 4.2. Консолидировать schema определения
|
||||||
|
|
||||||
|
**Файл**: `const.py:188-256`
|
||||||
|
|
||||||
|
Удалить мёртвые schema:
|
||||||
|
- `SERVICE_SCHEMA_ASK_QUESTION` (неиспользуемый, конфликтует с `__init__.py`)
|
||||||
|
- `SERVICE_SCHEMA_SET_SYSTEM_PROMPT` (дублирует `__init__.py`)
|
||||||
|
- `SERVICE_SCHEMA_GET_HISTORY` (дублирует `__init__.py`)
|
||||||
|
- `CONFIG_SCHEMA` (не используется)
|
||||||
|
|
||||||
|
Оставить единственное определение в `__init__.py`.
|
||||||
|
|
||||||
|
### 4.3. Удалить неиспользуемые зависимости из `manifest.json`
|
||||||
|
|
||||||
|
**Файл**: `manifest.json`
|
||||||
|
|
||||||
|
Удалить:
|
||||||
|
- `anthropic>=0.8.0` — не импортируется (API через `aiohttp`)
|
||||||
|
- `openai>=1.12.0` — не импортируется (API через `aiohttp`)
|
||||||
|
- `certifi>=2024.2.2` — уже есть в HA
|
||||||
|
- `async-timeout>=4.0.0` — deprecated, заменяем на `asyncio.timeout`
|
||||||
|
|
||||||
|
Обновить:
|
||||||
|
- `aiohttp>=3.9.4` (security fix: CVE-2024-23334, CVE-2024-23829)
|
||||||
|
|
||||||
|
### 4.4. Использовать `providers.py` вместо inline dispatch
|
||||||
|
|
||||||
|
Заменить во всех файлах inline-таблицы `default_endpoint`/`default_model`/headers:
|
||||||
|
|
||||||
|
**`__init__.py:326-362`**:
|
||||||
|
```python
|
||||||
|
# Было: 4 if/elif для endpoint + 4 if/elif для model + 4 if/elif для headers
|
||||||
|
# Станет:
|
||||||
|
from .providers import get_provider_config, build_auth_headers
|
||||||
|
config = get_provider_config(provider)
|
||||||
|
endpoint = entry_data.get(CONF_API_ENDPOINT) or config["default_endpoint"]
|
||||||
|
headers = build_auth_headers(provider, api_key)
|
||||||
|
```
|
||||||
|
|
||||||
|
**`config_flow.py:104-117, 487-502`**: аналогично.
|
||||||
|
|
||||||
|
### 4.5. Консолидировать `_async_validate_api`
|
||||||
|
|
||||||
|
**Файлы**: `config_flow.py` — два идентичных метода в `HATextAIConfigFlow` (372-407) и `OptionsFlowHandler` (517-549).
|
||||||
|
|
||||||
|
Извлечь в `utils.py`:
|
||||||
|
```python
|
||||||
|
async def validate_api_connection(hass, provider, endpoint, api_key) -> tuple[bool, str]:
|
||||||
|
```
|
||||||
|
|
||||||
|
### 4.6. Унифицировать timeout
|
||||||
|
|
||||||
|
Заменить все `from async_timeout import timeout` на `from asyncio import timeout` (Python 3.12+, HA 2024.12.0 требует).
|
||||||
|
|
||||||
|
**Файлы**: `api_client.py:3`, `__init__.py:19`
|
||||||
|
|
||||||
|
**Коммит**: `refactor: Remove dead code, consolidate schemas, unify provider dispatch, clean dependencies`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Фаза 5: Coordinator Refactoring
|
||||||
|
|
||||||
|
**Цель**: Разбить god object `coordinator.py` (~1160 строк) на модули с чёткими ответственностями.
|
||||||
|
|
||||||
|
### 5.1. Создать `history.py`
|
||||||
|
|
||||||
|
**Файл**: `custom_components/ha_text_ai/history.py` (новый)
|
||||||
|
|
||||||
|
Извлечь из `coordinator.py`:
|
||||||
|
- `_create_history_dir()`
|
||||||
|
- `_check_history_directory()`
|
||||||
|
- `async_initialize_history_file()`
|
||||||
|
- `_migrate_history_from_txt_to_json()`
|
||||||
|
- `_write_history_entry()`
|
||||||
|
- `_rotate_history()` / `_rotate_history_files()`
|
||||||
|
- `async_clear_history()`
|
||||||
|
- `async_get_history()`
|
||||||
|
- `_update_history()`
|
||||||
|
- `_get_limited_history()`
|
||||||
|
- `_conversation_history` list
|
||||||
|
- `_request_lock` (для записи в историю)
|
||||||
|
|
||||||
|
Класс:
|
||||||
|
```python
|
||||||
|
class HistoryManager:
|
||||||
|
def __init__(self, hass, instance_name, normalized_name, history_dir, max_history_size):
|
||||||
|
...
|
||||||
|
async def async_initialize(self): ...
|
||||||
|
async def write_entry(self, entry): ...
|
||||||
|
async def get_history(self, limit, filter_model, ...): ...
|
||||||
|
async def clear(self): ...
|
||||||
|
def get_limited(self, max_entries): ...
|
||||||
|
@property
|
||||||
|
def conversation_history(self) -> list: ...
|
||||||
|
@property
|
||||||
|
def size(self) -> int: ...
|
||||||
|
```
|
||||||
|
|
||||||
|
### 5.2. Создать `metrics.py`
|
||||||
|
|
||||||
|
**Файл**: `custom_components/ha_text_ai/metrics.py` (новый)
|
||||||
|
|
||||||
|
Извлечь из `coordinator.py`:
|
||||||
|
- `_initialize_metrics()`
|
||||||
|
- `_update_metrics()`
|
||||||
|
- `_save_metrics()` / `_load_metrics()`
|
||||||
|
- `_metrics` dict
|
||||||
|
- `_total_tokens`, `_prompt_tokens`, `_completion_tokens`
|
||||||
|
- `_successful_requests`, `_failed_requests`
|
||||||
|
- `_average_latency`, `_max_latency`, `_min_latency`
|
||||||
|
|
||||||
|
Класс:
|
||||||
|
```python
|
||||||
|
class MetricsTracker:
|
||||||
|
def __init__(self, hass, normalized_name, storage_dir):
|
||||||
|
...
|
||||||
|
async def async_initialize(self): ...
|
||||||
|
def update(self, tokens_used, prompt_tokens, completion_tokens, latency): ...
|
||||||
|
async def save(self): ...
|
||||||
|
@property
|
||||||
|
def summary(self) -> dict: ...
|
||||||
|
```
|
||||||
|
|
||||||
|
### 5.3. Упростить `coordinator.py`
|
||||||
|
|
||||||
|
После извлечения `coordinator.py` сократится с ~1160 до ~400-500 строк:
|
||||||
|
|
||||||
|
```python
|
||||||
|
class HATextAICoordinator(DataUpdateCoordinator):
|
||||||
|
def __init__(self, hass, client, config, ...):
|
||||||
|
self.history = HistoryManager(...)
|
||||||
|
self.metrics = MetricsTracker(...)
|
||||||
|
...
|
||||||
|
|
||||||
|
async def async_initialize(self):
|
||||||
|
await self.history.async_initialize()
|
||||||
|
await self.metrics.async_initialize()
|
||||||
|
|
||||||
|
async def async_process_question(self, question, **kwargs):
|
||||||
|
async with self._request_lock:
|
||||||
|
...
|
||||||
|
response = await self.client.create(...)
|
||||||
|
self.metrics.update(...)
|
||||||
|
await self.history.write_entry(...)
|
||||||
|
return response
|
||||||
|
|
||||||
|
async def _async_update_data(self):
|
||||||
|
...
|
||||||
|
```
|
||||||
|
|
||||||
|
### 5.4. Обновить `sensor.py`
|
||||||
|
|
||||||
|
`sensor.py` читает данные из координатора. Обновить доступ:
|
||||||
|
```python
|
||||||
|
# Было: self.coordinator._conversation_history
|
||||||
|
# Станет: self.coordinator.history.conversation_history
|
||||||
|
```
|
||||||
|
|
||||||
|
### 5.5. Добавить лимит архивов истории
|
||||||
|
|
||||||
|
**Файл**: `history.py` (в рамках ротации)
|
||||||
|
|
||||||
|
При ротации — хранить максимум 5 архивных файлов. Удалять самые старые.
|
||||||
|
|
||||||
|
**Коммит**: `refactor: Split coordinator into coordinator, history, and metrics modules`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Фаза 6: UI/UX & Translations
|
||||||
|
|
||||||
|
**Цель**: Исправить проблемы пользовательского интерфейса и переводов.
|
||||||
|
|
||||||
|
### 6.1. Исправить дублированный JSON-ключ `"provider"` [CRITICAL]
|
||||||
|
|
||||||
|
**Файлы**: Все 8 файлов в `translations/`
|
||||||
|
|
||||||
|
В каждом файле под `config.step` есть два ключа `"provider"`. Объединить в один с правильным содержимым (title + data из второго, т.к. первый перезаписывается).
|
||||||
|
|
||||||
|
### 6.2. Добавить `strings.json` [LOW]
|
||||||
|
|
||||||
|
**Файл**: `custom_components/ha_text_ai/strings.json` (новый)
|
||||||
|
|
||||||
|
Скопировать `translations/en.json` → `strings.json` (HA convention: `strings.json` = authoritative English source).
|
||||||
|
|
||||||
|
### 6.3. Извлечь form schema builder для config flow [HIGH]
|
||||||
|
|
||||||
|
**Файл**: `config_flow.py`
|
||||||
|
|
||||||
|
Создать `_build_provider_schema(user_input, provider, errors)` по образцу `_get_settings_schema` из `OptionsFlowHandler`. Использовать единый метод во всех 5 местах re-display формы. Все поля всегда присутствуют.
|
||||||
|
|
||||||
|
### 6.4. Использовать `NumberSelector` для числовых полей [MEDIUM]
|
||||||
|
|
||||||
|
**Файл**: `config_flow.py`
|
||||||
|
|
||||||
|
Для temperature, max_tokens, request_interval, api_timeout, context_messages, max_history_size:
|
||||||
|
|
||||||
|
```python
|
||||||
|
vol.Required(CONF_TEMPERATURE, default=DEFAULT_TEMPERATURE): selector.NumberSelector(
|
||||||
|
selector.NumberSelectorConfig(min=0.0, max=2.0, step=0.1, mode="slider")
|
||||||
|
),
|
||||||
|
```
|
||||||
|
|
||||||
|
### 6.5. Выровнять переводы с кодом
|
||||||
|
|
||||||
|
**Файлы**: Все `translations/*.json`
|
||||||
|
|
||||||
|
| Действие | Что |
|
||||||
|
|----------|-----|
|
||||||
|
| Удалить | Мёртвые error keys: `invalid_characters`, `name_too_long`, `history_storage_error`, `history_rotation_error`, `history_file_access_error`, `invalid_model`, `rate_limit`, `context_length` и др. |
|
||||||
|
| Удалить | Мёртвые state keys: `retrying`, `queued` |
|
||||||
|
| Добавить | Недостающие attribute keys для реальных атрибутов |
|
||||||
|
| Оставить | Все существующие используемые ключи |
|
||||||
|
|
||||||
|
### 6.6. Исправить ошибки в переводах
|
||||||
|
|
||||||
|
| Язык | Проблема | Исправление |
|
||||||
|
|------|----------|-------------|
|
||||||
|
| `it.json` | "Istanze" (мн.ч.) → "Istanza" (ед.ч.) | Заменить все вхождения |
|
||||||
|
| `hi.json` | "instance" = "उदाहरण" (example) | Заменить на "इंस्टैंस" |
|
||||||
|
| `sr.json` | "disconnected" = "Прекључено" (switched) | Заменить на "Искључено" |
|
||||||
|
| `de.json` | Опции селектора не переведены | Перевести: "OpenAI (kompatibel)" и т.д. |
|
||||||
|
|
||||||
|
### 6.7. Прочие UI-исправления
|
||||||
|
|
||||||
|
| # | Что | Файл |
|
||||||
|
|---|-----|------|
|
||||||
|
| A | `PLATFORMS: list[Platform] = [Platform.SENSOR]` | `const.py:20` |
|
||||||
|
| B | `supports_response=SupportsResponse.OPTIONAL` | `__init__.py:203` |
|
||||||
|
| C | `manufacturer="SMKRV"` вместо `"Community"` | `sensor.py:157` |
|
||||||
|
| D | `datetime` selector для `start_date` в services | `services.yaml:146-149` |
|
||||||
|
| E | Выровнять temperature default: 0.1 везде | `services.yaml:63` |
|
||||||
|
| F | Убрать "This service now returns..." из description | `services.yaml:6-7` |
|
||||||
|
| G | Убрать пустые массивы из manifest (`bluetooth`, `mqtt` и т.д.) | `manifest.json` |
|
||||||
|
| H | Убрать нестандартное копирование иконок в `www/` | `__init__.py:228-272` |
|
||||||
|
|
||||||
|
**Пункт H — детали**: Современные HA + HACS используют `icons/` directory напрямую. Удалить `_copy_icon_to_www()` и весь связанный код. Существующие файлы в `www/ha_text_ai/` не трогаем (пользователь может удалить сам).
|
||||||
|
|
||||||
|
### 6.8. Исправить 404 как успешную валидацию API
|
||||||
|
|
||||||
|
**Файлы**: `__init__.py:295`, `config_flow.py:399`
|
||||||
|
|
||||||
|
Только 200 = успех. 404 → `cannot_connect`.
|
||||||
|
|
||||||
|
**Коммит**: `fix: UI/UX — translations, config flow forms, sensor attributes, services.yaml alignment`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Финал: Full Review + Tests + CI/CD
|
||||||
|
|
||||||
|
### F.1. Полный финальный review
|
||||||
|
|
||||||
|
Запуск всех 4 review-агентов на весь codebase:
|
||||||
|
1. Code Review
|
||||||
|
2. Security Review
|
||||||
|
3. Architectural Review
|
||||||
|
4. UI/UX Review
|
||||||
|
|
||||||
|
Исправить все найденные проблемы. Повторить до чистого прохода.
|
||||||
|
|
||||||
|
### F.2. Добавить тесты
|
||||||
|
|
||||||
|
**Файлы**: `tests/` (новая директория)
|
||||||
|
|
||||||
|
Минимальный набор тестов:
|
||||||
|
|
||||||
|
```
|
||||||
|
tests/
|
||||||
|
├── conftest.py # HA test fixtures
|
||||||
|
├── test_config_flow.py # Config flow + options flow
|
||||||
|
├── test_api_client.py # API client per provider
|
||||||
|
├── test_coordinator.py # Coordinator logic
|
||||||
|
├── test_sensor.py # Sensor entity
|
||||||
|
├── test_utils.py # Utility functions
|
||||||
|
├── test_history.py # History manager
|
||||||
|
├── test_metrics.py # Metrics tracker
|
||||||
|
└── test_providers.py # Provider registry
|
||||||
|
```
|
||||||
|
|
||||||
|
Ключевые тесты:
|
||||||
|
- Config flow: создание entry для каждого провайдера, validation errors, options flow
|
||||||
|
- API client: успешный ответ, retry на 429, НЕ retry на 400, timeout
|
||||||
|
- Coordinator: `temperature=0` работает, concurrent requests, history write/read
|
||||||
|
- Security: endpoint validation (HTTPS-only, no private IPs), API key не в логах
|
||||||
|
- History: write, read, rotate, archive cleanup
|
||||||
|
- Translations: все файлы имеют одинаковые ключи
|
||||||
|
|
||||||
|
### F.3. CI/CD workflow для тестов
|
||||||
|
|
||||||
|
**Файл**: `.github/workflows/tests.yaml` (новый)
|
||||||
|
|
||||||
|
```yaml
|
||||||
|
name: Tests
|
||||||
|
on:
|
||||||
|
push:
|
||||||
|
branches: [main, dev]
|
||||||
|
pull_request:
|
||||||
|
branches: [main]
|
||||||
|
jobs:
|
||||||
|
test:
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v4
|
||||||
|
- uses: actions/setup-python@v5
|
||||||
|
with:
|
||||||
|
python-version: '3.12'
|
||||||
|
- name: Install dependencies
|
||||||
|
run: |
|
||||||
|
pip install pytest pytest-asyncio pytest-homeassistant-custom-component
|
||||||
|
pip install -r requirements_test.txt
|
||||||
|
- name: Run tests
|
||||||
|
run: pytest tests/ -v
|
||||||
|
```
|
||||||
|
|
||||||
|
### F.4. Обновить version в manifest.json
|
||||||
|
|
||||||
|
`"version": "2.3.0"` → `"version": "2.4.0"`
|
||||||
|
|
||||||
|
### F.5. Финальные коммиты
|
||||||
|
|
||||||
|
```
|
||||||
|
test: Add comprehensive test suite for all components
|
||||||
|
chore: Add test workflow to CI/CD
|
||||||
|
chore: Bump version to 2.4.0
|
||||||
|
```
|
||||||
|
|
||||||
|
### F.6. Push
|
||||||
|
|
||||||
|
Только после успешного прохождения всех проверок.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Трекинг прогресса
|
||||||
|
|
||||||
|
| Фаза | Статус | Дата начала | Дата завершения | Коммит |
|
||||||
|
|------|--------|-------------|-----------------|--------|
|
||||||
|
| 0. Подготовка | [ ] | | | |
|
||||||
|
| 1. Security | [ ] | | | |
|
||||||
|
| 2. Critical Bugs | [ ] | | | |
|
||||||
|
| 3. Concurrency | [ ] | | | |
|
||||||
|
| 4. Dead Code | [ ] | | | |
|
||||||
|
| 5. Coordinator | [ ] | | | |
|
||||||
|
| 6. UI/UX | [ ] | | | |
|
||||||
|
| F. Final Review | [ ] | | | |
|
||||||
|
| F. Tests | [ ] | | | |
|
||||||
|
| F. CI/CD | [ ] | | | |
|
||||||
|
| F. Push | [ ] | | | |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Новые файлы (итого)
|
||||||
|
|
||||||
|
```
|
||||||
|
custom_components/ha_text_ai/
|
||||||
|
├── utils.py (Phase 0)
|
||||||
|
├── providers.py (Phase 0)
|
||||||
|
├── history.py (Phase 5)
|
||||||
|
├── metrics.py (Phase 5)
|
||||||
|
├── strings.json (Phase 6)
|
||||||
|
tests/
|
||||||
|
├── conftest.py (Final)
|
||||||
|
├── test_*.py (Final)
|
||||||
|
.github/workflows/
|
||||||
|
├── tests.yaml (Final)
|
||||||
|
```
|
||||||
|
|
||||||
|
## Изменяемые файлы (все фазы)
|
||||||
|
|
||||||
|
```
|
||||||
|
custom_components/ha_text_ai/
|
||||||
|
├── __init__.py (Phases 1,2,4,6)
|
||||||
|
├── api_client.py (Phases 2,3,4)
|
||||||
|
├── config_flow.py (Phases 0,1,4,6)
|
||||||
|
├── const.py (Phases 4,6)
|
||||||
|
├── coordinator.py (Phases 1,2,3,4,5)
|
||||||
|
├── sensor.py (Phases 1,5,6)
|
||||||
|
├── manifest.json (Phases 4,6,Final)
|
||||||
|
├── services.yaml (Phase 6)
|
||||||
|
├── translations/*.json (Phase 6)
|
||||||
|
```
|
||||||
Reference in New Issue
Block a user