From 1babdfb55837ba7e8eadbdfb4604b92f88cffa76 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 01:19:32 +0000 Subject: [PATCH] docs: review document for #629 Issue: #629 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-629-r1.md | 186 +++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-629-r1.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 0def59ef..a067e86d 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -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 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-629-r1.md b/docs/reviews/SPEC-REVIEW-629-r1.md new file mode 100644 index 00000000..96935ad8 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-629-r1.md @@ -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ирована, а не +скрыта), единственное нетривиальное архитектурное отступление явно помечено, +обосновано и не требует эскалации владельцу. Материала для возврата автору нет. + +**Готово к разработке.** + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `adb1727c50b0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c5f33ffb2d92b3b2675ee1f23f67ac61ef572365` + ``` + git log --all --format='%H %T' | grep c5f33ffb2d92 + ``` +- Тело issue: `00fd8d5cfe7f3778ae654cefa41d7042e6f514b9a75b2db5eaa26c253a73deb9` +- Вердикт конвейера: `green` · High 0