diff --git a/docs/reviews/CODE-REVIEW-618-r1.md b/docs/reviews/CODE-REVIEW-618-r1.md new file mode 100644 index 00000000..a3a5463a --- /dev/null +++ b/docs/reviews/CODE-REVIEW-618-r1.md @@ -0,0 +1,203 @@ +# CODE-REVIEW-618-r1 + +Issue: #618 — «Редактор устройств: массовые операции над скрытыми маркерами и +«показать всё скрытое»». Материал: `cb07f7fddfe9dc4e93226efe45754f5b6e2a5d7e` +(дерево совпадает с хендоффом `c386057a` — `6e096861de0baf0b2d8a29ed59e07c869ddf4be8`; +вершина переподписана владельцем без изменения дерева после починки +`workflow_sync` в `main`, см. комментарии issue). Трек **полный** (владелец +и разработчик согласны: критерий §5 «нет нового UX-контракта» нарушен — +множественный выбор и пакетное действие). Заход r1, блокирующих циклов +израсходовано 0 из 4. + +## Скоуп + +Каталог устройств (`src/device-inbox.ts`, новый `src/device-inbox-batch.ts`, +`src/houseplan-editor-runtime.ts`, `src/houseplan-card.ts` тип-зеркало, +`src/styles/dialogs.styles.ts`), i18n `en/ru/de/fr`, `docs/USER-GUIDE.md`, +`docs/USER-GUIDE.ru.md`, `docs/FILTERING.md`, оба changelog, смок +`demo/smoke_device_inbox_batch.mjs`, юниты `test/device-inbox.test.mjs`, +4 мутанта в `scripts/mutation-registry.mjs`, `scripts/bundle-budget.mjs` +(потолок lazy editor), `scripts/monolith-baseline.json`, сборка `dist/**` + +`custom_components/houseplan/frontend/**` (класс D, синхронизирована). + +Продукт: персона — администратор дома, десктоп, редактор устройств +(`docs/SCOPE.md`, job J6 «Keep the plan true as the home evolves»). View и +киоск не затронуты. Ничего из `docs/SCOPE.md` «Out of scope» не задето: ни +локов, ни автоматизаций, ни истории. Правило SCOPE.md «никогда не удалять +файл пользователя по догадке» не касается этой задачи (маркеры, не файлы). + +## Как проверялось + +Дёшевые гейты подтверждены зелёным Validate на этом SHA +(https://github.com/Matysh/houseplan-card/actions/runs/35957711230) — это +принято без повторного прогона согласно вводной. Все шесть шардов «Мутанты +по диффу» и «Фронтенд: типы, юниты, мутанты, синхрон бандла» зелёные; +`golden`/`smoke`/`performance_smoke` в этом прогоне `skipped` (не heavy-триггер: +без `Release:`, не PR, не `full=true` — ожидаемо по AGENTS.md). + +Дальше прогнано самостоятельно (Linux-песочница, Node 22, Chromium +установлен), поскольку Validate не покрывает браузерные смоки и golden на +обычном push: + +| Гейт | Команда | Результат | +|---|---|---| +| Типы | `npx tsc --noEmit` | rc=0 | +| Юниты | `npm test` | 2925 тестов: 2924 pass, 1 skip, 0 fail — совпадает с хендоффом | +| Сборка + синхрон бандла | `npm run bundle:sync` | rc=0; `git status` после — 0 расхождений с закоммиченным `dist/**`/`custom_components/houseplan/frontend/**` (побайтовое совпадение подтверждено фактическим отсутствием диффа, а не только скриптом) | +| Бюджет бандла | `npm run bundle:budget` | rc=0; `lazy editor: 245612 B gzip` (потолок 246600±2000); `initial View: 289754 B gzip` | +| Новый `any` | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | «Новых any нет» (319 строк, 5 файлов) — совпадает с хендоффом | +| Документация | `node scripts/check-docs.mjs --screenshots=warn` | passed (только унаследованный WARN о неснятых скриншотах — не по этой задаче) | +| Манифест смоков | `node scripts/check-inputs.mjs --coverage` | rc=0 | +| Мутанты AC2–AC5 (**не** `--check`, реальный запуск) | `node scripts/mutation-gate.mjs --id=device-inbox-batch-eligibility-active`, `--id=device-inbox-show-keeps-stub`, `--id=device-inbox-batch-single-write`, `--id=device-inbox-batch-rollback` | каждый: «поймано 1 из 1» — мутация действительно красит названный тест/смок в красный, независимо подтверждено, не только со слов автора | +| Новый смок | `node demo/smoke_device_inbox_batch.mjs` | все 31 assertion true, `OK` | +| Регрессия (названа автором) | `smoke_device_inbox`, `smoke_hidden_flag`, `smoke_disabled_device`, `smoke_discovery_filters`, `smoke_help_affordance` | все `OK` | +| `smoke-select.mjs --base origin/dev --head HEAD` | — | 22 прямых совпадения, 49 слабых; см. ниже — 4 прямых совпадения не были в списке автора, прогнаны отдельно | +| Дополненные прямые совпадения | `smoke_cover_not_primary`, `smoke_tap_run`, `smoke_toggle_confirmation`, `smoke_value_face_source` | все `OK` | +| `golden:verify` (полная матрица — частичный запуск инструмент запрещает: «verify must run the complete matrix») | `node demo/golden/run.mjs --mode=verify` | ровно 3 сценария `different`: `device-inbox-desktop-en-light`, `device-inbox-desktop-ru-dark`, `device-inbox-narrow-ru-dark` — ожидаемо (новые чекбоксы/панель), больше нигде расхождений; встроенная в harness проверка `body.scrollWidth > body.clientWidth + 1` (см. `demo/golden/harness.mjs:1750`) не бросила исключение ни на одном из трёх сценариев, включая узкий 390 px — тем самым независимо подтверждён именно тот гейт, который AC10 называет «чем краснеет», а не только смок-замена автора | + +**Чего не проверял** (и почему): +- 49 «слабых» совпадений `smoke-select` — решение ревьюера: сэмпл прямых + совпадений плюс регрессия автора покрывает затронутые модули + (`_deviceInbox*`, `_showToast`, `_saveConfigNow`); полная матрица — + предрелизный гейт (§8), не гейт ревью. +- `python -m pytest tests_backend` — Python не менялся (класс изменений не + затрагивает `custom_components/houseplan/**/*.py`). +- Perf-профиль — не назван в AC, влияния на View по спецификации нет + (§11 ТЗ), и код это подтверждает: пакетные операции живут только в + редакторе, View не тронут. +- Windows-toolchain — канон гейтов Linux CI/этот прогон. +- Реальный серверный конфликт ревизии — как и в спецификации (§15 «принято + предположительно»), имитирован `callWS` с `code: 'conflict'`; путь конфликта + (`_reloadConfigOnly`) — существующий код, не новый в этой задаче. + +## AC · доказательство · независимая проверка + +| AC | Утверждение | Проверено | Вердикт | +|---|---|---|---| +| AC1 | Пакет только на «На плане»/«Скрытые» | смок (перепрогнан) | ✓ | +| AC2 | Неактивный статус не выбирается, N без него, N учитывает «Показать ещё» | unit `test/device-inbox.test.mjs` (прочитан + прогнан в `npm test`) + смок | ✓; мутант `device-inbox-batch-eligibility-active` лично прогнан, красит | +| AC3 | Пакет = 1 запись `config/set` с `expected_rev`; 3→2 записи, 1 из троих остаётся | смок | ✓; мутант `device-inbox-batch-single-write` лично прогнан, красит | +| AC4 | Пакет = свёртка одиночных, заглушка сохраняется, новая заглушка с точным id, без дублей | unit (прочитан + прогнан) | ✓; мутант `device-inbox-show-keeps-stub` лично прогнан, красит | +| AC5 | Отказ (ошибка/конфликт): маркеры не меняются, `toast.error`, выбор сохранён, каталог активен | смок | ✓; мутант `device-inbox-batch-rollback` лично прогнан, красит | +| AC6 | Сброс выбора при вкладке/поиске/«Только новые», переживает диалог, очищается после успеха | смок | ✓ | +| AC7 | `inert` во время записи, тост с фактическим K, счётчики вкладок | смок | ✓ | +| AC8 | Выбор не пишет config/layout | смок | ✓ | +| AC9 | Ключи `en/ru/de/fr` без мёртвых | диф i18n прочитан построчно (9 ключей × 4 локали, точное соответствие §8 ТЗ) + `npm test` (i18n-parity/dead-keys в общем прогоне) | ✓ | +| AC10 | Нет переполнения 390px/десктоп, соседние смоки зелёные | смок (перепрогнан) + `golden:verify` лично: harness-проверка `scrollWidth` не бросила исключение | ✓ | +| AC11 | Документация | прочитаны `docs/USER-GUIDE.md`, `docs/USER-GUIDE.ru.md`, `docs/FILTERING.md`, оба changelog — терминология («Скрыть выбранные», «Показать выбранные», «Выбрать все (N)», тосты) совпадает с §6/§8 ТЗ и с реальными строками i18n | ✓ | + +## Что проверено и корректно (сверх таблицы AC) + +- **Один источник для одиночного и пакетного действия.** `_setInboxHidden` + теперь — тонкая обёртка (`src/houseplan-editor-runtime.ts:7095-7101`) над + `writeInboxVisibility(..., [row], hidden, false)`; ровно то, что заявлено + в §15 ТЗ («одиночное действие = пакет из одной строки»). Убедился построчным + диффом, что весь прежний код записи/откат/тост удалён, а не задублирован. +- **`applyInboxVisibility`** — честный левый фолд: каждая итерация читает + «живой» маркер из уже обновлённого `next`, не из исходного массива; + дедуп по `id` и по `binding` (кроме tombstone) исключает два живых маркера + на одну привязку. Юнит-тест `issue 618: a batch equals the fold…` явно + прогоняет это на «испорченном» конфиге с дублем и tombstone — не только на + чистых данных. +- **`markerIdForBinding`** для `device:`/`entity:` привязок — детерминированный + id (`ref` / `lg_`), `newId()` вызывается только в недостижимой для + реальных привязок ветке — риска коллизии `Date.now()` в одном пакете нет + (проверил чтением `src/logic.ts:805-814`, подтверждено юнитом с `newId`, + бросающим исключение при вызове). +- **Откат при отказе** переносит буквально прежнюю проверку идентичности + (`this.host._serverCfg === cfg`) — при конфликте `_saveConfigNow` уже вызвал + `_reloadConfigOnly()`, который подменяет `_serverCfg` новым объектом до + возврата в `catch`, поэтому откат корректно НЕ перетирает свежепрочитанный + сервером конфиг. Логика не новая — заимствована из старого + `_setInboxHidden` без изменения контракта. +- **Переживание вложенного диалога (B9)** — `_deviceInboxReturn` сохраняет + весь объект диалога (включая `selected`) и `finishMarkerDialogClose` + (`src/marker-dialog-close.ts`) восстанавливает его целиком; смок + `selectionSurvivesNestedDialog` это подтверждает, а не только текст ТЗ. +- **Стили**: панель `.device-inbox-batch` — отдельный контейнер вне + `.device-inbox-filters` (§15 риск про `smoke_hidden_flag`, беспокойство + подтверждено самим смоком `batchLivesOutsideFilters`); сетка строки на + узкой раскладке (`grid-template-columns: 24px 36px minmax(0, 1fr)`) + проверена смоком на 390px без переполнения. +- **i18n**: все 9 ключей присутствуют и идентичны по набору плейсхолдеров во + всех четырёх локалях (сверено построчно). +- **Трейлеры**: единственный коммит несёт `Issue: #618` и + `User-Visible: yes`; оба changelog отредактированы в этом же коммите + (`docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md` — подтверждено `git diff + --stat`). + +## Находки + +### Low (сняты с записью, без правки — числа не участвуют ни в одном гейте) + +1. **Число gzip лениво-редактора продублировано трижды с расхождением + в 2 байта.** Хендофф-комментарий в issue называет «замер 245 612»; + тот же факт в комментарии кода `scripts/bundle-budget.mjs:469` + («замер 245 610») и производные («990 Б сверху», «1010 Б до нижней + границы») — на 2 Б иначе, чем показывает реальный прогон + (`245612 B gzip`, что даёт 988 / 1012). Порог `LAZY_EDITOR_GZIP_CEILING = + 246_600` не зависит от этих комментариев — гейт `bundle:budget` считает + число заново при каждом запуске, так что несоответствие не может уронить + CI. Файл-источник: `scripts/bundle-budget.mjs:466-472`. +2. **Число initial View в тексте коммита не совпадает с хендоффом.** Коммит + `cb07f7fd` пишет «289 449 → 289 765 (+316 Б)»; комментарий в issue — + «289 449 → 289 754 (+305 Б)»; реальный `bundle:budget` показывает + `289754 B gzip` — хендофф прав, текст коммита на 11 Б мимо. Тоже не влияет + ни на один гейт (порог считается заново), только на читаемость истории. + +Оба случая — ровно тот тип «одно число, два источника», о котором просит +инструкция ревью (§8): факт (размер бандла после сборки) должен иметь один +источник правды — вывод `bundle-budget.mjs` в момент сборки, а не +переписанный от руки текст. Здесь источник один (сам скрипт при запуске), +разошедшиеся числа — это исключительно свободный текст (комментарий/коммит), +который не читается никаким гейтом и не может создать двух конкурирующих +источников для одного и того же решения. Снимаю без возврата автору: правки +кода не требуют, а правка текста уже опубликованного коммита запрещена +(§12 «никогда не переписывать опубликованную историю»). + +### High / Medium + +Не найдено. + +## Продуктовое рассуждение + +Изменение точно закрывает заявленный сценарий (§1–§3 ТЗ): администратор с +десятками скрытых устройств получает пакетные «Скрыть»/«Показать» с одной +записью конфига вместо N. Не ухудшает соседние сценарии: одиночные кнопки +не изменены по контракту (проверено — тот же путь записи, тот же список +доступных действий), вкладки «Доступны»/«Доступны снова» не получили +чекбоксов (как и требовалось), View/киоск не затронуты. Лок-инвариант +`docs/SCOPE.md` не касается этой задачи — ни locks, ни alarm panel в +`device-inbox` не участвуют. + +## Вердикт + +Все 11 AC доказаны — частью автотестами с личным подтверждением, что тест +умеет падать (4 мутанта AC2–AC5 лично прогнаны, каждый «поймано 1 из 1»), +частью прочитанным и исполненным кодом. Найдены только два Low несоответствия +в свободнотекстовых числах истории, которые не влияют ни на один гейт и +сняты без правки. Продуктовое рассуждение не выявило деградации соседних +сценариев. + +**Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0** + +## Материал раунда + +``` +tree 6e096861de0baf0b2d8a29ed59e07c869ddf4be8 +commit cb07f7fddfe9dc4e93226efe45754f5b6e2a5d7e +``` + +--- + + + +## Материал раунда + +- Ветка: `issue/618-inbox-batch`, коммит `cb07f7fddfe9` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `6e096861de0baf0b2d8a29ed59e07c869ddf4be8` + ``` + git log --all --format='%H %T' | grep 6e096861de0b + ``` +- Тело issue: `1febf156fc3f3a1ba3e0dcdb91ffc12851abf4061419c0df08d78f930d04db04` +- Вердикт конвейера: `green` · High 0 diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index cf4e6821..7e783fda 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,6 +1,6 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1035, issue: 365. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1036, issue: 365. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| @@ -47,6 +47,7 @@ | #620 | [CODE-REVIEW-620-r1.md](CODE-REVIEW-620-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` | +| #618 | [CODE-REVIEW-618-r1.md](CODE-REVIEW-618-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | 1. Число gzip лениво-редактора продублировано трижды с расхождением в 2 байта. Хендофф-…; / Medium | `scripts/bundle-budget.mjs` `bundle-budget.mjs` | | #617 | [SPEC-REVIEW-617-r1.md](SPEC-REVIEW-617-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | «новый необязательный параметр» уже существует | `src/backdrop-pick.ts` `houseplan-editor-runtime.ts` | | #617 | [CODE-REVIEW-617-r1.md](CODE-REVIEW-617-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #617 | [CODE-REVIEW-617-r2.md](CODE-REVIEW-617-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — |