From adb1727c50b09a2fe37ec7e0342cfd45dfa4d491 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 01:08:12 +0000 Subject: [PATCH] docs: review document for #618 Issue: #618 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-618-r1.md | 166 +++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-618-r1.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 474b170f..a73c1da4 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,6 +1,6 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1009, issue: 351. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1010, issue: 352. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| @@ -25,6 +25,7 @@ | #624 | [CODE-REVIEW-624-r1.md](CODE-REVIEW-624-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #621 | [CODE-REVIEW-621-r1.md](CODE-REVIEW-621-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #619 | [CODE-REVIEW-619-r1.md](CODE-REVIEW-619-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | +| #618 | [SPEC-REVIEW-618-r1.md](SPEC-REVIEW-618-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | нотация h в B5 не встречается в коде | `docs/FILTERING.md` | | #615 | [SPEC-REVIEW-615-r1.md](SPEC-REVIEW-615-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | AC3 называет несуществующую защиту от расползания на плашки цвета | `smoke_room_settings_form.mjs` `smoke_space_settings_form.mjs` `smoke_device_settings_form.mjs` `smoke_dialog_polish_605.mjs` `smoke_general_settings_form.mjs` | | #614 | [SPEC-REVIEW-614-r1.md](SPEC-REVIEW-614-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #614 | [CODE-REVIEW-614-r1.md](CODE-REVIEW-614-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | AC1/AC2/AC3 требуют unit-тест, а он не написан | `test/dialog-baseline.test.mjs` `test/space-dialog.test.mjs` `general-form-state.ts` `space-form-state.ts` `marker-form-state.ts` `tsconfig.test.json` `dialog-baseline.ts` `scripts/mutation-registry.mjs` | diff --git a/docs/reviews/SPEC-REVIEW-618-r1.md b/docs/reviews/SPEC-REVIEW-618-r1.md new file mode 100644 index 00000000..58d20aa8 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-618-r1.md @@ -0,0 +1,166 @@ +# SPEC-REVIEW — issue #618 · заход r1 + +**Вердикт: зелёный** · High: 0 · Medium (в скоупе): 0 · Medium (вне скоупа): 0 · Low: 1 (снят с записью) + +## Скоуп + +ТЗ (тело issue #618, раздел `## ТЗ`) предлагает пакетные действия «Скрыть +выбранные» / «Показать выбранные» с множественным выбором строк в каталоге +устройств: вкладка «На плане» → скрыть пакетом, вкладка «Скрытые» → показать +пакетом; одна запись `houseplan/config/set` на пакет вместо N последовательных. +Трек — полный (аналитика в issue корректно фиксирует нарушение критерия §5: +множественный выбор и пакетное действие — новый UX-контракт, явно вынесенный +из скоупа #29 в `docs/specs/029-device-inbox-lifecycle.md` §6). + +Ревью — состязательное: без контекста автора, только тело issue, комментарий +аналитики и текущее состояние репозитория на коммите +`0f97e3e639c4eb06b7866b8c1c757d98e2c0a5f7`. + +## Как проверялось + +Ревью ТЗ не гоняет код-гейты (нет ни ветки, ни диффа реализации — проверено: +`git branch -a | grep 618` и `gh pr list --search 618` пусты, задача ещё не +покидала `S3-spec`). Проверка — построчная сверка каждого утверждения контракта +(§5 B1–B12), UX (§6), i18n (§8) и AC (§9) с фактическим состоянием кода и +документации, а не только с внутренней непротиворечивостью текста: + +- `src/device-inbox.ts` — `canHide`/`canShow`, `category`, статус-модель + (`src/ha-binding-status.ts`: `active`/`ha_disabled`/`orphaned`/`unverified`) — + сверены с формулировками B2 и текстами статусов в §5 дословно; +- `src/houseplan-editor-runtime.ts:7091–7122` (`_setInboxHidden`) — сверена с + B5 (создание заглушки, сохранение `hidden:false`, единственный живой маркер + на привязку) и с использованием `markerIdForBinding` (`src/logic.ts:805`); +- `_writeConfig`/`_saveConfigNow` (сериализованная очередь, `expected_rev`) — + подтверждает B5/B8/риск «конкурентная запись» как факт, а не предположение; +- `demo/smoke_hidden_flag.mjs:41` — подтверждён риск §12 «селекторы соседних + смоков»: смок берёт **последний** чекбокс в `.device-inbox-filters`, и + требование §15 «панель выбора — контейнер `.device-inbox-batch` вне + `.device-inbox-filters`» действительно единственный способ не сломать этот + смок новыми чекбоксами; +- `docs/FILTERING.md` — сверена терминология «заглушка», `hidden:false` = явно + видимое, `marker.removed` ≠ `marker.hidden`; +- `docs/USER-GUIDE.ru.md` — сверены имена вкладок «На плане», «Доступны», + «Скрытые», «Доступны снова», ключ `device_inbox.show_more` («Показать ещё»); +- `src/i18n/{en,ru,de,fr}.json` — все 9 новых ключей §8 не пересекаются с + существующими, паттерн `{count}` после двоеточия действительно используется + в `filters_preview_hide`/`filters_preview_appear` во всех четырёх словарях; +- `scripts/mutation-registry.mjs:2057` — существование `device-tombstone- + blocks-child-picker`, рядом с которым §9 просит вставлять новые мутанты, + подтверждено; +- `test/i18n.test.mjs`, `test/i18n-dead-keys.test.mjs` — существуют (AC9); +- golden id `device-inbox-desktop-en-light` / `-ru-dark` / `-narrow-ru-dark` — + существуют в `demo/golden/matrix.mjs:590-596`; +- npm-скрипты/файлы гейтов §10 (`typecheck`, `test`, `build`, `gate:small`, + `bundle:budget`, `scripts/no-new-any.mjs`, `scripts/mutation-gate.mjs --check + --id=`) — существуют и синтаксис `--id=` реально читается парсером + (`scripts/mutation-gate.mjs:60`); +- `gh issue list --search "bulk OR массов OR batch"` — дублей не найдено, + подтверждает заявление аналитики «дубликаты: нет»; +- `docs/TOUCH-SUPPORT.md` — B12/§11 «touch — best effort, без новых жестов» + не противоречит политике (редакторы вне гарантии View/kiosk). + +## Находки + +### Low — L1: нотация `h` в B5 не встречается в коде + +**Файл:** тело issue #618, раздел `## ТЗ` → §5, пункт B5, первая подпункт. +**В чём дело:** формулировка «"Показать" у автоматически скрытой заглушки +(`h`) оставляет маркер с `hidden: false`…» использует нотацию `h`, +которой нет ни в `src/logic.ts:markerIdForBinding` (device → `ref`, entity → +`'lg_' + ref`), ни в `docs/FILTERING.md`, ни где-либо ещё в репозитории — +проверено `grep -rn "h" .` по коду и докам. Похоже на собственную +иллюстративную нотацию автора ТЗ («h» = hidden-заглушка), не помеченную как +условная. +**Почему не блокирует:** наблюдаемое поведение описано однозначно и без этой +нотации («маркер остаётся, `hidden` становится `false`, не удаляется»), и AC4 +доказывается сравнением `applyInboxVisibility` с последовательным применением +к реальным маркерам, а не поиском строки `h` — импланентация не может +пойти по ложному следу из-за этой строки. +**Решение ревьюера:** снято записью, правки не требуется; автору стоит либо +убрать `h`, либо явно пометить как условное обозначение — на усмотрение, +без повторного цикла. + +## Что проверено и корректно + +- **Сценарий и «что человек увидит»** (§1–2) — персона, поверхность, момент по + `docs/SCOPE.md` (работа J6 «Keep the plan true as the home evolves»); текст + «до/после» описывает наблюдаемое поведение, не реализацию. +- **Скоуп/не-скоуп** (§4) — граница с #29 корректна и подтверждена цитатой + архивной спеки; список исключений (undo, доступны/доступны снова, + сидер, `new_device_ids`, выбор рамкой на плане, View/kiosk) исчерпывающий и + не оставляет неявных предположений о смежном поведении. +- **Контракт B1–B12** — каждое утверждение сверено с реальным кодом (см. «Как + проверялось»); противоречий с `docs/FILTERING.md` и текущей реализацией + `_setInboxHidden` не найдено. +- **AC1–AC11** — все пронумерованы, у каждого указан способ доказательства и, + для защитных AC2–AC5, обязательная по §2.7 таблица «чем доказан / чем + краснеет» с названным мутантом — заполнена для всех четырёх без исключения. +- **i18n** (§8) — 9 ключей, все четыре словаря, без коллизий с существующими + ключами и с уже принятым в проекте паттерном плюрализации через `{count}`. +- **Модель данных/миграция** (§7) — корректно установлено «нет»: переиспользуется + существующее поле `marker.hidden`, backend не меняется; `docs/CONFIG- + COMPATIBILITY.md` действительно не затрагивается (новых полей и ключей нет). +- **Touch** (§11, B12) — соответствует `docs/TOUCH-SUPPORT.md` (редактор — + best effort, View/kiosk не тронуты, что не является блокирующим по этому + документу). +- **Перф** (§11) — корректно: отбор строк уже мемоизирован (`_deviceInboxMemo`), + батч не меняет асимптотику; бюджет — через существующий `bundle:budget`. +- **Откат** (§13) и **release-артефакты** (§14) — конкретны и достаточны + (revert без миграции; changelog RU+EN, USER-GUIDE ru/en, `FILTERING.md`, + golden-пересъёмка перечислены явно). +- **Продуктовые умолчания** аналитики (7 пунктов в комментарии) корректно + перенесены в контракт (B1–B12) и не блокируют выпуск — ни один не требует + вопроса владельцу по критерию §7.1 («что человек видит/делает»); все + решаемые агентами технические детали (имя функции, поле state, сторожевое + значение `busy`) вынесены в §15 «Принято предположительно» и не подменяют + продуктовые решения. +- **Риски** (§12) — четыре названных риска реальны (сверено: селектор + `smoke_hidden_flag`, сериализованная запись, отбор «Показать ещё»), не + формальная отписка. +- **Открытых продуктовых вопросов нет** — аналитика прямо заявляет это и + объясняет почему (`blocked` не ставится); проверкой текста ТЗ подтверждено: + ни одно утверждение о поведении не выдано за факт без основания в коде или + в уже принятых доках (FILTERING.md, TOUCH-SUPPORT.md, USER-GUIDE.ru.md). +- **DoR-чеклист §2.5** — все пункты закрыты текстом ТЗ: AC с доказательством, + затронутые файлы названы в аналитике, i18n перечислен, миграция решена явно, + перф/touch названы, release-артефакты названы, откат описан, открытых + вопросов и рисков без описания нет. + +## Чего не проверял + +- **Код-гейты** (`tsc --noEmit`, `npm test`, `npm run build`, + `check-docs.mjs`, смоки, golden, инварианты модели) — не прогонялись: + предмет этого этапа — текст ТЗ, ветки реализации не существует + (`git branch -a`, `gh pr list --search 618` — пусто), гонять гейты не на + чем. Это будет предметом код-ревью после реализации. +- **Правильность будущей реализации** `applyInboxVisibility` и функции отбора + доступных строк — их ещё нет; их корректность и мутанты (§9, §15) — + предмет код-ревью. +- **Устный контекст автора** — не запрашивался и не использовался (правило + «ревьюер ≠ автор», §2.4). + +## Материал раунда + +- Issue: #618, репозиторий `Matysh/houseplan-card`. +- Тело issue на момент ревью: SHA-256 нормализованного текста (как выгружено + `gh issue view 618 --json body -q .body`) — + `e4e93cd0a5975d686b2a0809752a97320a070f968ba2eaa6305810143f877962`. +- Единственный комментарий на момент ревью: аналитика S2 от Matysh, + 2026-09-24T00:58:56Z (`issuecomment-5805532054`). +- Рабочая копия репозитория: коммит `0f97e3e639c4eb06b7866b8c1c757d98e2c0a5f7`. +- Заход r1, циклов ревью ТЗ израсходовано 0 из 4 (§4: зелёный вердикт цикл не + образует). + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `0f97e3e639c4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `f2603b2998c17fd63d3bc1248b5e2be36e4b598f` + ``` + git log --all --format='%H %T' | grep f2603b2998c1 + ``` +- Тело issue: `1febf156fc3f3a1ba3e0dcdb91ffc12851abf4061419c0df08d78f930d04db04` +- Вердикт конвейера: `green` · High 0