mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 11:49:16 +00:00
@@ -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.<code>`» — ни слова про `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.<code>`, либо
|
||||
задокументированный 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), без нового цикла владельца и без
|
||||
пересмотра архитектуры.
|
||||
Reference in New Issue
Block a user