docs: review document for #42

Issue: #42
User-Visible: no

docs/reviews/SPEC-REVIEW-42-r1.md

@@ -0,0 +1,216 @@
+# SPEC-REVIEW-42-r1
+
+- Issue: #42 «[HP-ENG-01] измеряемое инженерное качество backend»
+- Этап: ТЗ на ревью (PROCESS.md §2.4), полный трек (метки: P2, tests, tech-debt,
+  S4-spec-review; `small` отсутствует)
+- Артефакт ТЗ: `docs/specs/042-backend-engineering-quality.md`, ревизия 2
+  (коммит `582d673a`, docs-only, класс C)
+- Заход: r1 (документов `SPEC-REVIEW-42-*.md` в репозитории не найдено —
+  первый заход подтверждён)
+- Ревьюер: свежая сессия, без контекста написания ТЗ
+
+## Скоуп проверки
+
+ТЗ описывает одну ступень из пяти блоков: tooling-фундамент (pyproject +
+requirements_test), ruff narrow, mypy strict на растущем allowlist,
+coverage-механизм в CI с baseline-файлом, и формализация WS `ERROR_CODES` +
+JSON-details + локализованный fallback (единственная видимая пользователю
+часть). Дифф этого раунда — только сам файл ТЗ (177 добавлено / 58 удалено),
+продуктовый код не тронут.
+
+## Как проверялось
+
+Ревью ТЗ на этом этапе — не код-ревью: гейты `typecheck`/`test`/`build` и
+браузерные смоки к докс-диффу неприменимы (класс C, ноль файлов `src/**` или
+`custom_components/**/*.py` в этом коммите). Вместо этого перепроверялись
+фактические утверждения ТЗ по текущему коду `dev` — то, что обычно и выдаёт
+догадку, поданную как факт:
+
+- `python3 -m ruff check custom_components/houseplan --select E,F,B,I
+  --target-version py313 --statistics` → **333 (E501 291, B023 17, I001 13,
+  F401 6, E731 2, F841 2, B905 2)** — совпадает с цифрами ТЗ дословно;
+- `custom_components/houseplan/quality_scale.yaml` → ровно 4 `todo`:
+  `test-coverage`, `docs-troubleshooting`, `docs-examples`, `strict-typing` —
+  совпадает с заявлением владельца в issue;
+- `docs/USER-GUIDE.md` §22 Troubleshooting существует (строка 1034),
+  `docs/USER-GUIDE.ru.md` раздела «Troubleshooting»/«Устранение неполадок» не
+  содержит вовсе — совпадает;
+- `grep -c '"backup.error\.' src/i18n/en.json` → **26** ключей — совпадает;
+- `.github/workflows/mutation-gate.yml:67` действительно дублирует строку
+  `pip install pytest voluptuous pytest-homeassistant-custom-component
+  home-assistant-frontend` из `validate.yml` — совпадает;
+- прочитан весь путь ошибки на фронте (`src/houseplan-card.ts:9497-9536`
+  `_errText`, `src/houseplan-editor-runtime.ts:8002-8010` `_backupErrorText`)
+  и все точки эмиссии `send_error` (`custom_components/houseplan/
+  websocket_api.py`, 54 вызова) плюс классы с публичным `.code`
+  (`validation.py`, `junction_limits.py`, `import_export.py`) — см. находку
+  Medium 1.
+
+Не запускался: `npm run typecheck/test/build`, `check-docs`, `model-invariants`,
+браузерные смоки, `pytest tests_backend`, mypy/coverage прогон — см. «Чего не
+проверял».
+
+## Находки
+
+### Medium 1 (в скоупе) — AC5 не покрывает коды, эмитируемые не литералом в `send_error(...)`
+
+**Файл:** `docs/specs/042-backend-engineering-quality.md`, раздел «5. WS error
+contract + доки» и «AC5».
+
+**Формулировка ТЗ:** «ERROR_CODES ⊇ все коды send_error (скан исходника):
+каждый литерал `send_error(...)`-кода ∈ ERROR_CODES». Как метод верификации
+это подразумевает статический скан аргументов вызовов `send_error(...)` на
+литеральные строки.
+
+**Почему это не выполнимо как написано.** Как минимум четыре класса ошибок
+несут код не литералом внутри `send_error(...)`, а через `err.code`,
+прочитанный в обработчике (`websocket_api.py:189` `send_error(msg_id,
+err.code, err.message)`; `:1370` и `:1747` `send_error(msg["id"], err.code,
+str(err))`):
+
+- `validation.py:43-54` `OpeningPassageError.code = "invalid_passage_fields"`
+  и `validation.py:63-72`
+  `PartitionOpeningJambMarginError.code = "invalid_partition_opening_jamb_margin"`
+  — это ровно те два кода, вокруг которых построен весь блок 5 (JSON-details).
+  Скан по литералам `send_error(...)` их не найдёт — они читаются из
+  атрибута класса, а не передаются строкой в месте вызова;
+- `validation.py:57-60` `PartitionOpeningHostError.code =
+  "invalid_partition_opening_host"` и `validation.py:81-84`
+  `WallModelClientOutdatedError.code = "wall_model_client_outdated"` — то же;
+- `validation.py:35-40` `MarkerControlError.__init__(self, code, message)` —
+  код передаётся аргументом конструктора; часть литеральна
+  (`"duplicate_marker_control"`, `"marker_control_self"`,
+  `"invalid_value_badge"` и др., `validation.py:824-968`), часть собрана
+  f-строкой из `prefix` с ровно двумя значениями
+  (`validation.py:818`: `"value_badge"` / `"value_source"`) →
+  `value_badge_marker_missing`, `value_source_marker_missing`,
+  `value_badge_marker_not_light`, `value_source_marker_not_light`
+  (`validation.py:847,849`);
+- `junction_limits.py:56` `JunctionLimitError.code = f"junction_limit_{rule}"`
+  — `rule` пробегает конечное множество ключей П1–П4 (`junction_limits.py:427-430`),
+  но опять не литерал внутри `send_error(...)`.
+
+Итого не меньше дюжины уже существующих кодов — включая **оба** кода, ради
+которых написан блок 5 — невидимы для скана, читающего только литералы в
+`send_error(...)`. Реализация AC5 «в лоб» даст зелёный контракт-тест, который
+ничего не доказывает для этих кодов: они не попадут в `ERROR_CODES`, останутся
+без ключа `backup.error.<code>`, и `_errText` продолжит показывать сырой
+`e.message` (`houseplan-card.ts:9528`) ровно для того класса ошибок, который
+issue называет проблемой. Это технический, не продуктовый вопрос (какой метод
+верификации использовать), поэтому решаю его в вердикте, а не выношу
+владельцу.
+
+**Как чинится в скоупе:** AC5/раздел 5 должны явно назвать способ, которым
+скан достаёт коды из `err.code` — либо (а) перечислить классы-источники
+(`OpeningPassageError`, `PartitionOpeningHostError`,
+`PartitionOpeningJambMarginError`, `WallModelClientOutdatedError`,
+`MarkerControlError`, `JunctionLimitError`, `ImportFailure`) и извлекать их
+`code`-литералы/шаблоны статическим разбором модуля, либо (б) явно сузить
+AC5 до кодов-литералов в `send_error(...)` и отдельно перечислить
+раскрытые f-строкой/классом семейства как «покрыты общим локализованным
+fallback по коду, без выделенного ключа» — тогда `_errText`/`_backupErrorText`
+обязаны фактически падать в этот fallback для них, а не в сырой `e.message`
+(что снова упирается в порядок проверок в `_errText`, см. Medium 2).
+
+### Medium 2 (в скоупе) — раздел «i18n» (обязателен по PROCESS.md §7.1) отсутствует; повторное использование `err.code` не зафиксировано
+
+**Файл:** тот же, ТЗ целиком — раздела с заголовком «i18n» нет ни одного.
+
+DoR §2.5 требует «i18n: ключи en + ru перечислены». Блок 5 обещает:
+«неизвестный код → общий локализованный текст + код» — но не говорит,
+это НОВЫЙ ключ или переиспользование существующего.
+
+Проверка кода показывает, что подходящий ключ уже есть и уже переведён:
+`src/i18n/en.json:338` `"err.code": "code {code}"`,
+`src/i18n/ru.json:338` `"код {code}"` (де/фр тоже переведены). Реальная
+причина текущего дефекта не в отсутствии такого ключа, а в порядке проверок
+внутри `_errText` (`houseplan-card.ts:9528`: `if (e.message) return
+e.message;` стоит РАНЬШЕ ветки `err.code`) — значит для любой ошибки, где
+бэкенд шлёт одновременно код и `message`, независимо от известности кода,
+сейчас показывается сырое английское `message`. AC6 («неизвестный код →
+локализованный fallback») не сможет быть проверен юнит-тестом однозначно,
+пока ТЗ не решит: (а) переиспользуется `err.code`/`err.unknown` без новых
+ключей — тогда раздел i18n тривиален («новых ключей нет, порядок проверок в
+`_errText` меняется на code-first») — либо (б) вводится новый текст — тогда
+нужны конкретные en+ru строки. Без явного выбора ревьюер кода не сможет
+сверить AC6 с намерением автора, а автор рискует написать тест под
+собственную догадку, которую я не смогу отличить от решения.
+
+**Как чинится в скоупе:** добавить короткий раздел «i18n» с явным решением
+(рекомендация — вариант (а), ключ уже есть и уже переведён на 4 языка) и
+одной строкой описать смену порядка проверок в `_errText`
+(code-first → message → error → JSON).
+
+## Low (снимаю с записью, не блокирует)
+
+Разделы «UX» и «Модель данных и миграция», формально обязательные по
+PROCESS.md §7.1 как отдельные заголовки, в файле не оформлены как таковые —
+их содержание фактически присутствует, но разбросано («Что человек увидит до
+и после» покрывает UX: новых диалогов/интеракций нет, меняется только текст;
+«DoR-примечания» закрывает миграцию: единственное затронутое поле — формат
+`message` двух кодов, обратная совместимость на одну бету). Контент по
+существу верный и достаточный, поэтому не поднимаю до Medium — прошу
+консолидировать при следующей правке ради дословного соответствия §7.1, но
+это не требует нового цикла ради одного этого пункта.
+
+## Что проверено и корректно
+
+- **Сценарий** и **«что человек увидит до и после»** — на месте, продуктовые,
+  без терминов реализации; соответствуют J-рядам SCOPE.md лишь косвенно
+  (инженерное качество — не отдельная строка core user jobs), но решение
+  вести эту работу и её ценностная оценка (3/10 пользователю, 9/10 разработке)
+  уже приняты владельцем в комментариях issue — не переоткрываю;
+- **Скоуп/не-скоуп** разделены явно, «следующая ступень» зафиксирована
+  текстом, а не памятью;
+- **Контракт поведения**: код и структура успешных ответов не меняются;
+  единственное видимое изменение — формат `message` двух кодов на JSON,
+  риск для рассинхронизированной пары фронт/бэк одной беты назван и обоснован
+  явно, а не спрятан;
+- **AC1–AC4, AC6, AC7** — однозначны, у каждого назван способ доказательства
+  (CI/локально/юнит), и по каждому видно, как тест умеет упасть (AC1 —
+  «подмена baseline на большее число», AC2 — «удаление homeassistant из шага
+  установки», AC4 — «контракт-тест падает при сужении списка», AC6 — м1/м2
+  мутанты в разделе «План автотестов»);
+- **Риски** называют главный поведенческий риск (B023-фиксы) и версийный
+  разъезд (3.10 песочница / 3.13 CI) без сокрытия;
+- **Откат** — `git revert`, без потери данных, явно;
+- **Release-артефакты** — оба changelog, `ARCHITECTURE.md`, `USER-GUIDE.ru`
+  §Troubleshooting — названы;
+- **Принятые предположения** — присутствуют отдельным блоком, включая ту же
+  идею про `backup.error.<code>` как общее пространство кодов, которую
+  находка Medium 2 просит явно продолжить в раздел i18n;
+- порог coverage 90→95 сознательно вынесен в отдельные trivial-issues —
+  разумно, механизм этой ступени уже будет их принудительно проверять.
+
+## Чего не проверял и почему
+
+- `npx tsc --noEmit`, `npm test`, `npm run build` + сверка копий бандла,
+  `node scripts/check-docs.mjs`, `node scripts/model-invariants.mjs` — диф
+  этого раунда состоит из одного файла `docs/specs/042-*.md` (класс C), в
+  `src/**` и `custom_components/**/*.py` изменений нет; прогон гейтов кода
+  на неизменном коде не даёт сигнала по существу ТЗ и не входит в предмет
+  ревью ТЗ (PROCESS.md §2.4 против §2.7);
+- браузерные смоки, `golden:verify`, `pytest tests_backend`, performance —
+  та же причина, плюс на этом этапе AC не привязаны к конкретному коду,
+  который можно было бы прогнать;
+- фактическое покрытие 89.1% и «pure 240/0» из измерений владельца — не
+  переснимал (нужен рабочий HA-harness/`pytest-cov`, которых в песочнице нет
+  и установка которых для разового замера ушла бы за рамки ревью ТЗ). Считаю
+  косвенным подтверждением точное совпадение независимо перепроверенных
+  ruff-цифр (333/291/17/13/6/2/2) — метод измерения автора доверия
+  заслуживает;
+- осуществимость конкретного стартового allowlist mypy strict (`const`,
+  `projection`, `coordinate_canonicalization`, `frontend_asset_manifest`,
+  `junction_limits`, `plans` + часть `validation`/`wall_segment_model`/
+  `geometry_migration`) — ТЗ само откладывает точный финальный список «по
+  факту зелени» в handoff, это законное «assumed, решается в реализации», не
+  спец-пробел.
+
+## Итог
+
+Механизм в целом обоснован и предметен — почти все числа и ссылки в ТЗ
+проверяются на HEAD `dev` дословно, что необычно хорошо для спека такого
+объёма. Возврат — по двум находкам Medium в скоупе задачи: AC5 нужно
+дотянуть до кодов, приходящих через `err.code` (включая оба флагманских кода
+блока 5), и явно решить/записать пункт i18n. Обе решаются добавлением текста
+в тот же файл, без пересмотра архитектуры и без нового цикла владельца.