From cab4b4153af3992fbc23178d9255673dfb03d940 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 06:36:32 +0000 Subject: [PATCH] docs: review document for #687 Issue: #687 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-687-r1.md | 205 +++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-687-r1.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index a2d082e2..b1dae299 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,9 +1,10 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 161, issue: 75. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 162, issue: 76. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| +| #687 | [SPEC-REVIEW-687-r1.md](SPEC-REVIEW-687-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #686 | [CODE-REVIEW-686-r1.md](CODE-REVIEW-686-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #684 | [CODE-REVIEW-684-r1.md](CODE-REVIEW-684-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #683 | [SPEC-REVIEW-683-r1.md](SPEC-REVIEW-683-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-687-r1.md b/docs/reviews/SPEC-REVIEW-687-r1.md new file mode 100644 index 00000000..c3f16fe9 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-687-r1.md @@ -0,0 +1,205 @@ +# SPEC-REVIEW-687-r1 + +Issue: #687 — редактор плана: показывать значки устройств ориентирами, «1 в 1 +как в редакторе подложки». +Этап: spec (ревью ТЗ, PROCESS.md §2.4). +Заход: r1 · блокирующих циклов израсходовано 0 из 2 (лёгкий трек, §4). +Материал: тело issue #687, раздел `## ТЗ`, состояние на момент чтения +(2026-09-28T06:34Z), код на `HEAD` `a0b471ca1bbea96eb978936860d8250dfd83cd68`. +Продуктовый вопрос (0.2 vs «1 в 1 как в подложке») снят решением владельца от +2026-09-28, зафиксирован в теле issue до раздела `## ТЗ` — открытых вопросов к +владельцу в тексте не осталось. + +## Скоуп + +Задача обслуживает J6 из `docs/SCOPE.md` («Keep the plan true as the home +evolves» — расстановка стен/проёмов/лестниц) через устранение переключений в +View за ориентирами устройств; это явное расширение уже принятого прецедента +J6/#362 (ориентиры устройств в редакторе подложки), а не новая работа. Видимое +поведение меняется: `User-Visible: yes` в release-артефактах корректен. + +## Как проверялось + +Ревью текста ТЗ плюс построчная сверка каждого фактического утверждения о +текущем коде с исходниками на `HEAD`, чтобы отличить проверяемый факт от +недоказанной догадки: + +- `src/styles/plan.styles.ts:1179` — `.stage.markup .devlayer .dev { display: + none; }` — подтверждено дословно. +- `src/styles/plan.styles.ts:1002–1012` — правило подложки + `.stage.mode-decor .devlayer, .stage.mode-decor .devlayer *, + .stage.mode-decor .dev::before { pointer-events: none; }` с комментарием + именно про 44px псевдо-зону — подтверждено. +- `src/styles/plan.styles.ts:1013–1026` — групповая `opacity: var(--hp-mode- + architecture-opacity, 1)` на `.room, .devlayer, .opening, …`; значение 0.35 + для `decor` — подтверждено в `src/houseplan-card.ts` (переменная + `--hp-mode-architecture-opacity` ставится в шаблоне сцены, точная строка + сдвинута на несколько строк от заявленной `:10868`, но выражение и значение + найдены и совпадают). +- `src/houseplan-card.ts:5594,5736,6966` — `_clickDevice`, `_keyDevice`, + `_pointerDown` — все три уже начинаются с `if (this._mode !== 'view' && + this._mode !== 'devices') return;` — гварды действительно уже стоят вне CSS, + как и заявляет раздел «Как сейчас». Это существенно снижает риск задачи: + AC2 в основном проверяет уже существующую защиту плюс новую CSS-развязку + для передачи клика/курсора инструменту. +- `src/houseplan-card.ts:11780–11829` — `interactive = mode === 'view' || + mode === 'devices'`, `role`/`tabindex` условны на этом же флаге — + подтверждено. +- `src/styles/devices.styles.ts:176–186` — `.dev::before` (44px, `pointer- + events: auto`, `z-index: 1`) — подтверждено; ровно этот элемент требует + явного упоминания в новом правиле pointer-events, что ТЗ и делает по + аналогии с подложкой. +- `src/styles/devices.styles.ts:356–360` — `.dev.unavail { opacity: 0.35; … + }` — подтверждает математику контракта п.2: 0.35 (множитель) × 0.35 + (собственная) = 0.1225 для недоступного маркера. +- `src/styles/devices.styles.ts:369–376` — `.dev.ghost { opacity: 0.6; … }` — + показывает, зачем контракт умножает на *свою* непрозрачность маркера + (`filter`), а не переопределяет общий `opacity` тем же селектором: живой + `.dev` уже несёт разные собственные `opacity` по состоянию, которые нельзя + просто затереть одним правилом той же CSS-property. +- `src/houseplan-card.ts:10692–10693` — `showGhosts = mode === 'devices' && + this._showAll`, `devs = this._renderDevices.filter(d => d.space === space.id + && (!d.hidden || showGhosts))` — набор маркеров в Plan (`markup`) и во View + идентичен (оба `showGhosts === false`), что делает AC1 «то же множество, + что во View» проверяемым фактом, а не предположением. +- `src/houseplan-card.ts:12060,6679,6702` — `.roomlabel` — отдельный класс, + сосед `.dev` внутри `.devlayer`, а не его потомок и не сам `.dev`. Значит + правило pointer-events, если оно (по аналогии с подложкой) будет нацелено на + `.dev`/`.dev *`/`.dev::before`, а не на весь `.devlayer`, не заденет подписи + комнат — контракт п.3 и п.4 совместимы, конфликта нет. Сам выбор конкретного + селектора — техническая деталь реализации, а не продуктовое решение + (§7.1), и она уже дважды перепроверяется тестами (AC2 на неинтерактивность + маркера, AC3 на интерактивность подписи) — конфликт, если бы он был, + проявился бы там. +- `docs/DECOR-EDITOR.md:25` («Context emphasis») — «Rooms, labels, devices, + openings … render at 35% opacity … pointer-inert» — подтверждает, что + контракт п.2 корректно переносит уже задокументированное поведение подложки, + а не изобретает число 0.35. +- `docs/USER-GUIDE.ru.md:258–261` — таблица режимов; строка «Редактор плана» + сегодня действительно «Устройства: Скрыты», строка «Редактор подложки» — + «Не редактируются и полупрозрачны». Release-артефакт («как в подложке») + указывает на готовую, не придуманную формулировку. +- `demo/smoke_feedback_v2.mjs:17` — комментарий `// (в редакторе плана .dev + скрыты display:none — сравнивать не с чем)` найден дословно; ТЗ верно + определяет его как устаревший и требующий правки. +- `demo/smoke_decor.mjs:21–50` — существующий паттерн смок-теста подложки + (`elementFromPoint`, сверка вычисленной `opacity` `.devlayer`) — подтверждает + техническую посильность плана автотестов для нового `demo/ + smoke_plan_device_landmarks.mjs`, хотя для плана сверяется не `opacity` + группы, а составное `opacity × filter: opacity()` на самом маркере — ТЗ это + явно называет. +- `scripts/mutation-registry.mjs` — find/replace-мутанты на `display: none` и + `pointer-events: none` — уже устоявшийся паттерн в файле; два новых мутанта, + описанных в плане автотестов, механически однотипны существующим. +- PROCESS.md §7.1 (обязательные разделы), §2.5 (DoR). + +Гейты не запускались: код не менялся (это ревью текста ТЗ, а не реализации), +гейты этапа spec не входят в объём ревью ТЗ (PROCESS.md §2.4, §8). + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +## Что проверено и корректно + +**Обязательные разделы §7.1 присутствуют и в осмысленном порядке**: сценарий → +что человек увидит до/после → скоуп/не-скоуп → контракт поведения (5 пунктов) +→ UX/модель данных/i18n (одним разделом, содержательно — «ничего нового») → +критерии приёмки AC1–AC3 с колонкой доказательства → план автотестов → риски → +откат → release-артефакты → «принято предположительно». «Проблема» вынесена в +раздел `## Зачем` перед `## ТЗ` — формально вне заголовка `## ТЗ`, но по +содержанию это ровно требуемый раздел, а не пропуск; отделять его как +находку было бы придиркой к форме, а не к содержанию (§2.4: искать, где ТЗ не +выполнимо или не проверяемо). + +**Продуктовая развилка закрыта решением владельца, а не подставлена +предположением.** Исходные 0.2/выбор частей маркера заменены явным «1 в 1 как +в подложке» — ТЗ верно определяет, что это снимает оба вопроса из исходного +тела, и не пытается решить их самостоятельно. + +**Контракт непротиворечив и переносит уже работающий прецедент, а не +изобретает новый.** Числа (0.35, 0.35 × 0.35 для `.dev.unavail`), состав +маркера (ядро/кольцо/капсула/бейджи/LQI/анимации), объём неинтерактивности +(44px-зона, капсула, курсор, Tab, привязка) — всё это дословно повторяет уже +существующий и задокументированный контракт подложки (`DECOR-EDITOR.md`, +`plan.styles.ts:1002–1026`, `devices.styles.ts:356–376`), перепроверено против +кода, а не принято на слово автора. + +**Раздел «принято предположительно» разрешает именно ту техническую +развилку, которая реально важна для числа, видимого пользователю** — почему +множитель ставится через `filter: opacity()` на `.dev`, а не через групповую +`opacity` на `.devlayer` (иначе подписи комнат, лежащие в том же `.devlayer`, +потеряли бы обязательную по контракту п.4 непрозрачность/интерактивность). +Аналогичная развилка для pointer-events (там же нужно не задеть `.roomlabel`) +в тексте не разобрана отдельно, но это чисто техническая деталь реализации +(§7.1 — не наблюдается пользователем, значит не требует записи в ТЗ), и она +уже покрыта тестами AC2+AC3 с двух сторон. + +**AC1–AC3 однозначны и указывают способ доказательства** (`smoke` / `smoke` + +«ревью кода»), защитные пункты AC2 сформулированы как конкретные пробы +(прямая отправка событий, `elementFromPoint`, вычисленный курсор, отсутствие +`tabindex`), а не общими словами «должно работать». + +**План автотестов называет мутанты для обоих защитных срезов** (возврат +`display: none` → AC1, снятие `pointer-events: none` → AC2) и явно чинит +единственный найденный устаревший комментарий-регрессию. + +**Риски и откат достаточны для CSS-only задачи класса small**: откат — вернуть +одну строку CSS; риск golden — учтён через существующий предрелизный механизм +приёмки (`golden:accept --reviewed --expect-change`, §11.4), а не оставлен +как открытый вопрос. + +**Release-артефакты названы конкретно и совпадают с реальным состоянием +документов**: строка таблицы `USER-GUIDE.ru.md` действительно сегодня +«Скрыты» и нуждается в правке; `UX-MODES.md` › Plan существует как секция для +дополнения. + +## Чего не проверял + +- Не проверял golden-снимки и реальный рендер маркеров в редакторе плана + исполнением — кода ещё нет, это гейт этапа code, а не spec (PROCESS.md §8). +- Не проверял точный будущий текст CSS-селектора для pointer-events (см. выше + про `.roomlabel`) — это решает автор при реализации; отмечено как + непроблемное, а не как «принято на веру». +- Не проверял влияние на layout/paint-бюджеты производительности исполнением + (профиль не запускался) — риск назван текстом («это только редактор, View + не затронут»), но не измерен числом; для задачи класса `small`, чисто CSS, + без новых DOM-узлов (маркеры и так в DOM) это не блокирует ревью ТЗ — + измерение, если оно понадобится, ляжет на этап code (AC явно не требует + performance-профиля). +- Не проверял состояние других открытых issue на пересечение скоупа, кроме + явно названного в оценке владельца #362 (ориентиры подложки, закрыт, + контракт которого переносится). + +## Вердикт + +Зелёный. ТЗ содержит все обязательные разделы, однозначные и проверяемые +AC1–AC3 со способом доказательства, продуктовая развилка закрыта решением +владельца, а не догадкой автора, и каждое проверяемое утверждение о текущем +коде подтверждено чтением `HEAD`. Технические детали, оставленные вне текста +(конкретный CSS-селектор pointer-events), не наблюдаются пользователем и не +требуют ответа владельца; риск, который они несут, уже закрыт пересекающимися +AC. Замечаний, требующих возврата автору, нет. + +--- + +## Материал раунда + +- Issue: #687, тело на момент ревью — `2026-09-28T06:28:35Z` (последний + комментарий автора, статус на момент чтения `S4-spec-review`). +- Код: без изменений, `HEAD` `a0b471ca1bbea96eb978936860d8250dfd83cd68`. +- Ветка задачи не создана — код в этом раунде не пишется (spec-этап). + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `a0b471ca1bbe` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `d8cdbaac038e822d048c65fa20e5e1837946cf95` + ``` + git log --all --format='%H %T' | grep d8cdbaac038e + ``` +- Тело issue: `266af7ca2b7a21e08ab181761390dc4bab1425862dc2c6e48811d7777da34daa` +- Вердикт конвейера: `green` · High 0