docs: review document for #301

Issue: #301
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-25 08:33:33 +00:00
parent 65e55872e8
commit 6d094ca01b
+154 -125
View File
@@ -1,160 +1,189 @@
# SPEC-REVIEW — issue #301, заход r1
**Тема:** поиск/фильтр в селекторах контакта и замка диалога проёма (дверь/окно/ворота).
**Трек:** `small` — ТЗ в теле issue, ревью комментарием (этот документ прикладывается как файл ревьюера; в issue уходит краткий комментарий).
**Трек:** `small` — ТЗ в теле issue, ревью комментарием (этот документ — файл ревьюера; в issue уходит краткий комментарий).
**Заход:** r1 · блокирующих циклов израсходовано 0 из 2 (лимит лёгкого трека — 2).
**Материал:** тело issue #301 + комментарий-ТЗ от 2026-08-25 (Codex), код на `dev` @ `4b8f17ba`.
**Материал:** тело issue #301 + комментарий-ТЗ (Codex, редактирован 2026-08-25, текущая версия на момент этого ревью), код на `dev` @ `65e55872`.
## Контекст: почему это снова r1, а не r2
Автоматическое ревью уже отработало один раз (комментарий от `claude`,
2026-08-25T08:09:14Z, документ `docs/reviews/SPEC-REVIEW-301-r1.md`, коммит
`68498e52`, вердикт жёлтый, Medium: 2). Владелец зафиксировал сбой конвейера
*после* публикации вердикта: статусная метка не переставилась
(issue-комментарий 2026-08-25T05:14:19Z, прогон
`actions/runs/32811973025`). По правилу «после прогона ревью метка всегда
меняется; если не сменилась — упал сам прогон» (`AGENTS.md`) этот заход
считается не состоявшимся механически, и настоящий прогон переисполняет его
как r1, а не как r2 — с полным разбором, не по дельте (§2.10 применяется
начиная со второго *засчитанного* цикла).
Автор уже ответил на находки того захода правкой текста ТЗ (комментарий
2026-08-25T08:23:38Z: «M1 — accept, M2 — accept, Low — accept»). Ниже —
самостоятельная проверка текущего (уже отредактированного) ТЗ с нуля, а не
доверие к заявлению автора о зачёте. Совпадение с прежними находками там, где
оно есть, отмечено явно.
## Скоуп
Заменить нативный `<select class="areasel">` (локальный хелпер `opt()`) в диалоге
проёма для двух полей — контакта и замка — на существующий паттерн
`dropbtn`/`droppanel`/`candlist` с текстовым поиском, без изменения набора
кандидатов, их порядка (кроме как под фильтром) и формата конфига. Второй раунд
не открывается — это первый цикл, поэтому разбор полный, разделов «Закрытие r0»
и «Унаследовано из r0» нет.
Заменить нативный `<select class="areasel">` (локальный хелпер `opt()`,
`houseplan-card.ts:19463`) в диалоге проёма для двух полей — контакта и замка —
на существующий визуальный паттерн `dropbtn`/`droppanel`/`candlist` с текстовым
полем поиска, без изменения набора кандидатов, их базового порядка (кроме как
под фильтром) и формата конфига (`opening.contact`/`opening.lock`).
## Как проверялось
Ревью ТЗ на этом этапе — не код-ревью: кода нет, есть только текст ТЗ. Задача
ревьюера — проверить, что описанное реализуемо и не противоречит себе, и что
каждая ссылка на код/паттерн/строку i18n в тексте ТЗ соответствует
действительности (иначе автор строит решение на несуществующей опоре).
Прочитаны: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2.4/§2.10/§7.1/§7.2,
`docs/TOUCH-SUPPORT.md`, `docs/USER-GUIDE.ru.md` (раздел 9, «Настройки проёма»).
Каждая фактическая ссылка ТЗ сверена с текущим `src/houseplan-card.ts` на `dev`:
Кода нет — есть только текст ТЗ, поэтому это не код-ревью. Задача — проверить,
что описанное реализуемо, не противоречит себе и что каждая фактическая ссылка
на код/паттерн/строку i18n соответствует действительности (иначе автор строит
решение на несуществующей опоре). Прочитаны: `docs/SCOPE.md`, `AGENTS.md`,
`PROCESS.md` §2.4/§2.5/§2.10/§7.1/§7.2/§5, `docs/TOUCH-SUPPORT.md`,
`docs/USER-GUIDE.ru.md` (раздел «Настройки проёма»). Каждая фактическая ссылка
ТЗ сверена с текущим `src/houseplan-card.ts` на `dev`@`65e55872`:
| Ссылка ТЗ | Проверка | Результат |
|---|---|---|
| `opt()` — локальный select-хелпер, `:19463` | `grep`/чтение | подтверждено, строка 19463 |
| `_contactCandidates()` `:12871`, `_lockCandidates()` `:12888` | чтение | подтверждено, сигнатуры и сортировка как описаны |
| Паттерн `dropbtn`/`droppanel`/`candlist` у диалога маркера, фильтрация `:13901` | чтение | подтверждено — это `_bindingCandidates()` (13832–13905), рендер в 20868–20903 |
| Фильтр — подстрока по `label+sub+value`, регистронезависимый, кап 200 | чтение `_bindingCandidates()` | подтверждено (`filtered.slice(0, 200)`, строка ~13905) |
| `marker.nothing_found`, `opening.none`, `opening.contact_label`, `opening.lock_label`, `opening.invert`, `marker.search_ph` | `grep` в `src/i18n/{ru,en}.json` | все ключи существуют в обоих файлах |
| `_saveOpening` | чтение (`:12751`, вызов `:19557`) | подтверждено |
| `passage` не имеет селектора контакта | чтение блока рендера (`d.type !== 'passage'` guard, `:19519`) | подтверждено — блок контакта/замка целиком под условием |
| Лок-селектор только для door/gate | чтение (`d.type === 'door' || d.type === 'gate'`, `:19528`) | подтверждено |
| `scripts/mutation-gate.mjs`, `test-build` (`fix-test-build.mjs`) | `ls`/`grep package.json` | существуют |
| `opt(` используется только в контакте/замке (не задевает другой локальный `opt` в другом диалоге, `:21656`) | `grep` всех вызовов | подтверждено — разные локальные функции, конфликта нет |
| `opt()` — локальный select-хелпер, `:19463` | чтение | подтверждено (строка 19463, ровно тот блок) |
| `_contactCandidates()` `:12871`, дверные `device_class` первыми | чтение (12871–12886) | подтверждено — сортировка `doorish ? 0 : 1` перед алфавитом |
| `_lockCandidates()` `:12888` | чтение (12888–12892) | подтверждено — только `lock.*`, алфавитная сортировка |
| Контактный селектор скрыт для `passage` | чтение (guard `d.type !== 'passage'`, `:19519`) | подтверждено — весь блок контакта/замка под условием |
| Замковый селектор только для `door`/`gate` | чтение (guard `:19528`) | подтверждено |
| Паттерн `dropbtn`/`droppanel`/`candlist` существует | `grep dropbtn/droppanel` | подтверждено — **два** живых экземпляра: `_bindingCandidates()` (рендер 20868–20903) и `_roomSrcCandidates()` (рендер 21705–21728), не один, как можно понять из текста ТЗ |
| `_bindingCandidates()` пересортировывает безусловно даже без запроса | чтение (`:13901`, `filtered.sort(...)` вне ветки `f ?`) | подтверждено — ровно то, что называет ТЗ как причину отказа от этого образца |
| `_runCandidates()` не пересортировывает при фильтрации | чтение (сборка/сортировка один раз `:13815–13827`, фильтр без ресорта на рендере `:20981`) | подтверждено |
| Кап 200 «как у поисковых селекторов карточки» | чтение (`_bindingCandidates` `:13905`, `_roomSrcCandidates` `:18722`) | подтверждено — оба капают на 200. `_runCandidates()`, чей алгоритм фильтрации ТЗ просит взять за образец, сам капает иначе (`.slice(0, 40)`, `:20981`) — ТЗ берёт кап числом от одного образца, а форму фильтрации от другого; явных противоречий в тексте это не создаёт, но стоило назвать оба источника |
| `opening.none`, `opening.contact_label`, `opening.lock_label`, `opening.invert`, `marker.nothing_found` | `python -m json` по `src/i18n/{en,ru}.json` | все пять ключей существуют в обоих языках с ожидаемым смыслом |
| `opening.search_ph` — новый ключ | тот же разбор | подтверждено отсутствие — ТЗ корректно называет его новым |
| `_saveOpening` | чтение (`:12751`, вызов из футера диалога `:19557`) | подтверждено |
| `scripts/mutation-gate.mjs`, `scripts/fix-test-build.mjs` | `ls` | оба существуют |
| Именование смока `demo/smoke_opening_entity_search.mjs` | `ls demo/smoke_opening*.mjs` | согласуется с существующим рядом (`smoke_opening_binding.mjs`, `smoke_opening_measure.mjs`, `smoke_opening_preview.mjs`, …) |
| Раздел `docs/USER-GUIDE.ru.md` «Настройки проёма» существует и описывает контакт без поиска | чтение (строки 611–629) | подтверждено — там же явно сказано, из чего выбирается контакт; корректное место для правки release-артефакта |
Не проверялось (и не должно на этом этапе): код ещё не написан, поэтому
`typecheck`/`test`/`build`/смоки не прогонялись — предмета для них нет. Это не
пропуск гейта, а корректная граница этапа ТЗ.
Не проверялось (и не должно на этом этапе): кода нет, поэтому
`typecheck`/`test`/`build`/смоки/`check-docs`/инварианты модели не прогонялись —
предмета для них нет. Это граница этапа ТЗ, а не пропущенный гейт.
## Находки
### [Medium, в скоупе] AC6 противоречит явно указанному образцу для переиспользования
### [Medium, в скоупе] Контракт не решает, что показывает строка кандидата в открытой панели
П.4 «Контракт поведения» требует: «Наборы кандидатов и их порядок не меняются:
те же `_contactCandidates()` (дверные первыми)… тот же кап, что у биндинга
маркера (200)» — и AC6 требует: «Без запроса порядок прежний: дверные
device_class первыми».
П.4.1 фиксирует, что в **закрытом** состоянии кнопка `dropbtn` показывает
`friendly_name` **и** `entity_id` подписью. П.4.2 описывает открытую панель как
«поле поиска + список кандидатов», но не говорит, что видно в каждой строке
`candlist` — только `label`, или `label` плюс `entity_id` как подпись.
Но образец, на который ТЗ прямо ссылается в п.2 как на паттерн для
переиспользования — `_bindingCandidates()` (`houseplan-card.ts:13832`,
фильтрация на `:13901`) — устроен так:
Это не мелочь ради красоты: у двух реально существующих `candlist`-виджетов
в проекте поведение расходится. `_bindingCandidates()` показывает под именем
`sub` — модель устройства (`sub: dev.model || …`), а `_roomSrcCandidates()`
показывает под именем ровно `entity_id` (`sub: eid`, `houseplan-card.ts:18722`).
Тип, который возвращают `_contactCandidates()`/`_lockCandidates()`
(`{ value: string; label: string }[]`), вообще не содержит поля `sub` —
значит, без явного решения реализация с равной вероятностью повторит любой
из двух образцов или не покажет `entity_id` в строке вовсе.
```
const filtered = f ? list.filter(...) : list;
filtered.sort((a, b) => a.label.localeCompare(b.label)); // безусловно, даже без запроса (f === '')
return filtered.slice(0, 200);
```
Это напрямую задевает AC2 («фильтр находит и по `entity_id`: запрос
`binary_sensor.win` находит сущность, чьё имя не содержит `win`») по духу, а
не только по факту: AC2 останется технически проверяемым смоком без этого
решения (можно кликнуть единственный оставшийся кандидат и проверить
сохранённое значение), но пользователь, ради которого A2 существует, увидит
находку без объяснения, почему она совпала — ровно то отличие, которое делает
`entity_id` в подписи полезным, а не декоративным. Учитывая, что `value`
контакта/замка уже и есть исходный `entity_id` (в отличие от `_bindingCandidates`,
где `value` — это `device:id`/`entity:id` с префиксом), техническое решение
дешёвое: `sub: c.value`, без изменения сигнатур `_contactCandidates()`/
`_lockCandidates()`.
Сортировка по алфавиту применяется **безусловно**, независимо от того, пуст ли
запрос. Если реализация буквально повторит эту функцию для контакта/замка (что
и предлагает текст ТЗ, называя её образцом), результат без запроса окажется
отсортирован по алфавиту — а не «дверные первыми», как того явно требует AC6.
Второй существующий в проекте образец, `_runCandidates()` + рендер `:20981`,
устроен иначе: фильтр применяется **в момент рендера** над уже готовым
(единожды отсортированным) списком, без повторной пересортировки внутри
фильтра — именно эта форма сохраняет исходный порядок при пустом запросе.
**Что нужно поправить:** одно предложение в п.4.2 — какая из двух форм строки
берётся (рекомендация: подпись `entity_id` под именем, по образцу
`_roomSrcCandidates()`, поскольку именно `entity_id`, а не модель устройства,
и есть второй канал поиска по AC2).
ТЗ называет ровно тот образец, который ломает собственный AC6, и не
оговаривает, что реализация должна взять форму без пересортировки. Это не
продуктовый вопрос (человек как раз получает однозначное требование в AC6) —
это техническое противоречие внутри самого текста, которое реализация,
написанная «по образцу из п.2», воспроизведёт как дефект. Автор решает это
сам (`AGENTS.md`: то, чего пользователь не наблюдает как решение — можно
принять любым способом), но должен явно записать выбор, а не оставить два
взаимоисключающих образца рядом.
**Как воспроизвести неоднозначность:** взять текст ТЗ буквально и попросить
двух разных исполнителей написать рендер строки кандидата — получится два
разных, оба «по образцу из ТЗ».
**Что нужно поправить:** одно предложение в п.4 или блок «принято
предположительно»: новая функция фильтрации контакта/замка не должна
безусловно пересортировывать список — она либо фильтрует уже отсортированный
`_contactCandidates()`/`_lockCandidates()` без повторного `.sort()` (модель
`_runCandidates()`), либо сортирует **только** непустой результат фильтра,
оставляя пустой запрос как есть.
### [Medium, в скоупе] DoR требует явного заявления о влиянии на производительность — в ТЗ его нет
### [Medium, в скоупе] AC-раздел не указывает способ доказательства по каждому пункту
`PROCESS.md` §2.5 перечисляет обязательные пункты «Готово к разработке»,
включая «влияние на производительность и бюджеты названо (или явно «нет»)» —
и это пункт из категории «все обязательны», трек `small` упрощает форму ТЗ
(§5: «проблема · контракт · AC1…ACn с доказательством · откат»), но не снимает
требований DoR при выходе из ревью в `S5-ready`.
DoR (`PROCESS.md` §2.5) требует у каждого AC явно названный способ
доказательства: `unit`/`backend`/`smoke`/`golden`/«ревью кода». Раздел 6
перечисляет AC1…AC8 без этой пометки; раздел 7 «План тестов» описывает тесты
общо (юнит на функцию фильтрации, один смок, мутанты), но не привязывает
явно, какой AC каким пунктом доказывается. Большинство связей угадывается
(AC1–AC4, AC6, AC8 → юнит на чистую функцию фильтрации; AC5 → тот же юнит для
замка; AC7 → смок/проверка сохранённого конфига), но это должно быть написано,
а не восстановлено ревьюером — иначе на DoR это будет заново всплывать как
«не готово».
В тексте раздела 9 («Риски и откат») и раздела 8 («Release-артефакты») нет ни
слова о производительности — ни явного «нет», ни оценки. Мотивирующий сценарий
issue («сотни `binary_sensor` и `cover`») — это ровно тот случай, где
«очевидно, что дешёво» стоило написать явно: фильтрация подстрокой на каждое
нажатие клавиши без debounce по массиву в несколько сотен записей — тот же
порядок величины, что уже фильтруют `_bindingCandidates()`/`_roomSrcCandidates()`
сегодня без debounce, поэтому риска здесь по факту нет, но раздел, где это
следовало сказать, пуст.
**Что нужно поправить:** приписать к каждому AC1…AC8 в скобках способ
доказательства, например `AC1 (unit)`, `AC7 (smoke: demo/smoke_opening_entity_search.mjs)`.
**Что нужно поправить:** одна строка в разделе 9, например: «Производительность:
фильтрация подстрокой по тому же порядку кандидатов (десятки–сотни записей),
что уже обрабатывают `_bindingCandidates()`/`_roomSrcCandidates()` без debounce
— нового узкого места не создаёт».
### [Low] «Что человек увидит до и после» не выделено отдельным предложением
### [Low, к сведению] Второй живой образец `dropbtn`/`droppanel` не назван
§7.1 требует отдельно от «сценария» одну фразу без терминов реализации о том,
что человек видит до/после. Раздел 1 ТЗ смешивает сценарий, причину и намёк на
результат («минута прокрутки на каждый проём» — это «до»; отдельного «после»
нет, оно восстанавливается из AC). Не блокирует — не блокирую, так как раздел
6 (AC) фактически описывает «после» покритериально, но при правке по Medium-
находкам выше стоит добавить одну строку явно, чтобы не тратить время
следующего ревью на восстановление того же вывода.
Текст ТЗ (раздел 2) называет ровно один образец, который «не подходит
буквально» — `_bindingCandidates()`. Но безусловно пересортировывает список и
второй существующий `candlist`-виджет с идентичной разметкой,
`_roomSrcCandidates()` (`houseplan-card.ts:13901` там нет, ресорт на
`:18722`, вне ветки `q ?`, — то есть тот же дефект «сортирует, даже если запрос
пуст»). Вывод ТЗ (взять форму фильтрации `_runCandidates()`) от этого не
меняется — он верен для обоих контрпримеров, — но перечисление только одного
из двух даёт читателю неполную картину «почему». Не блокирует, снимаю
записью: автор может поправить текст или оставить как есть — вывод не
меняется независимо от того, назван ли один контрпример или оба.
## Что проверено и корректно
## Проверено и корректно
- Персона и job по `docs/SCOPE.md`: единственный, кто видит диалог проёма — Home
admin в Редакторе плана (десктоп-first поверхность); job — J4/J6 («держать
план актуальным», «GUI без внешних инструментов») — усложнение поиска входит
в тот же job, что и сам диалог. В скоуп попадает.
- Конфликта со `SCOPE.md` не найдено: не расширяет функциональность за пределы
UI выбора существующего значения, формат конфига не меняется (§4 п.6),
подтверждено — свойства `opening.contact`/`opening.lock` остаются как есть в
`OpeningCfg`.
- `passage` действительно не имеет и не получит контакта — подтверждено кодом,
а не только текстом ТЗ.
- Touch: заявлено «поддерживается» — обоснованно, потому что переиспользуемый
паттерн (`droppanel` с текстовым полем) уже несёт ту же тач-поверхность в
диалоге маркера; отдельного тач-риска не вносится, `TOUCH-SUPPORT.md` не
требует нового смока для точечного расширения существующего паттерна.
- i18n-план (7 существующих ключей + 1 новый, `opening.search_ph`, EN+RU) —
корректен, ни один из перечисленных существующих ключей не «придуман».
- «Не входит» — явный и обоснованный (правила отбора кандидатов, `ha-entity-picker`,
прочие селекторы вне диалога проёма).
- Откат — «одним revert, конфиг не меняется» — верно, чисто UI-правка, ничего
персистентного не создаётся.
- Release-артефакты (оба changelog, строка в `USER-GUIDE.ru.md`, скриншоты по
требованию `check-docs`) названы верно и по формату AGENTS.md/User-Visible: yes.
- Единственное число — источник: фича не добавляет никакой новой видимой
пользователю величины (только текстовый фильтр над существующим списком),
`test/single-source-numbers.test.mjs` не затрагивается по природе задачи.
- Все фактические ссылки ТЗ на код и i18n подтверждены (таблица выше) —
решение не построено на несуществующей опоре.
- **M1 предыдущего (сбойного) захода закрыт по существу.** ТЗ теперь явно
называет форму фильтрации без пересортировки (`_runCandidates()`) и явно
объясняет, почему буквальное копирование `_bindingCandidates()` сломало бы
AC6 (безусловный `.sort()`). Я перепроверил это заявление по коду независимо
(см. таблицу) — оно точное, а не пересказ автора.
- **M2 предыдущего захода закрыт.** Каждый AC1…AC8 несёт пометку `unit`
и/или `smoke`; раздел 7 (план тестов) соответствует этим пометкам по
содержанию (чистая функция фильтрации для unit, реальный диалог для smoke).
- **Low предыдущего захода закрыт.** Раздел 1 содержит отдельное предложение
«Что человек увидит после» без терминов реализации.
- Скоуп укладывается в `docs/SCOPE.md` (J4/J6 — GUI-настройка проёмов,
поддержание актуальности плана на больших инсталляциях), без конфликта с
«View mode is the product»: правка целиком внутри редактора плана, ничего
не протекает в Просмотр.
- Формат конфига не меняется (`opening.contact`/`opening.lock` — те же поля,
раздел 5 явно исключает изменение модели), поэтому `CONFIG-COMPATIBILITY.md`
не затрагивается — миграция не нужна, и ТЗ это не придумывает, а выводит из
раздела «Не входит».
- Touch: диалог общий для всех вводов, новый виджет — воспроизведение уже
работающего на touch паттерна (`_bindingCandidates`/`_roomSrcCandidates`
используют тот же `dropbtn`/`droppanel` без специальной touch-деградации) —
утверждение «поддерживается» не противоречит `docs/TOUCH-SUPPORT.md`
(best-effort редактора, без нового регресса).
- AC1–AC8 однозначны и проверяемы по отдельности; ни один не выдаёт догадку за
факт — там, где текст опирается на конкретный код (образец фильтрации,
наличие i18n-ключей), это подтверждено чтением, а не заявлено на веру.
- Release-артефакты названы верно: `docs/USER-GUIDE.ru.md` действительно имеет
подходящий раздел («Настройки проёма», строки 611–629) с описанием текущего
(безпоискового) выбора контакта — естественное место для правки.
## Чего не проверял
- Код не написан — не прогонялись `typecheck`/`test`/`build`/`check-docs`/смоки:
предмета нет, это этап ТЗ, а не код-ревью.
- Не проверял поведение `hp-dropdown`-паттерна на реальном тач-устройстве —
ссылка на «уже работает в диалоге маркера» принята по чтению кода и по
отсутствию отдельного тач-дефекта в истории по этому паттерну, не по живому
прогону.
- Не оценивал производительность на «сотнях» сущностей — паттерн уже несёт кап
200 и используется на маркерах с сопоставимым объёмом сущностей; отдельного
риска не вижу, но не измерял.
- Реальную реализацию — её нет, это стадия ТЗ.
- `typecheck`/`test`/`build`/`check-docs`/смоки/инварианты модели — неприменимо
без кода; это не пропуск гейта, а корректная граница этапа.
- Не сверял поведение `_openingEntityAvailable()` (фильтр доступности сущностей
внутри `_contactCandidates`/`_lockCandidates`) построчно — оно не меняется
этим ТЗ и не упомянуто как предмет изменения, вне скоупа проверки ссылок.
## Вывод
## Вердикт
Изменение попадает в скоуп продукта (job J4/J6), ТЗ методологически полное
(все обязательные разделы присутствуют по существу), но содержит техническое
противоречие между заявленным образцом для переиспользования и собственным
AC6, плюс отсутствие явной привязки AC → способ доказательства. High-находок
нет. Оба Medium — в скоупе задачи, решаются правкой текста ТЗ (не кода), без
нового issue. Вердикт — жёлтый, возврат автору на правку текста.
Жёлтый. High: 0, Medium: 2 (оба в скоупе — правятся текстом ТЗ, без нового
issue), Low: 1 (к сведению, не блокирует). Заход r1 (переисполнение сбойного
автоматического прогона), блокирующих циклов израсходовано 0 из 2 до этого
вердикта.