mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-07 15:09:30 +00:00
@@ -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_<ref>`), `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
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/618-inbox-batch`, коммит `cb07f7fddfe9` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `6e096861de0baf0b2d8a29ed59e07c869ddf4be8`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 6e096861de0b
|
||||
```
|
||||
- Тело issue: `1febf156fc3f3a1ba3e0dcdb91ffc12851abf4061419c0df08d78f930d04db04`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
@@ -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<ref> в 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 | — | — |
|
||||
|
||||
Reference in New Issue
Block a user