diff --git a/docs/reviews/SPEC-REVIEW-42-r2.md b/docs/reviews/SPEC-REVIEW-42-r2.md new file mode 100644 index 00000000..9134232e --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-42-r2.md @@ -0,0 +1,175 @@ +# SPEC-REVIEW-42-r2 + +- 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`, ревизия 3 + (коммит `0ed83c36`, docs-only, класс C) +- Заход: r2 · блокирующих циклов израсходовано 1/4 (перед этим раундом) +- Предыдущий раунд: r1, вердикт жёлтый, найдено на коммите ревизии 2 + `582d673a` (документ `docs/reviews/SPEC-REVIEW-42-r1.md`, зафиксирован в + дереве коммитом `e270bd71`) + +## Скоуп проверки (дельта) + +Между раундами в историю `dev` попал один посторонний коммит +`50851dc3` (`ci: classify against the last proven-green ancestor`, +issue #387) — он не относится к #42, файла ТЗ не трогает, из разбора +исключён. + +Сам раунд r2 — это ровно один коммит автора, `0ed83c36` +(`docs: #42 spec revision 3 per SPEC-REVIEW-42-r1`): 32 добавлено / 10 +удалено, единственный файл `docs/specs/042-backend-engineering-quality.md`. +Диф не трогает `src/**` и `custom_components/**/*.py` — код-гейты +(`typecheck`/`test`/`build`/`check-docs`/смоки/`pytest tests_backend`) +к этому раунду неприменимы по той же причине, что и в r1 (PROCESS.md §2.4 +vs §2.7). Правка — точечный ответ на M1/M2 из r1: переписан блок 5 «WS error +contract» и добавлен раздел «i18n»; больше ничего в файле не менялось +(строка ревизии в заголовке — техническая). + +Полный повторный разбор ТЗ не требовался: правка локальна, контракт +поведения не меняется, новая подсистема не затронута, объём дельты (один +абзац + новый короткий раздел) кратно меньше исходной задачи. Инвариантов +модели, i18n-паритета файлов, смоков — гейты не относятся к этому дифу, +т.к. геометрия/UI-код не изменились. + +## Как проверялось + +- Построчный дифф `git diff 582d673a..0ed83c36 -- docs/specs/042-*.md`, + сверка против текста находок M1 и M2 из + `docs/reviews/SPEC-REVIEW-42-r1.md` (читал полностью). +- Перечитан весь текущий файл ТЗ целиком (203 строки) — не только + изменённый блок 5, чтобы поймать несогласованность между новым текстом + блока 5 и разделом «Критерии приёмки», который дифф НЕ трогал. +- Проверено присутствие обязательных разделов §7.1 в актуальной версии + файла (список секций постранично). + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где видно | +|---|---|---| +| **M1** — AC5 не покрывает коды, эмитируемые не литералом в `send_error(...)` (err.code-классы `OpeningPassageError`/`PartitionOpeningHostError`/`PartitionOpeningJambMarginError`/`WallModelClientOutdatedError`, `JunctionLimitError`, `MarkerControlError`) | **Частично.** Раздел «5. WS error contract» переписан: введён `ERROR_CODE_FAMILIES`, оба пути эмиссии (литералы `send_error` и `err.code`-классы) перечислены явно с методом извлечения (regex по class-attr, f-строка по списку rules, префиксные семейства) — ровно вариант (а) из рекомендации r1. **Но** сама формулировка **AC5** в разделе «Критерии приёмки» (`docs/specs/042-*.md:148-149`) и её мутант в «Плане автотестов» (`:162`, «удалить код из ERROR_CODES → красный AC5») остались текстом ревизии 2 — тем самым, который M1 назвал недостаточным («скан исходника: каждый литерал `send_error(...)`-кода»). Правка не долетела с блока 5 до самого критерия приёмки. Разбираю как новую находку r2 ниже. | `docs/specs/042-backend-engineering-quality.md:83-99` (закрыто) против `:148-149,162` (не закрыто) | +| **M2** — раздел «i18n» отсутствует; не зафиксировано, новый ключ или переиспользование `err.code` | **Полностью.** Добавлен раздел «## i18n» (`:123-129`): новых ключей нет, переиспользуются существующие `err.unknown`/`err.code` (4 локали), плюс явное решение по порядку проверок в `_errText` — code-first — вынесено в блок 5 (`:106-110`, помечено «M2 r1»). | `docs/specs/042-backend-engineering-quality.md:106-110,123-129` | +| Low — разделы «UX» и «Модель данных и миграция» не оформлены отдельными заголовками §7.1 | Не тронуто. В r1 уже снято с записью («не блокирует, консолидировать при следующей правке, отдельного цикла не требует») — повторно не поднимаю, автор правку не делал и не обязан был. | без изменений, см. «Унаследовано» | + +## Находки + +### Medium 1 (в скоупе) — AC5 и её мутант не синхронизированы с исправленным блоком 5; критерий приёмки снова недоказателен + +**Файл:** `docs/specs/042-backend-engineering-quality.md` + +**Что не так.** Раздел 5 (`:83-99`) теперь корректно требует от контракт-теста +покрывать оба пути эмиссии кода (литералы `send_error` + перечисленные +err.code-источники/семейства). Но: + +- **AC5** (`:148-149`) буквально повторяет старую формулировку: «ERROR_CODES + ⊇ все коды send_error (скан исходника); каждый код имеет en-ключ + `backup.error.`» — ни слова про `ERROR_CODE_FAMILIES`, про + перечисленные классы-источники `err.code`, про требование «family-ключ + ИЛИ задокументированный fallback». Как единственный формально + «критерий приёмки», именно этот текст, а не проза блока 5, будет тем, с + чем код-ревьюер на этапе `code` сверяет реализацию; +- мутант AC5 в «Плане автотестов» (`:162`) — «удалить код из ERROR_CODES → + красный AC5» — проверяет только фиксированный список, не проверяет, что + скан действительно достаёт err.code-семейства. Реализация «в лоб» под + букву AC5 (наивный скан литералов `send_error(...)`) снова даст зелёный + тест, который ничего не доказывает для `invalid_passage_fields` и + `invalid_partition_opening_jamb_margin` — то есть воспроизводит именно + тот сценарий, который M1 описывал как дефект. + +**Почему это находка именно r2, а не «M1 не закрыта».** Текст блока 5 +переписан правильно и по существу отвечает на M1; пропуск — в том, что +правка не была протянута до самого критерия приёмки и его мутанта, а +именно они образуют доказательную часть ТЗ (PROCESS.md §7.1: «критерии +приёмки AC1…ACn с указанием доказательства»). Автор в комментарии к +ревизии 3 заявляет «AC5/блок 5 покрывают ОБА пути эмиссии» — по факту диффа +это верно только для блока 5. + +**Как чинится в скоупе:** переформулировать AC5, например: «ERROR_CODES ⊇ +все коды-литералы `send_error(...)`; ERROR_CODE_FAMILIES ⊇ все коды из +перечисленных err.code-источников (`OpeningPassageError`, +`PartitionOpeningHostError`, `PartitionOpeningJambMarginError`, +`WallModelClientOutdatedError`, `JunctionLimitError`, `MarkerControlError`); +каждый код/семейство имеет либо `backup.error.`, либо +задокументированный fallback; появление кода/класса вне обоих списков → +красный» — и добавить к мутанту м1 вторую ветку («убрать класс-источник из +перечня err.code → красный AC5»), либо явно сослаться на текст блока 5 как +на нормативный (тогда AC5 короче, но должен явно писать «см. §5» вместо +повторения устаревшей формулировки). + +Технический вопрос (какой именно текст AC), не продуктовый — решаю сам, не +выношу владельцу. + +## Унаследовано из r1 + +Без повторной проверки в r2 принято на основании +`docs/reviews/SPEC-REVIEW-42-r1.md` (документ ревью r1, найден на дереве +коммитом `e270bd71`; измерения и выводы получены на 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`) — дельта r2 этих фактов не касается, они не + переизмерялись повторно; +- продуктовые разделы «Сценарий» и «Что человек увидит до и после» — на + месте, продуктовые, без терминов реализации (дельта их не трогала); + ценностная оценка задачи (3/10 пользователю, 9/10 разработке) принята + владельцем в комментариях issue ранее, не переоткрываю; +- «Скоуп/не-скоуп», «Контракт поведения», AC1–AC4, AC7 — однозначны, способ + доказательства назван у каждого, тест умеет упасть (дельта r2 их текст не + меняла); +- AC6 — проверен ПОВТОРНО в этом раунде (см. ниже), т.к. дельта его касается + через M2/блок 5; +- риски, откат, release-артефакты, принятые предположения — не изменялись + дельтой, приняты как в r1; +- Low-находка про заголовки «UX»/«Модель данных и миграция» — статус не + меняется: снята с записью в r1, автор консолидацию не делал и не был + обязан её делать в этом раунде. + +## Что проверено и корректно (в этом раунде) + +- **M2 закрыта полностью** — раздел «i18n» присутствует, решение явное + (переиспользование `err.unknown`/`err.code`, никаких новых ключей), + согласуется с фактом, перепроверенным в r1 (ключи уже переведены на 4 + языка); порядок «code-first» в `_errText` зафиксирован текстом в блоке 5 + (`:106-110`) — ровно то, что просила находка M2; +- **AC6** (`:150-152`) остаётся однозначным и согласован с новым текстом + блока 5: «JSON-message двух кодов парсится в structured details; старый + regex-формат по-прежнему принимается; неизвестный код → локализованный + fallback, английский message не попадает в DOM» — этого достаточно для + проверки code-first фикса без необходимости упоминать порядок явно в + самом AC (порядок — деталь реализации, наблюдаемый результат AC6 не + меняется); +- блок 5 (`:83-99`) — единственный содержательно переписанный фрагмент — + сам по себе не содержит новых пробелов: оба пути эмиссии названы, + метод извлечения кода для каждого класса описан конкретно (regex по + `code = "..."`, список `rules` П1–П4, префиксы `value_badge_`/ + `value_source_`), критерий «family-ключ ИЛИ документированный fallback» + однозначен, «код/класс вне списков → красный» закрывает вопрос полноты + перечня на будущее; +- трейлеры коммита `0ed83c36` — `Issue: #42`, `User-Visible: no` — + корректны (видимого поведения этот коммит не меняет, он правит только + текст ТЗ). + +## Чего не проверял и почему + +- код-гейты (`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); +- повторный замер ruff/coverage/quality_scale.yaml — факты не менялись + этим раундом, перепроверены в r1 и наследуются без повторного прогона; +- содержимое коммита `50851dc3` (`ci: classify against the last + proven-green ancestor`) — прочитан только `git show --stat` для + подтверждения, что он не относится к #42; по существу не ревьюился, + т.к. принадлежит issue #387. + +## Итог + +M2 закрыта полностью. M1 закрыта только в прозе блока 5 — критерий +приёмки AC5 и его мутант остались текстом ревизии 2, который сам M1 назвал +недоказательным. Возврат — по одной находке Medium в скоупе, чинится +правкой того же файла (переформулировать AC5 и добавить вторую ветку +мутанта либо явно сослаться на блок 5), без нового цикла владельца и без +пересмотра архитектуры.