diff --git a/docs/reviews/SPEC-REVIEW-42-r3.md b/docs/reviews/SPEC-REVIEW-42-r3.md new file mode 100644 index 00000000..742149a6 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-42-r3.md @@ -0,0 +1,251 @@ +# SPEC-REVIEW-42-r3 + +- 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`, ревизия 4 + (коммит `5bdde98b`, docs-only, класс C) +- Заход: r3 · блокирующих циклов израсходовано 2/4 перед этим раундом +- Предыдущий раунд: r2, вердикт жёлтый, найдено на коммите ревизии 3 + `0ed83c36` (документ `docs/reviews/SPEC-REVIEW-42-r2.md`, зафиксирован в + дереве коммитом `6ee53e61`) + +## Скоуп проверки (дельта) + +Раунд r3 — ровно один коммит автора, `5bdde98b` +(`docs: #42 spec revision 4 per SPEC-REVIEW-42-r2`): 11 добавлено / 3 +удалено, единственный файл `docs/specs/042-backend-engineering-quality.md`. +`git diff 0ed83c36..5bdde98b` (полный, не только statshort) — правка +касается ровно трёх мест: + +1. строка ревизии в шапке (техническая, revision 3→4); +2. переформулирован **AC5** целиком: вместо старого «ERROR_CODES ⊇ все + коды send_error (скан исходника)» — явное требование обоих путей + эмиссии (литералы + перечисленные err.code-источники), явное упоминание + `invalid_passage_fields`/`invalid_partition_opening_jamb_margin`; +3. в «Плане автотестов» добавлен мутант **м1b** — убрать один + err.code-источник из перечня сканера → красный AC5. + +Диф не трогает `src/**` и `custom_components/**/*.py` — код-гейты +(`typecheck`/`test`/`build`/`check-docs`/смоки/`pytest tests_backend`) к +этому раунду неприменимы по той же причине, что в r1 и r2 (PROCESS.md §2.4 +vs §2.7; диф докс-only, класс C). + +Правка — точечный ответ ровно на Medium r2 («AC5 и её мутант не +синхронизированы с исправленным блоком 5»). Блок 5 (`:81-116`), раздел +i18n (`:123-129`), «Критерии приёмки» AC1-AC4/AC6/AC7 и всё остальное этой +правкой не тронуто. + +Полный повторный разбор ТЗ не требовался по букве delta-правила: правка +локальна, контракт поведения не меняется, новая подсистема не затронута. +Но AC5 — ровно то, что дельта редактирует, а сама формулировка AC5 теперь +явно ссылается на конкретные классы-источники блока 5 («четыре +validation-класса, `JunctionLimitError`, `MarkerControlError`») — поэтому +её пришлось перепроверить не только на согласованность с прозой блока 5 +(синтаксически совпадает), но и на то, верна ли сама проза блока 5 по +существу для каждого названного класса. Это и есть «AC, чьё доказательство +дельта задевает» — не по тексту, а по субстанции, которую AC5 теперь +инкапсулирует по ссылке. + +## Как проверялось + +- Построчный дифф `git diff 0ed83c36..5bdde98b -- docs/specs/042-*.md`. +- Перечитан весь текущий файл ТЗ целиком (211 строк), не только AC5 и + «План автотестов». +- Для каждого из шести источников, перечисленных в новой AC5 и в блоке 5 + (`OpeningPassageError`, `PartitionOpeningHostError`, + `PartitionOpeningJambMarginError`, `WallModelClientOutdatedError`, + `JunctionLimitError`, `MarkerControlError`), прочитан код на `dev` заново: + - `custom_components/houseplan/validation.py:35-84` — классы и способ + задания `code` (класс-атрибут-литерал у первых четырёх; + `self.code = code`, аргумент конструктора, у `MarkerControlError` — + иначе, чем описывает блок 5); + - `custom_components/houseplan/validation.py:790-999` — все вызовы + `raise MarkerControlError(...)` (16 сайтов), с выпиской каждого + переданного кода: литералы и f-строки с префиксом; + - `custom_components/houseplan/junction_limits.py:56` — + `f"junction_limit_{rule}"`, не тронуто дельтой, сверено с прежним + выводом r1; + - `custom_components/houseplan/websocket_api.py:1366-1370,1742-1747` — + общий `except (...)`-блок, откуда `err.code` уходит в `send_error`, и + поиск (`grep`) литералов MarkerControl-кодов в `websocket_api.py` — + их там нет, то есть путь (а) их тоже не покрывает. +- Сверка новой AC5 с текстом мутантов м1/м1b в «Плане автотестов». + +## Закрытие раунда r2 + +| Находка r2 | Чем закрыта | Где видно | +|---|---|---| +| **Medium** — AC5 (`:148-149` ревизии 3) и её мутант (`:162`) буквально повторяли старую формулировку, не упоминая `ERROR_CODE_FAMILIES` и err.code-источники; наивная реализация «в лоб» дала бы зелёный тест, не доказывающий покрытие `invalid_passage_fields`/`invalid_partition_opening_jamb_margin` | **Закрыта в своей букве.** AC5 переписана: явно требует обоих путей эмиссии, называет по имени `ERROR_CODES`/`ERROR_CODE_FAMILIES`, явно требует, чтобы `invalid_passage_fields` и `invalid_partition_opening_jamb_margin` были доказаны тестом, «источник вне перечня → красный». Добавлен мутант м1b на branch (б). Ровно первый вариант рекомендации r2 (переформулировать AC5 + добавить вторую ветку мутанта), реализован без ссылки-заглушки «см. §5». | `docs/specs/042-backend-engineering-quality.md:148-155` (AC5), `:168-170` (мутант м1b) | + +Закрытие по букве полное — но проверка по существу (не только «синхронизирован ли текст AC5 с блоком 5», а «верен ли сам блок 5 для каждого названного класса») вскрыла отдельный пробел в блоке 5, который AC5 теперь наследует явной ссылкой на `MarkerControlError`. Разбираю ниже как новую находку r3, а не как «M1/r2 не закрыта» — сам предмет r2 (синхронизация текста) закрыт полностью. + +## Находки + +### Medium 1 (в скоупе) — `MarkerControlError` описан в блоке 5/AC5 только по двум префиксным семействам; ~14 её литеральных кодов не покрыты ни одним из двух путей сканирования + +**Файл:** `docs/specs/042-backend-engineering-quality.md:89-99` (блок 5, +не тронут дельтой) и `:148-155` (AC5, тронута дельтой этого раунда). + +**Формулировка ТЗ.** Блок 5 описывает метод извлечения кода для каждого +источника. Для первых четырёх классов — «литеральный class-attr `code = +"..."` — извлекается regex'ом» (это действительно так: +`validation.py:43,57,63,81` — `code = "invalid_passage_fields"` и т. д., +проверено заново). Для `MarkerControlError` метод другой и один: «префиксные +коды `value_badge_*` / `value_source_*` — семейства» (`:94-95`). AC5 +(новая, `:148-151`) повторяет это же деление: «четыре validation-класса, +`JunctionLimitError`, `MarkerControlError`» — как единый список источников +без оговорки, что для `MarkerControlError` описан только один из двух его +режимов эмиссии. + +**Почему это не покрывает класс.** `MarkerControlError` — единственный из +шести источников, у которого `code` не класс-атрибут и не одношаблонная +f-строка с конечным алфавитом (как `junction_limit_{rule}`), а +произвольный аргумент конструктора, разный на каждом из 16 сайтов +`raise MarkerControlError(...)` (`validation.py:790-999`). Из них ровно +четыре сайта используют префиксную f-строку +(`f"{prefix}_marker_missing"`/`f"{prefix}_marker_not_light"`, +`:847,849`, prefix ∈ {`value_badge`,`value_source`}) — это и есть +семейства, которые блок 5 называет. Но ещё 14 сайтов передают чистые +литералы, ни один из которых не начинается с `value_badge_`/`value_source_` +и ни один из которых блок 5 не упоминает: +`invalid_value_badge_source`, `invalid_value_source`, +`invalid_value_badge_attribute`, `invalid_value_source_attribute` +(`:824,833,837,839,844` — код выбирается из двух локальных переменных +`source_error`/`attribute_error`, но обе тоже литералы, не паттерн), +`invalid_value_badge`, `invalid_value_badge_position`, +`value_badge_source_required` (`:861,863,865,868`), +`invalid_light_entity`, `invalid_toggle_entity` (`:919`, через кортеж +`(field, code, message)`), `duplicate_marker_control`, +`invalid_marker_control` (дважды), `marker_control_self`, +`marker_control_missing`, `marker_control_not_light`, +`marker_control_cycle` (`:968-999`). + +Все 14 реально доходят до пользователя тем же путём, что и +`invalid_passage_fields` — общим `except (JunctionLimitError, +MarkerControlError, OpeningPassageError, PartitionOpeningHostError, +PartitionOpeningJambMarginError, WallModelClientOutdatedError) as err: +connection.send_error(msg["id"], err.code, str(err))` +(`websocket_api.py:1366-1370`, идентично `:1742-1747`) — я перепроверил, +что ни один из 14 литералов не встречается как строка в `websocket_api.py` +(`grep` по каждому — пусто), то есть путь (а) их тоже не ловит. Контракт-тест, +реализованный «в лоб» по тексту блока 5 («у MarkerControlError сканируем +только префиксные семейства»), не найдёт эти 14 кодов ни в ERROR_CODES, ни +в ERROR_CODE_FAMILIES — они останутся без `backup.error.` и без +family-фоллбэка, ровно тот же класс дефекта («сырой `e.message` в UI»), +ради которого написан весь блок 5 и обе предыдущие находки M1. + +**Почему это находка r3, а не «пере-открытие r1/r2».** Предмет r1-M1 и +r2-Medium — синхронизация текста AC5 с прозой блока 5; блок 5 сам по себе +r2 признал полным («метод извлечения кода для каждого класса описан +конкретно... префиксы value_badge_/value_source_») — этот вывод оказался +неверным по существу для одного конкретного класса, но текстуально диф +r3 блок 5 не трогал, поэтому по правилам дельты he являлся предметом +проверки текста; предметом стала именно AC5, а новая AC5 инкапсулирует +блок 5 по ссылке дословно («четыре validation-класса, JunctionLimitError, +MarkerControlError» — без оговорок), из-за чего пробел блока 5 стал частью +самого критерия приёмки этого раунда. + +**Как чинится в скоупе:** дополнить описание `MarkerControlError` в блоке 5 +явным перечнем литеральных кодов (аналогично четырём validation-классам) +либо общим правилом извлечения — «сканировать все сайты +`raise MarkerControlError(, ...)`; строковый литерал первым +аргументом → фиксированный код; f-строка с одной из двух переменных +`prefix`/`source_error`/`attribute_error` → семейство по этой переменной; +код без литерала и без опознанного паттерна → красный (недоказуемый +источник)» — и добавить к AC5/плану автотестов третий мутант (например +«убрать `duplicate_marker_control` из ERROR_CODES → красный AC5»), чтобы +литеральная ветвь MarkerControlError была так же доказана падением, как +`send_error`-литералы и err.code-семейства. + +Технический вопрос (какой именно метод сканирования для +`MarkerControlError`), не продуктовый — решаю сам, не выношу владельцу. + +## Унаследовано из r2 + +Без повторной проверки в r3 принято на основании +`docs/reviews/SPEC-REVIEW-42-r2.md` (документ ревью r2, найден на дереве +коммитом `6ee53e61`; измерения и выводы получены на SHA `0ed83c36`) и +транзитивно `docs/reviews/SPEC-REVIEW-42-r1.md` (SHA `582d673a`): + +- независимая перепроверка числовых фактов ТЗ (ruff 333/291/17/13/6/2/2, + `quality_scale.yaml` — ровно 4 `todo`, отсутствие Troubleshooting в + `USER-GUIDE.ru.md`, 26 ключей `backup.error.*`, дубль pip-строки в + `mutation-gate.yml:67`) — дельта r3 их не касается; +- раздел «i18n» (M2 r1) — закрыт полностью в r2, дельта r3 его не трогала, + повторно не открываю; +- продуктовые разделы «Сценарий» и «Что человек увидит до и после», + ценностная оценка задачи (3/10 пользователю, 9/10 разработке) — приняты + владельцем ранее, дельта их не трогала; +- «Скоуп/не-скоуп», «Контракт поведения», AC1-AC4, AC7 — однозначны, + способ доказательства назван, дельта r3 их текст не меняла; +- AC6 и раздел о порядке проверок в `_errText` (`:106-110`) — не менялись + дельтой r3, приняты как в r2; +- риски, откат, release-артефакты, принятые предположения — не менялись, + приняты как в r1/r2; +- Low-находка про заголовки «UX»/«Модель данных и миграция» — статус не + меняется: снята с записью в r1, автор консолидацию не делал и не был + обязан; +- метод извлечения кода для `OpeningPassageError`, `PartitionOpeningHostError`, + `PartitionOpeningJambMarginError`, `WallModelClientOutdatedError`, + `JunctionLimitError` — перепроверен ЗАНОВО в этом раунде (не наследуется + слепо, т.к. AC5 теперь ссылается на них явно) и подтверждён корректным; + только `MarkerControlError` не подтверждён — см. находку выше. + +## Что проверено и корректно (в этом раунде) + +- Синхронизация AC5 с блоком 5 (предмет r2) — полностью закрыта: оба пути + эмиссии названы в самой AC5, `invalid_passage_fields` и + `invalid_partition_opening_jamb_margin` явно поименованы как обязанные + быть доказанными, «источник вне перечня → красный» сформулирован в самом + критерии, а не только в прозе блока 5; +- мутант м1b — корректно нацелен именно на ветку (б), тестируем, дублирует + паттерн м1; +- метод извлечения для пяти из шести перечисленных источников + (`OpeningPassageError`, `PartitionOpeningHostError`, + `PartitionOpeningJambMarginError`, `WallModelClientOutdatedError`, + `JunctionLimitError`) — перепроверен на текущем `dev` и однозначен: + четыре — литеральный класс-атрибут, извлекаемый regex'ом; один — + одношаблонная f-строка с конечным перечнем `rule` (П1-П4), уже + зафиксированным в `junction_limits.py:427-430` (сверено в r1, не + изменилось); +- трейлеры коммита `5bdde98b` — `Issue: #42`, `User-Visible: no` — + корректны (docs-only правка, видимое поведение не меняет); +- строка ревизии в шапке (`4 (2026-08-30) — по SPEC-REVIEW-42-r2 (M1-хвост + в AC5)`) корректно называет раунд и коммит-источник находки. + +## Чего не проверял и почему + +- код-гейты (`typecheck`/`test`/`build`/`check-docs`/`model-invariants`/ + браузерные смоки/`pytest tests_backend`) — диф раунда состоит из одного + файла `docs/specs/042-*.md`, `src/**` и `custom_components/**/*.py` не + тронуты этим коммитом; прогон на неизменном коде не даёт сигнала по + существу ТЗ и не входит в предмет ревью ТЗ (PROCESS.md §2.4 vs §2.7) — + та же причина, что в r1/r2; +- повторный замер ruff/coverage/quality_scale.yaml — факты не менялись + этим раундом, наследуются без повторного прогона; +- фактическая реализация контракт-теста AC5 — её ещё нет (этап spec, не + code); находка о `MarkerControlError` получена чтением исходника + `validation.py`/`websocket_api.py`/`junction_limits.py` на `dev`, а не + исполнением кода — «проверено чтением, не исполнением»; +- допустимость выбора `source_error`/`attribute_error` как «переменных, а + не литералов, но с конечным алфавитом из двух значений» — технически они + ближе к литералам (каждая переменная жёстко привязана к `field`, т.е. по + сути тоже два конкретных литерала на функцию) — не отдельная находка, а + часть того же самого пробела, включена в рекомендацию выше одним пунктом. + +## Итог + +Предмет r2 закрыт полностью: AC5 и её мутант синхронизированы с блоком 5, +явно называют оба пути эмиссии и оба флагманских кода. Но перепроверка +AC5 «не только по тексту, но по существу для каждого названного класса» +(обязательна, поскольку правка этого раунда сама сделала AC5 нормативной +ссылкой на блок 5) вскрыла отдельный пробел: `MarkerControlError` описан +в блоке 5 и, значит, в новой AC5 только по двум префиксным семействам, а +14 её литеральных кодов, доходящих до пользователя тем же путём err.code, +не покрыты ни одним из двух методов сканирования. Реализация «в лоб» по +тексту revision 4 воспроизведёт тот же класс дефекта, ради которого +написан весь блок 5. Возврат — по одной находке Medium в скоупе, чинится +правкой того же файла (дополнить блок 5 явным перечнем/правилом для +литеральных кодов `MarkerControlError` и третий мутант в плане +автотестов), без нового цикла владельца и без пересмотра архитектуры.