Files
houseplan-card/docs/reviews/SPEC-REVIEW-301-r1.md
2026-08-25 08:55:50 +00:00

18 KiB
Raw Permalink Blame History

SPEC-REVIEW-301-r1

  • Issue: #301 — Add search/filter to entity selector for doors, windows and other openings
  • Трек: small (лёгкий) — ТЗ живёт в теле/комментарии issue, отдельного файла в docs/specs/ нет
  • Этап: ТЗ на ревью (PROCESS.md §2.4)
  • Заход: r1 · блокирующих циклов израсходовано (на входе) 0 из 2
  • Ревьюер: свежая сессия, без контекста автора
  • Материал: финальная редакция ТЗ в комментарии issue IC_kwDOTOcLQM8AAAABQjQZeg (после всех правок accept), код на dev@fd4fc801 (HEAD на момент ревью)

Скоуп ревью

ТЗ описывает замену нативного <select> в диалоге проёма (сенсор контакта и замок) на существующий UI-паттерн dropbtn/droppanel/candlist с текстовым поиском по friendly_name и entity_id. Изменение класса A, без изменения формата конфига и модели данных. Персона — home admin (docs/SCOPE.md), поверхность — Редактор плана, задача закрывает J4/J6 («editable icon rules», «keep the plan true as the home evolves» — оба про качество и скорость настройки проёмов). Конфликта со SCOPE.md нет: изменение чисто UI/UX, не трогает View, не добавляет поверхность актуации (замок по-прежнему только привязывается, а не переключается из этого диалога) — инвариант блокировки (SCOPE.md, «The lock invariant») не затронут.

Замечание по контексту захода: в комментариях issue уже есть четыре более ранних вердикта «жёлтый · заход r1», каждый со своими Medium-находками и последующим accept-исправлением автора (владелец явно отметил, что первый прогон конвейера не переставил метку, и последующие — по всей видимости, повторы того же сбоя, раз каждый снова маркирован «заход r1»). Поскольку эти прогоны не считаются циклами (метка не переходила), я не наследую их выводы на слово — ниже приведена независимая сверка того, что все находки этих прогонов закрыты в тексте, который сейчас фактически лежит в issue.

Как проверялось

Инструмент проверки — чтение, не исполнение (стадия ТЗ, кода ещё нет). Каждая фактическая ссылка ТЗ на код сверена построчно с src/houseplan-card.ts на dev@fd4fc801:

