mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,160 @@
|
||||
# SPEC-REVIEW — issue #301, заход r1
|
||||
|
||||
**Тема:** поиск/фильтр в селекторах контакта и замка диалога проёма (дверь/окно/ворота).
|
||||
**Трек:** `small` — ТЗ в теле issue, ревью комментарием (этот документ прикладывается как файл ревьюера; в issue уходит краткий комментарий).
|
||||
**Заход:** r1 · блокирующих циклов израсходовано 0 из 2 (лимит лёгкого трека — 2).
|
||||
**Материал:** тело issue #301 + комментарий-ТЗ от 2026-08-25 (Codex), код на `dev` @ `4b8f17ba`.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Заменить нативный `<select class="areasel">` (локальный хелпер `opt()`) в диалоге
|
||||
проёма для двух полей — контакта и замка — на существующий паттерн
|
||||
`dropbtn`/`droppanel`/`candlist` с текстовым поиском, без изменения набора
|
||||
кандидатов, их порядка (кроме как под фильтром) и формата конфига. Второй раунд
|
||||
не открывается — это первый цикл, поэтому разбор полный, разделов «Закрытие r0»
|
||||
и «Унаследовано из r0» нет.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью ТЗ на этом этапе — не код-ревью: кода нет, есть только текст ТЗ. Задача
|
||||
ревьюера — проверить, что описанное реализуемо и не противоречит себе, и что
|
||||
каждая ссылка на код/паттерн/строку 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`:
|
||||
|
||||
| Ссылка ТЗ | Проверка | Результат |
|
||||
|---|---|---|
|
||||
| `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` всех вызовов | подтверждено — разные локальные функции, конфликта нет |
|
||||
|
||||
Не проверялось (и не должно на этом этапе): код ещё не написан, поэтому
|
||||
`typecheck`/`test`/`build`/смоки не прогонялись — предмета для них нет. Это не
|
||||
пропуск гейта, а корректная граница этапа ТЗ.
|
||||
|
||||
## Находки
|
||||
|
||||
### [Medium, в скоупе] AC6 противоречит явно указанному образцу для переиспользования
|
||||
|
||||
П.4 «Контракт поведения» требует: «Наборы кандидатов и их порядок не меняются:
|
||||
те же `_contactCandidates()` (дверные первыми)… тот же кап, что у биндинга
|
||||
маркера (200)» — и AC6 требует: «Без запроса порядок прежний: дверные
|
||||
device_class первыми».
|
||||
|
||||
Но образец, на который ТЗ прямо ссылается в п.2 как на паттерн для
|
||||
переиспользования — `_bindingCandidates()` (`houseplan-card.ts:13832`,
|
||||
фильтрация на `:13901`) — устроен так:
|
||||
|
||||
```
|
||||
const filtered = f ? list.filter(...) : list;
|
||||
filtered.sort((a, b) => a.label.localeCompare(b.label)); // безусловно, даже без запроса (f === '')
|
||||
return filtered.slice(0, 200);
|
||||
```
|
||||
|
||||
Сортировка по алфавиту применяется **безусловно**, независимо от того, пуст ли
|
||||
запрос. Если реализация буквально повторит эту функцию для контакта/замка (что
|
||||
и предлагает текст ТЗ, называя её образцом), результат без запроса окажется
|
||||
отсортирован по алфавиту — а не «дверные первыми», как того явно требует AC6.
|
||||
Второй существующий в проекте образец, `_runCandidates()` + рендер `:20981`,
|
||||
устроен иначе: фильтр применяется **в момент рендера** над уже готовым
|
||||
(единожды отсортированным) списком, без повторной пересортировки внутри
|
||||
фильтра — именно эта форма сохраняет исходный порядок при пустом запросе.
|
||||
|
||||
ТЗ называет ровно тот образец, который ломает собственный AC6, и не
|
||||
оговаривает, что реализация должна взять форму без пересортировки. Это не
|
||||
продуктовый вопрос (человек как раз получает однозначное требование в AC6) —
|
||||
это техническое противоречие внутри самого текста, которое реализация,
|
||||
написанная «по образцу из п.2», воспроизведёт как дефект. Автор решает это
|
||||
сам (`AGENTS.md`: то, чего пользователь не наблюдает как решение — можно
|
||||
принять любым способом), но должен явно записать выбор, а не оставить два
|
||||
взаимоисключающих образца рядом.
|
||||
|
||||
**Что нужно поправить:** одно предложение в п.4 или блок «принято
|
||||
предположительно»: новая функция фильтрации контакта/замка не должна
|
||||
безусловно пересортировывать список — она либо фильтрует уже отсортированный
|
||||
`_contactCandidates()`/`_lockCandidates()` без повторного `.sort()` (модель
|
||||
`_runCandidates()`), либо сортирует **только** непустой результат фильтра,
|
||||
оставляя пустой запрос как есть.
|
||||
|
||||
### [Medium, в скоупе] AC-раздел не указывает способ доказательства по каждому пункту
|
||||
|
||||
DoR (`PROCESS.md` §2.5) требует у каждого AC явно названный способ
|
||||
доказательства: `unit`/`backend`/`smoke`/`golden`/«ревью кода». Раздел 6
|
||||
перечисляет AC1…AC8 без этой пометки; раздел 7 «План тестов» описывает тесты
|
||||
общо (юнит на функцию фильтрации, один смок, мутанты), но не привязывает
|
||||
явно, какой AC каким пунктом доказывается. Большинство связей угадывается
|
||||
(AC1–AC4, AC6, AC8 → юнит на чистую функцию фильтрации; AC5 → тот же юнит для
|
||||
замка; AC7 → смок/проверка сохранённого конфига), но это должно быть написано,
|
||||
а не восстановлено ревьюером — иначе на DoR это будет заново всплывать как
|
||||
«не готово».
|
||||
|
||||
**Что нужно поправить:** приписать к каждому AC1…AC8 в скобках способ
|
||||
доказательства, например `AC1 (unit)`, `AC7 (smoke: demo/smoke_opening_entity_search.mjs)`.
|
||||
|
||||
### [Low] «Что человек увидит до и после» не выделено отдельным предложением
|
||||
|
||||
§7.1 требует отдельно от «сценария» одну фразу без терминов реализации о том,
|
||||
что человек видит до/после. Раздел 1 ТЗ смешивает сценарий, причину и намёк на
|
||||
результат («минута прокрутки на каждый проём» — это «до»; отдельного «после»
|
||||
нет, оно восстанавливается из AC). Не блокирует — не блокирую, так как раздел
|
||||
6 (AC) фактически описывает «после» покритериально, но при правке по Medium-
|
||||
находкам выше стоит добавить одну строку явно, чтобы не тратить время
|
||||
следующего ревью на восстановление того же вывода.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Персона и 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` не затрагивается по природе задачи.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Код не написан — не прогонялись `typecheck`/`test`/`build`/`check-docs`/смоки:
|
||||
предмета нет, это этап ТЗ, а не код-ревью.
|
||||
- Не проверял поведение `hp-dropdown`-паттерна на реальном тач-устройстве —
|
||||
ссылка на «уже работает в диалоге маркера» принята по чтению кода и по
|
||||
отсутствию отдельного тач-дефекта в истории по этому паттерну, не по живому
|
||||
прогону.
|
||||
- Не оценивал производительность на «сотнях» сущностей — паттерн уже несёт кап
|
||||
200 и используется на маркерах с сопоставимым объёмом сущностей; отдельного
|
||||
риска не вижу, но не измерял.
|
||||
|
||||
## Вывод
|
||||
|
||||
Изменение попадает в скоуп продукта (job J4/J6), ТЗ методологически полное
|
||||
(все обязательные разделы присутствуют по существу), но содержит техническое
|
||||
противоречие между заявленным образцом для переиспользования и собственным
|
||||
AC6, плюс отсутствие явной привязки AC → способ доказательства. High-находок
|
||||
нет. Оба Medium — в скоупе задачи, решаются правкой текста ТЗ (не кода), без
|
||||
нового issue. Вердикт — жёлтый, возврат автору на правку текста.
|
||||
Reference in New Issue
Block a user