docs: review document for #629

Issue: #629
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-24 01:19:32 +00:00
parent 4a5bf2290a
commit 1babdfb558
2 changed files with 188 additions and 1 deletions
+2 -1
View File
@@ -1,6 +1,6 @@
# Индекс ревью
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1012, issue: 354. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1013, issue: 355. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|---|---|---|---|---:|---:|---|---|
@@ -16,6 +16,7 @@
| #635 | [CODE-REVIEW-635-r1.md](CODE-REVIEW-635-r1.md) | code · r1 | 🟡 жёлтый | 1 | 0 | индекс молчаливо теряет находки и врёт числами по текущему | `docs/reviews/CODE-REVIEW-639-r1.md` `CODE-REVIEW-637-r1.md` `docs/reviews/CODE-REVIEW-594-r1.md` `docs/LESSONS.md` |
| #635 | [CODE-REVIEW-635-r2.md](CODE-REVIEW-635-r2.md) | code · r2 | 🟡 жёлтый | 1 | 1 | docs/reviews/INDEX.md, зафиксированный в материале ревью, устарел на собственном SHA — …; parseFindings/parseFiles: фолбэк «первая строка тела блока» вырезает начало буллета и п… | `docs/reviews/INDEX.md` `SPEC-REVIEW-625-r1.md` `SPEC-REVIEW-625-r2.md` `CODE-REVIEW-625-r1.md` `CODE-REVIEW-625-r2.md` `process.yml` `test/reviews-index.test.mjs` `INDEX.md` |
| #635 | [CODE-REVIEW-635-r3.md](CODE-REVIEW-635-r3.md) | code · r3 | 🟢 зелёный | 0 | 0 | firstParagraph: ветка нет\b в фильтре мёртвая из-за ASCII-only \b в JS-регэкспах, расхо… | `scripts/reviews-index.mjs` |
| #629 | [SPEC-REVIEW-629-r1.md](SPEC-REVIEW-629-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #627 | [SPEC-REVIEW-627-r1.md](SPEC-REVIEW-627-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | избыточное (не противоречивое) условие в AC2; влияние на touch не названо явным пунктом | `docs/TOUCH-SUPPORT.md` |
| #625 | [SPEC-REVIEW-625-r1.md](SPEC-REVIEW-625-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | новый инвариант markers[].id не описывает исход для уже испорченной хранимой конфигурации; продуктовые формулировки §7.1 неполны; не проговорены явные «нет» по i18n/touch | `validation.py` `__init__.py` |
| #625 | [SPEC-REVIEW-625-r2.md](SPEC-REVIEW-625-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
+186
View File
@@ -0,0 +1,186 @@
# SPEC-REVIEW-629-r1
**Issue:** #629 · **Этап:** spec (ТЗ на ревью, PROCESS.md §2.4) · **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
**Материал:** тело issue #629, раздел `## ТЗ` (плюс верхняя часть `## Факты/Правка/AC`, которую ТЗ явно наследует и уточняет). Аналитика (S2) — комментарий https://github.com/Matysh/houseplan-card/issues/629#issuecomment-5805622166.
**Трек:** полный (S5 «сложность ≤ 3» нарушена — гейт с разбором AST, фасад из 10 операций, перевод трёх смоков; названо явно в ТЗ и в аналитике).
**Проверено на дереве репозитория:** `adb1727c50b09a2fe37ec7e0342cfd45dfa4d491` (для сверки технических утверждений ТЗ с текущим кодом — не как материал code-review).
**Вердикт:** зелёный.
## Скоуп ревью
Задача — инструментальная (харнесс/гейт тестов), ни одна из трёх персон SCOPE.md
ничего не видит; продукт получает единственный невидимый DOM-атрибут
(`data-hp="mode-tab"`) на вкладках режимов. Проверялось:
1. Обязательные разделы §7.1 присутствуют и в правильном порядке (сценарий →
что человек увидит → проблема → скоуп/не-скоуп → контракт поведения → UX →
модель данных и миграция → i18n → AC → план автотестов → перф/touch → риски →
откат → release-артефакты) — раздел «Принято предположительно» сверх нормы,
по правилу §7.1.
2. Каждый AC (AC1–AC11) — однозначен, у каждого указан способ доказательства и
явно назван мутант/красный случай («чем краснеет»), кроме AC11 (документация,
доказывается чтением — верно для документного AC) и AC10 (перевод смоков,
доказывается прямым сравнением списков проверок до/после — по тексту AC явно
написано, что «защиты нет, §2.7 не требуется», это осознанное и корректное
исключение, не пропуск).
3. Продуктовая рамка (SCOPE.md): задача не создаёт нового продуктового
поведения, только повышает надёжность тестов, защищающих J3/J4/J6-поверхности
(редакторы). Вопросов, требующих продуктового решения владельца, в ТЗ
корректно не возникло — видимых изменений нет, «что человек видит» отвечено
явно: «пользователь карточки не видит ничего».
4. Раздел §15 «Принято предположительно» — проверено, что каждое допущение
действительно техническое (не продуктовое) и обосновано, а не выдано за факт
без пометки. Главное отступление — «фасад в харнессе, а не в продукте»,
вопреки формулировке в верхней (более старой) части issue «Фасад — продуктовый
код»; это архитектурное решение о механизме инструмента, а не о видимом
поведении, и потому по PROCESS.md §7.1 относится к зоне «агенты решают сами»,
не к зоне вопросов владельцу. Обоснование (три причины: приватные методы
остались бы приватными; отсутствие бэкдора по флагу в публичном бандле;
отсутствие новых членов карточки) — предметное, ревьюер имеет право
оспорить и не оспаривает: аргументы состоятельны, а альтернатива
(`__HP_VERSION_OVERRIDE__`-подобный продуктовый флаг) была бы избыточным
риском ради задачи с нулевой пользовательской видимостью.
## Как проверялось
Ревью ТЗ построено на сверке фактических технических утверждений ТЗ с текущим
деревом — не потому что это код-ревью (это не он), а потому что «утверждение о
поведении, которого нет ни в одном документе и которое не помечено как
предположение, — замечание» (инструкция ревью), и единственный способ отличить
проверенный факт от красиво написанной догадки — заглянуть в код, на который ТЗ
ссылается. Расхождение здесь означало бы, что реализация в 4/5 случаев наткнётся
на несуществующий хук, и задача вернётся на второй круг ещё до кода.
Проверенные утверждения (все подтвердились):
- `.modetab` существует (`src/houseplan-card.ts:10864`), `data-hp="mode-tab"`
на нём — действительно отсутствует; `data-editor-navigation` и
`[data-hp="editor-close"]` — на месте (не меняются, как и требует P1).
- Все 9 остальных селекторов фасада (F) уже существуют как контрактные хуки:
`data-hp="space-tab"` (houseplan-card.ts:10825), `data-hp="tool"
data-tool=…` и `data-hp="toolbar"` (houseplan-editor-runtime.ts,
editor-secondary.ts), `data-hp="room-settings" data-room=…`
(houseplan-editor-runtime.ts:10658), `data-hp="space-settings"`
(houseplan-card.ts:10839), `data-hp="space-add"` / `data-hp="create-space"`,
`data-hp="dialog-cancel"`, `data-kind="room"|"marker"|"space"` на `hp-dialog`
(houseplan-editor-runtime.ts:12336, src/editors/marker-dialog.ts:858,
src/space-copy-runtime.ts:86). Значит утверждение §3 «не хватает только
хука вкладки режима» — не догадка, а проверенный факт.
- `docs/data-hp-contract.json` действительно уже несёт `audience: ["test"]` для
чисто тестовых хуков (`create-space` и др.) — паттерн для нового `mode-tab`
воспроизводим без нового прецедента.
- `docs/STYLING-HOOKS.md` §7.4 «Editors» существует и уже описывает
`data-hp="toolbar"`/`"tool"`/`"editor-close"` — ссылка ТЗ на «STYLING-HOOKS
§7.4» для новой записи корректна и не потребует создавать раздел с нуля.
- `demo/srv/demo.html`: `CFG`/`LAYOUT` сейчас действительно `const`
(строки 52, 69), `CFG_REV`/`LAYOUT_REV` уже `let`; `CONNECTION.subscribeEvents`
(строка 111) на неизвестном имени события возвращает `()=>{}` немедленно —
подтверждает утверждение §7 «подписка и раньше возвращала функцию отписки,
просто пустую» дословно.
- `scripts/no-new-any.mjs` действительно экспортирует `movedLinesByFile` и
`MOVED_BLOCK_MIN`, использует маркер `any-ok` с тем же форматом сообщения,
который ТЗ переиспользует для `private-ok` (G3, G4) — «переиспользуется, не
копируется» реалистично, а не пожелание.
- `scripts/unused-locals-gate.mjs` действительно оперирует терминами
`portPrivates`/`harnessPrivates` — F3/риск «lint:unused» ссылается на
существующий механизм, а не на выдуманный.
- `scripts/mutation-registry.mjs`: анкер `hp-dialog-ignores-flex-content`
существует (строка 9922), новых коллизий с предложенными id
(`room-settings-click-does-not-open`, `hp-dialog-escape-does-not-close`,
`config-updated-event-ignored`, `private-writes-*`) нет — место вставки в §9
ТЗ выполнимо буквально.
- `demo/smoke_area_relocation.mjs` действительно оборачивает `hass.callWS` и
копит `calls` (строка 13+) — риск §12 о журнале вызовов при добавлении
`setServerConfig` обоснован, не гипотетичен.
- `demo/helpers/` сейчас содержит ровно фикстуру ha-dialog #505
(`README-ha-dialog-505.md`, `ha-dialog-assets.mjs`, `ha-dialog-fixture.mjs`) —
подтверждает и утверждение issue «demo/helpers — только фикстура ha-dialog»,
и что F4 (крестик HA — приватный shadow root) опирается на реальный
существующий контекст, а не изобретён для этой задачи.
- `scripts/check-inputs.mjs`: `BROWSER_PROTOCOL` и `frontend`/`smoke` роуты
существуют в описанном виде — AC5 (манифест) выполним без новой концепции.
- `dist/` и `custom_components/houseplan/frontend/` — оба реально существуют и
оба содержат `houseplan-card.js`/`houseplan-panel.js` — команда AC9
(`grep -rc __hpTest dist custom_components/houseplan/frontend`) исполнима как
написана.
- `scripts/gate-small.mjs` и `.github/workflows/validate.yml` действительно
вызывают `no-new-any.mjs` в описанных местах (parallelSteps / шаг `frontend`).
- `test/data-hp-contract.test.mjs` существует — AC8 ссылается на реальный
существующий валидатор, а не на будущий.
- `scripts/monolith-metrics.mjs` существует — ссылка в «не-скоупе» корректна.
Ни одно проверенное фактическое утверждение не разошлось с кодом. Это необычно
высокая для полного трека степень технической проработки — автор явно провёл
собственный разбор дерева перед написанием ТЗ, а не собрал правдоподобный текст.
## Находки
Нет ни одной High- или Medium-находки. Мелочей, которые стоило бы фиксировать
как Low, тоже не нашлось после проверки: формулировки однозначны, зачёты (G3),
исключения (G4) и покрытие вызовов (G5) описаны с точными границами и
контрпримерами прямо в тексте («c._tool = 'draw' → 'select' проходит, новая
запись в новое поле — нет»), что снимает обычный для лёгких ревью риск
«красиво звучит, но не проверяемо».
Единственное, что заслуживало разбора — уже разобрано в §15 «Принято
предположительно» самим автором (отступление «фасад в харнессе, не в
продукте» от формулировки верхней части issue) — и разбор корректен по существу
(см. «Скоуп ревью», п. 4). Это не находка, а пример того, как раздел §15 должен
работать: явное решение с обоснованием, которое ревьюер может оспорить и не
находит оснований оспаривать.
## Что проверено и корректно
- Структура ТЗ полностью соответствует §7.1 PROCESS.md, обе продуктовые секции
(сценарий, что человек увидит) отвечены по существу и корректно constatируют
отсутствие видимого изменения.
- Все AC1–AC9 несут мутант или отрицательную пробу («чем краснеет»); AC10/AC11 —
документные/сравнительные, корректно освобождены от требования мутанта самим
текстом §2.7/AC.
- Технические утверждения (см. «Как проверялось») подтверждены чтением
актуального кода, а не приняты на слово.
- Скоуп/не-скоуп разграничены точно, включая явный вывод продуктовых дефектов,
найденных при переводе смоков, в отдельные issue (§12, риск 1) — соответствует
правилу «скоуп не расширяется» (PROCESS §2.6).
- Откат и release-артефакты покрыты корректно для задачи без пользовательской
видимости (`User-Visible: no` везде, changelog не требуется).
- i18n, touch, перф — отвечены по существу («нет»/«не затрагивается» с кратким
обоснованием), не формальной отпиской.
## Чего не проверял
- Не проверял, что реализация действительно уложится в объём (гейт + фасад из
10 операций + фикстура + 3 перевода смоков + новый смок + документы) без
выхода за скоуп — это предмет код-ревью, не спецификации.
- Не прогонял никакие гейты (typecheck/test/build) — это этап spec-review,
материал которого есть текст ТЗ, а не диапазон коммитов; на этом этапе они не
требуются и не относятся к предмету ревью.
- Не проверял глубину покрытия трёх переводимых смоков построчно (какие именно
строки `smoke_area_relocation`/`smoke_glow`/`smoke_grid_snap` попадут под
исключения) — это будет видно по факту перевода в коде, спецификация лишь
обязывает к результату (AC10) и это корректно для стадии ТЗ.
- Не проверял названия i18n-ключей (их нет — задача не создаёт пользовательских
строк).
## Заключение
ТЗ полное, однозначное, каждый AC доказуем и способен покраснеть, продуктовая
рамка выдержана (нулевая видимая поверхность корректно constatирована, а не
скрыта), единственное нетривиальное архитектурное отступление явно помечено,
обосновано и не требует эскалации владельцу. Материала для возврата автору нет.
**Готово к разработке.**
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `adb1727c50b0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `c5f33ffb2d92b3b2675ee1f23f67ac61ef572365`
```
git log --all --format='%H %T' | grep c5f33ffb2d92
```
- Тело issue: `00fd8d5cfe7f3778ae654cefa41d7042e6f514b9a75b2db5eaa26c253a73deb9`
- Вердикт конвейера: `green` · High 0