Ссылка ТЗ Файл:строка Проверено
opt() — нативный <select class="areasel"> без фильтра src/houseplan-card.ts:19463-19467 ✅ совпадает дословно
_contactCandidates() — binary_sensor + «дверные» cover, дверные device_class первыми, тип {value,label} без sub :12871-12886 ✅ подтверждено; сортировка `a[2]-b[2]
_lockCandidates() — lock.*, алфавитная сортировка, тип {value,label} без sub :12888-12893 ✅ совпадает
_runCandidates() — фильтруется на рендере (:20981-20983) без повторной сортировки списка кандидатов; кап на рендере — 40, не 200 (:20996) :13815-13829, :20980-21002 ✅ форма фильтрации подтверждена; кап 40 подтверждён — ТЗ корректно исключает это число из переиспользования (п.4)
_bindingCandidates() — безусловная пересортировка (filtered.sort(...) выполняется даже при пустом f), кап slice(0,200) :13832-13907, сортировка :13905, срез :13906 ✅ подтверждено: сортировка не зависит от наличия фильтра — ТЗ право, что этот паттерн «не подходит буквально»
_roomSrcCandidates() — тот же дефект (безусловная сортировка), тот же кап 200 :18703-18724, сортировка :18722, срез :18723 ✅ подтверждено
Паттерн dropbtn/droppanel/candlist, поля .cl/.cs для label/sub, отсутствие autofocus во всех трёх существующих droppanel (binding, run, roomSrc) :20868-20899 (binding), :20977-21003 (run), :21705-21727 (roomSrc) ✅ подтверждено; ни один инстанс не использует autofocus — решение ТЗ снять автофокус согласуется с реальным прецедентом, а не выдумано
_saveOpening() существует, пишет opening.contact/opening.lock без изменения формата :12751 и далее ✅ подтверждено (сигнатура и начало функции совпадают)
passage не имеет селектора контакта; замок — только door/gate :19512-19531 ✅ подтверждено построчно: d.type !== 'passage' для контакта, d.type === 'door' || d.type === 'gate' для замка
opening.none, marker.nothing_found, marker.search_ph (стиль «Поиск: …») — переиспользуемые строки src/i18n/en.json:142,178,177; ru.json те же ключи ✅ ключи существуют, значения соответствуют цитируемым
opening.search_ph — новый ключ src/i18n/{en,ru}.json ✅ ключа пока нет ни в одном файле — ожидаемо для стадии ТЗ, будет добавлен в реализации

Отдельно проверено (не на слово автора) закрытие находок предыдущих (незасчитанных) прогонов — таблица ниже.

Закрытие находок предыдущих (незасчитанных) прогонов

Находка прежнего прогона Чем закрыта в текущем тексте ТЗ Где видно
M1 (1-й прогон): AC6 противоречит образцу _bindingCandidates() (та безусловно пересортировывает) П.2 явно исключает _bindingCandidates()/_roomSrcCandidates() как образец именно из-за пересортировки и называет _runCandidates() образцом сохранения порядка; п.4 повторяет это тем же текстом П.2, абзац 2; п.4, предложение 2
M2 (1-й прогон): AC1–AC8 без пометки способа доказательства Каждый AC маркирован [unit]/[unit + smoke]/[smoke] Раздел 6, каждая строка
Low (1-й прогон): нет отдельной фразы «что человек увидит после» Отдельное предложение сразу после сценария: «Что человек увидит после: …» Раздел 1, абзац 2
M1 (2-й прогон): строка кандидата не решено, виден ли entity_id в candlist П.4.2, последнее предложение: «Каждая строка результата показывает friendly_name как основную подпись и полный entity_id (c.value) как вторичную» П.4.2
M2 (2-й прогон): нет заявления о производительности Раздел 9, абзац 1 Раздел 9
Low (2-й прогон): _roomSrcCandidates() не назван вторым «неподходящим буквально» образцом П.4, предложение 2: «_bindingCandidates()/_roomSrcCandidates()» (оба названы) П.4
M1 (3-й прогон): кап 200 заимствован из образца, который ТЗ же отвергло; принятый _runCandidates() капает на 40, а не 200 П.4, предложения 3–4: лимит 200 зафиксирован как «отдельное явное правило этого селектора», заимствован только как «безопасная верхняя граница»; лимит 40 явно исключён П.4
M2 (3-й прогон): автофокус ничем не обоснован, не подтверждён ни одним прецедентом П.4.2: «Поле получает фокус только после отдельного клика/тапа пользователя: открытие панели само не вызывает экранную клавиатуру» — автофокус снят П.4.2

Все восемь находок закрыты текстом, а не заявлением — сверено построчно с итоговой редакцией комментария и с кодом (таблица выше). Расхождений не найдено.

Проверка Acceptance Criteria (AC1–AC8)

Каждый AC однозначен, привязан к способу доказательства и опирается на реально существующий код (не на выдуманное поведение):

  • AC1–AC3, AC6 (фильтрация, регистр/пробелы, кап, сохранение порядка) — воспроизводимы как чистая функция; форма _runCandidates(), на которую они ссылаются, подтверждена построчно.
  • AC4, AC8 (пункт «нет», пустой результат) — используют реальные строки opening.none/marker.nothing_found, подтверждено.
  • AC5 — сформулирован через отсылку («ведёт себя так же, как контакт») вместо перечисления; в контексте small-трека и одного смока demo/smoke_opening_entity_search.mjs, который явно называет проверку и контакта, и замка в разделе 7, это не создаёт двусмысленности при написании теста.
  • AC7 — явно разграничивает «значение меняется» от «формат данных не меняется», что и есть предмет проверки.

Ни один AC не выдаёт догадку за решение: каждый либо констатирует наблюдаемое поведение, либо явно ссылается на переиспользуемый паттерн, который я сверил с кодом.

Находки

Low (не блокирует, снимаю с записью)

L1. Контракт п.4.1/4.2 требует показывать entity_id вторичной подписью в двух местах (закрытая кнопка dropbtn и каждая строка candlist), но ни один AC и ни один пункт плана тестов (раздел 7) явно не проверяет именно факт отображения — только факт того, что поиск находит по entity_id (AC2).

  • Файл: тело ТЗ (комментарий issue), разделы 4 и 6.
  • Сценарий отказа: реализация корректно фильтрует список по entity_id (AC2 зелёный), но по недосмотру не пробрасывает sub: c.value в шаблон candlist или в подпись dropbtn — пользователь по-прежнему не видит, почему найденная строка совпала, хотя именно это было явной причиной правки M1 второго прогона. Ни один автотест такую регрессию не поймает.
  • Почему не блокирую: это утверждение о видимом поведении, но не отдельный AC, поэтому его всё ещё можно закрыть на код-ревью через «проверено чтением, не исполнением» (PROCESS.md §2.7) — DoR требует пометки доказательства только у существующих AC, а не превращения каждого предложения контракта в отдельный AC. Свойство простое (одна строка кода в шаблоне на каждое место), а не поведенческая развилка.
  • Рекомендация код-ревьюеру следующего этапа: явно свериться чтением, что sub: c.value (или эквивалент) реально попадает в оба места рендера — закрытую кнопку и candlist — а не только используется в фильтрации.

Что проверено и корректно

  • Все фактические ссылки ТЗ на код (строки, функции, сигнатуры типов, i18n-ключи) — точны на dev@fd4fc801.
  • Выбор образца для фильтрации (_runCandidates() вместо _bindingCandidates()/_roomSrcCandidates()) обоснован верно: только _runCandidates() не пересортировывает список повторно.
  • Кап 200 и отказ от автофокуса — не голословные утверждения, обе оговорки явно зафиксированы как самостоятельные решения ТЗ (не «взято по умолчанию» из отвергнутого образца).
  • Не-скоуп (раздел 5) исключает изменение правил отбора кандидатов, других селекторов, ha-entity-picker, формата данных — не сужает и не расширяет задачу relative к issue.
  • DoR (PROCESS.md §2.5) выполнен по всем пунктам: AC пронумерованы с доказательством, файлы/модули названы по существу (пусть не единым списком — точечными ссылками на функции), i18n-ключи перечислены с текстом на обоих языках, миграции нет и это явно сказано, влияние на производительность и на touch названо, release-артефакты и откат описаны, открытых продуктовых вопросов к владельцу нет.
  • Терминология: пользовательские строки (opening.contact_label = «Датчик открытия», формат плейсхолдера «Поиск: …») согласуются с docs/USER-GUIDE.ru.md (раздел 9) и с существующим прецедентом marker.run_search_ph = «Поиск: автоматизация, скрипт или сцена…» — новый ключ не изобретает стиль.
  • Инвариант блокировки (docs/SCOPE.md, «The lock invariant») не затрагивается: изменение — только представление выбора привязки замка, не добавляет путь актуации.
  • Проверил закрытие всех 8 находок предыдущих (незасчитанных) прогонов по коду и тексту, а не на слово автора — таблица выше.

Чего не проверял

  • Код ещё не написан (стадия ТЗ) — гейты typecheck/test/build, смоки, golden, инварианты модели, мутационные гварды не запускались и запускаться на этой стадии не должны.
  • Не проверял реальную производительность (нет кода) — оценка в разделе 9 ТЗ принята как правдоподобная по аналогии с уже работающими _bindingCandidates()/_roomSrcCandidates() на том же порядке величин сущностей.
  • Не проверял docs/USER-GUIDE.ru.md на предмет точной будущей формулировки — ТЗ (light track) не обязано фиксировать точный текст документации, только факт правки.
  • Не проверял мутационные имена (opening-search-*) на синтаксическую валидность в scripts/mutation-gate.mjs — это тестовая инфраструктура, решается на этапе реализации, не на этапе ТЗ.

Вердикт

High: 0, Medium: 0, Low: 1 (снят с записью, см. L1). AC полны, однозначны, привязаны к доказательствам и опираются на подтверждённый код. DoR удовлетворён по всем пунктам. Продуктового конфликта со SCOPE.md, TOUCH-SUPPORT.md или инвариантом блокировки нет.

Зелёный.