docs: review document for #42

Issue: #42
User-Visible: no

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

@@ -0,0 +1,294 @@
+# SPEC-REVIEW-42-r4
+
+- 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`, ревизия 5
+  (коммит `84008a0a`, docs-only, класс C)
+- Заход: r4 · блокирующих циклов израсходовано 3/4 перед этим раундом
+- Предыдущий раунд: r3, вердикт жёлтый, найдено на коммите ревизии 4
+  `5bdde98b` (документ `docs/reviews/SPEC-REVIEW-42-r3.md`, зафиксирован в
+  дереве коммитом `6542d190`)
+
+## Скоуп проверки (дельта)
+
+Между раундами в историю `dev` попал один посторонний коммит,
+`65339f63` («ci: judge the push range from the last proven-green ancestor»,
+`Issue: #388`) — инфраструктура классификации CI-диапазона, класс B/infra,
+не относится к #42, `docs/specs/042-*.md` не трогает, из разбора исключён.
+Коммит `6542d190` — мой собственный документ ревью r3, зафиксированный в
+дереве; тоже не предмет этого раунда.
+
+Предмет раунда r4 — ровно один коммит автора, `84008a0a`
+(«docs: #42 spec revision 5 per SPEC-REVIEW-42-r3»): 12 добавлено / 4
+удалено, единственный файл `docs/specs/042-backend-engineering-quality.md`.
+`git diff 5bdde98b..84008a0a -- docs/specs/042-backend-engineering-quality.md`
+(полный) касается ровно двух мест:
+
+1. строка ревизии в шапке (техническая, revision 4→5, ссылка на
+   SPEC-REVIEW-42-r3);
+2. переписан абзац про `MarkerControlError` внутри блока 5 (`:94-107`):
+   вместо старого «префиксные коды `value_badge_*`/`value_source_*` —
+   семейства» (это было единственное, что блок 5 говорил о классе, и ровно
+   это r3 назвал неполным) — теперь заявлены «два пути»: (1) ~15
+   литеральных кодов аргументом конструктора, извлекаемых «regex'ом по
+   вызовам `MarkerControlError("<код>"`», обязаны быть ∈ ERROR_CODES; (2)
+   f-string-семейства `value_badge_*`/`value_source_*` ∈
+   ERROR_CODE_FAMILIES; явное fail-closed правило для кода вне обоих путей.
+
+Диф не трогает `src/**` и `custom_components/**/*.py` — код-гейты
+(`typecheck`/`test`/`build`/`check-docs`/смоки/`pytest tests_backend`) к
+этому раунду неприменимы по той же причине, что в r1-r3 (PROCESS.md §2.4 vs
+§2.7; диф докс-only, класс C).
+
+«Критерии приёмки» (AC5, `:156-163`) и «План автотестов» (`:170-180`) диффом
+НЕ тронуты — те же м1/м1b/м2, что в ревизии 4.
+
+Правка — точечный ответ на Medium r3 («MarkerControlError описан только по
+двум префиксным семействам; ~14 литеральных кодов не покрыты ни одним из
+путей сканирования»). Полный повторный разбор ТЗ не требовался по букве
+delta-правила: правка локальна, контракт поведения не меняется, новая
+подсистема не затронута. Но, как и в r3, сама суть правки — конкретный
+метод статического извлечения кода — обязывает перепроверить не только
+«назван ли метод», а «работает ли названный метод на реальных сайтах
+`raise MarkerControlError(...)`» — это и есть «AC, чьё доказательство
+дельта задевает» по существу.
+
+## Как проверялось
+
+- Построчный дифф `git diff 5bdde98b..84008a0a -- docs/specs/042-*.md`.
+- `git log --oneline 5bdde98b..84008a0a` и `git show --stat` на каждом из
+  трёх промежуточных коммитов — подтверждено, что предмет раунда ровно один
+  коммит `84008a0a`, `65339f63` относится к другому issue (#388).
+- Перечитан весь текущий файл ТЗ целиком (219 строк), не только изменённый
+  абзац блока 5.
+- Для каждого из 16 сайтов `raise MarkerControlError(...)` в
+  `custom_components/houseplan/validation.py:790-999` заново прочитан код с
+  точными номерами строк (`grep -n "raise MarkerControlError\|source_error
+  =\|attribute_error =\|for field, code, message"`) и классифицирован
+  способ передачи кода: прямой строковый литерал / f-строка с `prefix` /
+  локальная переменная, связанная условным присваиванием / локальная
+  переменная из распакованного литерального кортежа.
+- Сверка новой формулировки блока 5 с найденной классификацией — построчно,
+  сайт за сайтом.
+
+## Закрытие раунда r3
+
+| Находка r3 | Чем закрыта | Где видно |
+|---|---|---|
+| **Medium** — блок 5 описывал `MarkerControlError` только по двум префиксным семействам (`value_badge_*`/`value_source_*`); ~14 её литеральных кодов не были упомянуты ни в блоке 5, ни в AC5 ни одним путём сканирования | **Закрыта в букве, не по существу.** Блок 5 переписан: теперь заявлено «два пути», литеральная ветвь названа явно (~15 кодов, примеры перечислены), дано правило извлечения («regex по вызовам `MarkerControlError("<код>"`») и fail-closed для кода вне обоих путей — ровно первая половина рекомендации r3. Но перепроверка каждого из 16 сайтов по коду показала, что заявленный метод извлечения для литеральной ветви (простой regex на строковый литерал сразу после открывающей скобки) находит только 9 из заявленных ~15 кодов; 6 остальных передаются через локальную переменную (`source_error`/`attribute_error`/`code` из распакованного кортежа), а не литералом на месте вызова — регэксп, буквально описанный текстом, их не найдёт. Вторая часть рекомендации r3 (третий мутант для литеральной ветви) не добавлена вовсе — «План автотестов» не тронут диффом. Разбираю как новую находку r4 ниже, а не «r3 не закрыта»: предмет r3 (упоминание литеральных кодов в блоке 5) закрыт, но сам названный метод оказался неполон для другого поднабора этих же кодов. | `docs/specs/042-backend-engineering-quality.md:94-107` (новый текст блока 5); `custom_components/houseplan/validation.py:819-844,900-919` (сайты, не покрываемые заявленным методом) |
+
+## Находки
+
+### Medium 1 (в скоупе) — заявленный метод извлечения кода `MarkerControlError` («regex по вызовам `MarkerControlError("<код>"`») не находит 6 из ~15 кодов, которые сам блок 5 относит к этой ветви — они передаются через локальную переменную, а не литералом на месте вызова
+
+**Файл:** `docs/specs/042-backend-engineering-quality.md:94-103` (блок 5,
+правка этого раунда).
+
+**Формулировка ТЗ (ревизия 5).** «MarkerControlError — два пути (r3): (1)
+~15 ЛИТЕРАЛЬНЫХ кодов аргументом конструктора (...) — сканер извлекает их
+regex'ом по вызовам `MarkerControlError("<код>"` и требует каждый ∈
+ERROR_CODES; (2) f-string-коды с префиксами `value_badge_`/`value_source_`
+— семейства ∈ ERROR_CODE_FAMILIES. Вызов MarkerControlError с нелитеральным
+кодом вне известных f-string-паттернов → красный сканер (fail-closed).»
+
+Формулировка описывает единственный метод для ветви (1): текстовый
+regex, ищущий кавычку сразу после `MarkerControlError(`. Это подразумевает,
+что первым аргументом КАЖДОГО из ~15 сайтов ветви (1) стоит инлайн-строковый
+литерал.
+
+**Почему это не так.** Перечитан код на `dev`, все 16 сайтов
+`raise MarkerControlError(...)` (`validation.py:790-999`) заново
+классифицированы по способу передачи первого аргумента:
+
+- **9 сайтов** — первым аргументом действительно стоит строковый литерал в
+  кавычках прямо на месте вызова: `"invalid_value_badge"` (`:861,863`,
+  дважды один код), `"invalid_value_badge_position"` (`:865`),
+  `"value_badge_source_required"` (`:868-870`, литерал переносится на
+  следующую строку — под regex попадает, если тот терпим к
+  переводу строки), `"duplicate_marker_control"` (`:968`),
+  `"invalid_marker_control"` (`:974,990`, дважды один код),
+  `"marker_control_self"` (`:992`), `"marker_control_missing"` (`:995`),
+  `"marker_control_not_light"` (`:997`), `"marker_control_cycle"` (`:999`).
+  Для этих 9 заявленный метод действительно работает.
+- **6 кодов передаются через локальную переменную**, а не литералом на
+  месте вызова — заявленный текстовый regex (кавычка сразу после
+  открывающей скобки) их НЕ находит, потому что первым аргументом на этих
+  сайтах стоит голый идентификатор:
+  - `source_error`/`attribute_error` — присваиваются условно чуть выше по
+    коду (`validation.py:819` `source_error = "invalid_value_badge_source"
+    if field == "value_badge" else "invalid_value_source"`; `:821`
+    аналогично для `attribute_error` → `invalid_value_badge_attribute`/
+    `invalid_value_source_attribute`), затем используются в
+    `raise MarkerControlError(source_error, ...)` на `:824,833,837,844` и
+    `raise MarkerControlError(attribute_error, ...)` на `:839`. Ни один из
+    этих пяти сайтов не содержит кавычки сразу после `MarkerControlError(`
+    — там идентификатор `source_error`/`attribute_error`. Итого 4 кода:
+    `invalid_value_badge_source`, `invalid_value_source`,
+    `invalid_value_badge_attribute`, `invalid_value_source_attribute`;
+  - `code` — распаковывается из литерального кортежа в цикле
+    (`validation.py:900-916`: `for field, code, message in (("light_entity",
+    "invalid_light_entity", ...), ("toggle_entity", "invalid_toggle_entity",
+    ...))`), используется в `raise MarkerControlError(code, message)`
+    (`:919`) — снова голый идентификатор, не литерал на месте вызова.
+    Итого 2 кода: `invalid_light_entity`, `invalid_toggle_entity`.
+
+Итого 9 + 6 = 15 — общее число совпадает с заявленным в ТЗ («~15»), но
+метод извлечения, которым ТЗ объясняет, как сканер их найдёт, работает
+только для 9. Оставшиеся 6 — не догадка и не пограничный случай: это
+конечный, статически разрешимый набор (переменная присваивается ровно
+одним из двух литералов по условию, читаемому в том же модуле, либо
+берётся из литерального кортежа прямо над циклом), но простой текстовый
+regex «кавычка сразу после `MarkerControlError(`» это не описывает и не
+поймает.
+
+**Почему это находка r4, а не «r3 не закрыта».** Предмет r3 — то, что блок
+5 вообще не упоминал ~14 литеральных кодов `MarkerControlError`, описывая
+только два префиксных семейства. Это закрыто: блок 5 теперь называет
+литеральную ветвь явно, с верным количеством и с примерами. Пробел
+переместился на уровень ниже: сам заявленный МЕТОД извлечения для этой
+ветви (простой regex по кавычке) не покрывает часть тех же кодов, которые
+эта же правка обязалась покрыть. Ровно тот же тип эскалации, что был между
+r2 и r3 (текст обновлён по букве находки, но не выдерживает проверки по
+существу для конкретных сайтов кода).
+
+**Почему это существенно, а не техническая тонкость.** Все 6 кодов
+принадлежат реальным пользовательским сценариям валидации маркеров (выбор
+источника/атрибута value-бейджа, выбор entity для light/toggle) — то есть
+ровно тот класс ошибок, ради которого написан весь блок 5 (`_errText`
+показывает сырое английское сообщение для кода без `backup.error.<code>`).
+Реализация контракт-теста «в лоб» по тексту ревизии 5 (наивный regex на
+кавычку) даст зелёный тест, который не находит эти 6 кодов в принципе —
+не «не докажет покрытие» (как в r1/r3), а структурно не сможет их увидеть,
+потому что описанный метод ищет не в том месте.
+
+**Как чинится в скоупе.** Один из двух вариантов, оба текстовые правки того
+же файла:
+
+- (а) описать вторую подветвь литерального пути явно — «переменная,
+  присвоенная ровно одним из конечного набора строковых литералов условным
+  выражением в том же модуле (`source_error`, `attribute_error`) либо
+  распакованная из литерального кортежа над циклом (`code` в
+  `validate_marker_light_entities`), — сканер обязан резолвить такую
+  переменную статически до её объявления»; либо
+- (б) вариант, который сам r3 уже предлагал как альтернативу и который
+  проще формально доказать: трактовать `source_error`/`attribute_error`
+  как ЕЩЁ ОДНО семейство по этой переменной (аналогично `prefix`) — тогда
+  ветвь (2) блока 5 расширяется третьей парой семейств, а не требует
+  произвольной резолюции условных присваиваний.
+
+В обоих случаях — добавить к «Плану автотестов» мутант, специфично
+нацеленный на эту подветвь (например «убрать `invalid_light_entity` из
+ERROR_CODES → красный AC5»), иначе третий раз подряд остаётся риск, что
+заявленный текст и фактически проверяемое поведение разойдутся невидимо
+для автора реализации. Это та же рекомендация про третий мутант, что была
+дана в r3 и не выполнена этим раундом — привязываю её к конкретному коду
+теперь, чтобы дальше не расходилась.
+
+Технический вопрос (какой именно метод статического извлечения выбрать),
+не продуктовый — решаю не выносить владельцу; выбор между (а) и (б) не
+меняет видимого пользователю поведения и не требует его решения.
+
+## Унаследовано из r3
+
+Без повторной проверки в r4 принято на основании
+`docs/reviews/SPEC-REVIEW-42-r3.md` (документ ревью r3, найден на дереве
+коммитом `6542d190`; измерения и выводы получены на SHA `5bdde98b`) и
+транзитивно `docs/reviews/SPEC-REVIEW-42-r2.md` (SHA `0ed83c36`,
+документ `6ee53e61`) и `docs/reviews/SPEC-REVIEW-42-r1.md` (SHA `582d673a`,
+документ `e270bd71`):
+
+- независимая перепроверка числовых фактов ТЗ (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`) — дельта r4 их не касается;
+- раздел «i18n» (M2 r1) — закрыт полностью в r2, дельта r4 его не трогала;
+- продуктовые разделы «Сценарий» и «Что человек увидит до и после»,
+  ценностная оценка задачи (3/10 пользователю, 9/10 разработке) — приняты
+  владельцем ранее, дельта их не трогала;
+- «Скоуп/не-скоуп», «Контракт поведения», AC1-AC4, AC6, AC7 — однозначны,
+  способ доказательства назван, дельта r4 их текст не меняла;
+- метод извлечения кода для `OpeningPassageError`, `PartitionOpeningHostError`,
+  `PartitionOpeningJambMarginError`, `WallModelClientOutdatedError`
+  (класс-атрибут-литерал, regex) и `JunctionLimitError`
+  (`f"junction_limit_{rule}"`, конечный список rules П1-П4) — перепроверены
+  ЗАНОВО в r3 и подтверждены корректными; дельта r4 этих классов не
+  касалась, повторно не перепроверял;
+- текст самой AC5 (`:156-163`) — не менялся дельтой r4, остаётся тем же,
+  что был признан корректным (достаточно общим, чтобы покрывать оба пути
+  без переписывания при каждом уточнении блока 5) в r3;
+- риски, откат, release-артефакты, принятые предположения — не менялись,
+  приняты как в r1-r3;
+- Low-находка про заголовки «UX»/«Модель данных и миграция» — статус не
+  меняется: снята с записью в r1, автор консолидацию не делал и не был
+  обязан.
+
+## Что проверено и корректно (в этом раунде)
+
+- Правка блока 5 закрывает предмет r3 по букве: литеральная ветвь
+  `MarkerControlError` теперь названа явно, с верным числом кодов (15) и
+  представительными примерами;
+- 9 из 15 кодов литеральной ветви — прямые строковые литералы на месте
+  вызова, для них заявленный regex-метод действительно работает
+  (`invalid_value_badge`, `invalid_value_badge_position`,
+  `value_badge_source_required`, `duplicate_marker_control`,
+  `invalid_marker_control`, `marker_control_self`, `marker_control_missing`,
+  `marker_control_not_light`, `marker_control_cycle`);
+- f-string-семейства (ветвь (2), `value_badge_*`/`value_source_*`) — текст
+  не менялся дельтой этого раунда и остаётся корректным (подтверждено в r3);
+- fail-closed правило («код вне обоих путей → красный сканер») сформулировано
+  явно и корректно закрывает вопрос полноты перечня на будущее — само по
+  себе не имеет пробелов;
+- трейлеры коммита `84008a0a` — `Issue: #42`, `User-Visible: no` —
+  корректны (docs-only правка, видимое поведение не меняет);
+- строка ревизии в шапке (`5 (2026-08-30) — по SPEC-REVIEW-42-r3
+  (MarkerControlError-коды)`) корректно называет раунд и коммит-источник
+  находки.
+
+## Чего не проверял и почему
+
+- код-гейты (`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-r3;
+- повторный замер ruff/coverage/quality_scale.yaml — факты не менялись
+  этим раундом, наследуются без повторного прогона;
+- фактическая реализация контракт-теста AC5 — её ещё нет (этап spec, не
+  code); находка о неполноте заявленного метода извлечения получена
+  чтением исходника `validation.py:790-999` построчно на `dev`, а не
+  исполнением кода — «проверено чтением, не исполнением»;
+- содержимое коммита `65339f63` (`ci: judge the push range from the last
+  proven-green ancestor`) — прочитан только `git show --stat` для
+  подтверждения, что он не относится к #42; по существу не ревьюился, т.к.
+  принадлежит issue #388.
+
+## Итог
+
+Предмет r3 закрыт по букве: блок 5 больше не молчит о литеральных кодах
+`MarkerControlError`. Но перепроверка заявленного метода извлечения по
+каждому из 16 реальных сайтов `raise MarkerControlError(...)` на `dev`
+показала, что метод («regex по вызовам `MarkerControlError("<код>"`»)
+находит только 9 из 15 заявленных кодов; оставшиеся 6
+(`invalid_value_badge_source`, `invalid_value_source`,
+`invalid_value_badge_attribute`, `invalid_value_source_attribute`,
+`invalid_light_entity`, `invalid_toggle_entity`) передаются через
+локальную переменную, а не литералом на месте вызова, и описанный текстом
+regex их структурно не увидит. Это тот же класс дефекта, ради которого
+писался весь блок 5 — реализация «в лоб» по тексту ревизии 5 оставит эти
+шесть кодов без `backup.error.<code>` и без family-фоллбэка. Возврат — по
+одной находке Medium в скоупе, чинится правкой того же файла (описать
+вторую подветвь литерального пути через резолюцию конечного набора
+переменных либо трактовать её как ещё одно семейство — рекомендация (б)
+проще формально доказать) и добавлением специфичного мутанта в «План
+автотестов», без нового цикла владельца и без пересмотра архитектуры.
+
+Заход r4 — четвёртый; с учётом этого вердикта блокирующих циклов
+израсходовано **4 из 4** (§4 PROCESS.md). Пятого захода на правку по
+летящему циклу не будет: следующий шаг после исправления этой находки —
+решение владельца (разделить/отклонить/принять как есть), если правка не
+будет признана владельцем самоочевидной технической деталью, либо, если
+владелец сочтёт находку в рамках уже согласованного техрешения — арбитраж
+по PROCESS.md §4 п.3. Находка узкая (одна фраза + один мутант в одном
+файле), сам механизм ТЗ и его продуктовая часть вопросов не вызывают.