diff --git a/docs/reviews/SPEC-REVIEW-29-r2.md b/docs/reviews/SPEC-REVIEW-29-r2.md new file mode 100644 index 00000000..7d01e620 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-29-r2.md @@ -0,0 +1,163 @@ +# SPEC-REVIEW-29-r2 + +- Issue: https://github.com/Matysh/houseplan-card/issues/29 +- Этап: spec (PROCESS.md §2.4) +- Заход: r2 · блокирующих циклов израсходовано 1 из 4 +- ТЗ: `docs/specs/029-device-inbox-lifecycle.md` +- SHA предыдущего вердикта (r1): `a6ce1ae7e534fa4b41fcc0ab93e5ed95a693c4c1` +- SHA материала этого раунда: `375b402d940a0ea30f23becef95f9755212ecbd9` + (ветка `issue/29-device-inbox-lifecycle`, коммит «docs: preserve hidden + device ghost mode») +- Трек: обычный, лимит циклов ревью ТЗ — 4. + +## Скоуп разбора (по дельте, PROCESS.md §2.10) + +Предыдущий вердикт (r1, документ `docs/reviews/SPEC-REVIEW-29-r1.md`) — +жёлтый, две находки Medium в скоупе (M1, M2), High нет. Автор ответил +комментарием в issue со ссылкой на коммит `375b402b` (фактический SHA — +`375b402d`, см. выше) и заявил закрытие обеих находок. + +Дельта — `git diff a6ce1ae7e534fa4b41fcc0ab93e5ed95a693c4c1..HEAD` по файлу +ТЗ: 36 добавленных / 11 удалённых строк, все — в §10.1, §10.2, §10.3, §10.4, +§14, AC1, AC4, AC6, §18 (риски). Дельта локальна: один документ, правки +адресуют ровно M1 и M2, новая подсистема не затронута, поведенческий +контракт вне уже согласованных Q1/AC не меняется, объём дельты не +сопоставим с объёмом исходного ТЗ (548 строк). Условия «разбор остаётся +полным» (ребейз на ушедший вперёд `dev`, смена контракта, новая подсистема, +сопоставимый объём) не выполнены — сокращаю объём разбора до дельты и её +последствий для AC1, AC4, AC6, §14, §18. + +Продуктового кода на этом этапе нет, гейты §8 к ревью ТЗ не относятся (как +и в r1). + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **M1** (Medium, в скоупе) — §10.1 убирал существующий режим просмотра скрытых/HA-disabled маркеров призраками на плане, не давая эквивалента в каталоге (`Find` работал только для реально отрисованного marker) | Автор не выбрал ни один из трёх предложенных путей закрытия буквально, а снял противоречие в корне: существующий локальный ghost-режим **сохранён** целиком (позиция, выбор, настройка, drag), его переключатель просто переносится с панели внутрь каталога как новый switch «Показывать скрытые на плане», доступный во всех вкладках. `Find` для скрытых/HA-disabled строк теперь доступен при включённом ghost-режиме (было — недоступен вовсе) | `docs/specs/029-device-inbox-lifecycle.md` §10.1 (новый абзац «Существующий локальный режим… сохраняется»), §10.2 п.6 (новый switch), §10.3 (столбец «Дополнительные действия» для трёх категорий — «Найти призрак при включённом ghost-режиме»), §10.4 (переписанное условие доступности `Find`), AC1 («Локальный ghost-toggle доступен внутри каталога, а его активность видна на кнопке «Устройства»»), AC4 (переписан), AC6 (добавлен абзац с явным описанием эквивалентности старому поведению), §18 (новая строка риска с защитой) | +| **M2** (Medium, в скоупе) — `marker.hide_tip` (ru/en) ссылался на удаляемую кнопку «Скрытые и деактивированные» / «Hidden and disabled», §14 не включал обновление существующих строк | §14 получил новый абзац: `marker.hide_tip` обновляется на обеих языках, чтобы вести в каталог «Устройства», плюс явное требование поиска прочих ссылок на старые названия кнопок перед реализацией | `docs/specs/029-device-inbox-lifecycle.md` §14, новый абзац сразу после списка тестируемых строк i18n | + +Обе находки закрыты по существу, а не декларативно: текст, который был +предметом претензии (удаление режима просмотра позиции; необновлённая +подсказка), в текущей редакции отсутствует, проверено чтением файла +целиком, а не поиском одной фразы. + +## Проверка дельты + +- **AC1** (golden + smoke): формулировка «активность видна на кнопке + «Устройства»» — проверяемое поведение (класс/атрибут кнопки для smoke, + визуальное состояние для golden), метод доказательства не изменился и + остаётся адекватным. +- **AC4** (unit): новая формулировка «Show недоступен, а Find доступен + только для призрака при включённом ghost-режиме» согласована с §10.3/§10.4 + той же дельты — противоречий нет. +- **AC6** (smoke): добавленный абзац описывает переключатель ghost-режима + как воспроизводящий текущее поведение 1:1 (позиция/выбор/настройка/drag, + без изменения `marker.hidden`, сброс при выходе из редактора) — это то же + утверждение, что и в §10.1, не новое обязательство. +- **§14** (i18n): новый пункт обязывает обновить `marker.hide_tip` в обоих + языках и найти прочие ссылки на старые названия кнопок. Точечно + перепроверено по коду (не только по тексту ТЗ): `src/i18n/ru.json:673` и + `src/i18n/en.json:673` — единственная пара строк с буквальной ссылкой на + удаляемую кнопку (тот же результат, что и в r1); `title.show_all` и + `devbar.show_all` и так входят в объём замены по §10.1 и отдельного + упоминания не требуют. +- **§18** (риски): новая строка «Перенос ghost-toggle делает активный режим + незаметным после закрытия каталога» ссылается на AC6 как на защиту, но + видимое active-state кнопки описано в AC1, а не в AC6 — см. находку L1 + ниже. + +Остальные AC (AC2, AC3, AC5, AC7–AC11) дельтой не задеты текстуально и не +зависят по доказательству от изменённых разделов — наследую вывод r1 без +повторной проверки (см. «Унаследовано»). + +## Находки + +### L1 (Low) — риск в §18 ссылается не на тот AC + +**Файл:** `docs/specs/029-device-inbox-lifecycle.md`, §18, строка риска +«Перенос ghost-toggle делает активный режим незаметным после закрытия +каталога». + +Защита названа как «Активное состояние кнопки «Устройства» и сброс при +выходе из редактора; AC6». Однако видимое active-state кнопки «Устройства» +описано в AC1 («его активность видна на кнопке «Устройства»»), а не в AC6 +(AC6 — про воспроизведение самого ghost-поведения: позиция/выбор/drag). +Сброс режима при выходе из редактора действительно упомянут в AC6 — +ссылка верна только для половины защиты. + +Не создаёт двусмысленности AC и не блокирует реализацию: оба AC +(AC1 и AC6) существуют, проверяемы и в сумме покрывают заявленную защиту — +перепутана только адресация в таблице рисков. + +**Решение ревьюера:** снимается без правки текста ТЗ. Основание — +PROCESS.md §2.4 «Low либо правится, либо снимается решением ревьюера с +записью»: неверная перекрёстная ссылка в информационной таблице рисков не +меняет ни один AC, не вводит в заблуждение исполнителя относительно того, +*что* защищает риск (только относительно того, *под каким номером* это +искать), и правка не стоит очередного цикла ревью на light-независимой +задаче с уже потраченным одним из четырёх циклов. + +## Унаследовано из r1 + +Документ: `docs/reviews/SPEC-REVIEW-29-r1.md`, SHA `a6ce1ae7e534fa4b41fcc0ab93e5ed95a693c4c1`. +Принято без повторной проверки в r2, поскольку дельта их не касается: + +- Соответствие `docs/SCOPE.md` (job J4/J6, персона Home admin, отсутствие + выхода за продуктовую рамку, `Не входит §6`). +- Формальная полнота §7.1 (все обязательные разделы присутствуют). +- AC1–AC11 однозначны, у каждого назван способ доказательства (кроме + точечных правок AC1/AC4/AC6, разобранных выше в этом раунде). +- Корректность отражения решения владельца по Q1 в §7.2 и AC3. +- Проверка фактических утверждений о текущем поведении кодом и каноном: + `_bindingCandidates`, `HaBindingStatus.kind`, `marker.hidden`/`marker.removed`, + `.slice(0, 200)` кап, `duplicate_name_area` устарела, фильтрация + `new_device_ids` от уже скрытых id, терминология «Добавить»/«Правила + иконок». +- §21 «Принятые технические предположения» — маркировка предположений + корректна и отделена от продуктовых решений (кроме пробела M1, который + в r2 закрыт не через §21, а прямым сохранением поведения — что делает + сам вопрос неактуальным: решать стало нечего, поведение не меняется). +- Существование тестовых артефактов: `test/devices.test.mjs`, + `test/ha-binding-status.test.mjs`, `test/device-presentation*.test.mjs`, + `demo/smoke_hidden_flag.mjs`, `demo/smoke_binding_picker.mjs`, + `scripts/smoke-select.mjs`. +- Откат (§19) реалистичен, обратно совместим. + +## Что проверено и признано корректным (в этом раунде) + +- M1 и M2 закрыты по существу — см. таблицу выше. +- Новый текст §10.1–§10.4 и AC1/AC4/AC6 внутренне непротиворечив: описание + условия доступности `Find` (§10.4) совпадает с формулировками в таблице + действий (§10.3) и в AC4. +- Новая формулировка `marker.hide_tip` (замена ссылки на кнопку) не создаёт + нового расхождения с `src/i18n/*.json` — единственная затронутая пара + строк подтверждена прямым чтением файлов. +- Старый (ошибочный) текст §10.1 «Скрытые markers больше не рисуются…» и + «устраняет состояние панели» полностью удалён из документа — проверено + `grep` по всему файлу, совпадений нет. +- Терминология не расходится с `docs/USER-GUIDE.ru.md`/`FILTERING.md`: + старое «Скрытые и деактивированные» (панель) и новое «Показывать скрытые + на плане» (switch внутри каталога) — разные элементы UI с разной + областью действия, противоречия нет; обновление USER-GUIDE уже стоит в + §20 Release-артефакты. + +## Чего не проверял + +- Не повторял полную сверку §7.1/AC1–AC11 с кодом — она выполнена в r1 и + дельта её не аннулирует (см. «Унаследовано»). +- Не запускал никакие гейты (`typecheck`/`test`/`build`/смоки/`check-docs`) — + на этапе ТЗ продуктового кода нет, диф не затрагивает `src/**`, гейты §8 + относятся к код-ревью. +- Не проверял `docs/ARCHITECTURE.md`/`docs/CANVAS.md` — дельта их не + касается. +- Не оценивал заново перформанс (§17 дельтой не затронут). + +## Вердикт + +High-находок нет. Обе Medium-находки r1 закрыты по существу и подтверждены +чтением текущего файла, а не заявлением автора. Единственная новая находка +этого раунда — Low, снята решением ревьюера с записью, без возврата на +правку. + +**Вердикт: зелёный.** ТЗ переходит в «Готово к разработке».