mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,190 @@
|
||||
# SPEC-REVIEW-44-r2
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/44
|
||||
- Этап: spec (ревью ТЗ, PROCESS.md §2.4)
|
||||
- Заход: r2 · блокирующих циклов израсходовано 1/4 (потрачен на r1, красный)
|
||||
- ТЗ: `docs/specs/044-filter-grouping-policy.md`, ревизия 3, зафиксирована
|
||||
коммитом `ba568763` («docs: #44 spec revision 3 per SPEC-REVIEW-44-r1»,
|
||||
единственный файл, 34 добавлено / 16 удалено — `git show --stat ba568763`)
|
||||
- Предыдущий раунд: `docs/reviews/SPEC-REVIEW-44-r1.md`, вердикт **красный**,
|
||||
получен на ревизии 2, коммит `3d4c5090`
|
||||
- Вердикт: **жёлтый**
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Разбор по дельте (PROCESS.md §2.9): дельта локальна — один файл ТЗ,
|
||||
изменения сосредоточены ровно вокруг двух High из r1 (H1, H2), новых
|
||||
подсистем не задето, продуктовый код не менялся ни разу с r1 (только
|
||||
докс-коммиты `df1dbccf`, `ba568763`). Полный повторный разбор всего документа
|
||||
не требуется; передельчиваю только то, что задевает дельта, и то, что дельта
|
||||
могла сломать по соседству (см. «Унаследовано» ниже за границей проверки).
|
||||
|
||||
Дельта: `git diff 3d4c5090..ba568763 -- docs/specs/044-filter-grouping-policy.md`
|
||||
— 34/16 строк, три места: заголовок ревизии, «Что человек увидит»/раздел
|
||||
«Скоуп → 2» (H1), Контракт п.1a + AC4b + План автотестов + Риски (H2).
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта в ревизии 3 | Где это видно |
|
||||
|---|---|---|
|
||||
| H1 — сценарий/AC4 утверждали, что причина `excluded_integration` сегодня не существует и появляется на «Скрытых»; факт неверен, категория `hidden` структурно недостижима для такого кандидата | Раздел «Что человек увидит» и «Скоуп → 2» переписаны: причина уже существует и рендерится на «Доступны» (#29); скоуп задачи сведён к добавлению плейсхолдера `{integration}` в текст, вкладка/значение reason не меняются, перенос категории явно объявлен нерешаемым здесь вопросом. AC4 переписан под факт | `docs/specs/044-filter-grouping-policy.md:23-27, 70-78, 138-140`; факты перепроверены чтением кода — совпадают (`houseplan-editor-runtime.ts:7637`, `src/i18n/ru.json:484`, `row.integration` уже в структуре строки, `houseplan-editor-runtime.ts:11842`) |
|
||||
| H2 — `roomClimateMap` хардкодит `EXCLUDED_DOMAINS` в обход настраиваемого резолвера; Контракт №3 («один источник для всех потребителей») был неверен | Добавлен пункт контракта 1a: `roomClimateMap` переводится на тот же резолвер, что и discovery; явный climate-opt-in (`optClimate`) остаётся сильнее — ветка не трогается; добавлен AC4b (юнит) и регресс-риск | `docs/specs/044-filter-grouping-policy.md:99-105, 141-144, 173-175`; проверено чтением: `optClimate` действительно существующая ветка (`devices.ts:1488-1492`), вызовы `roomClimateMap` в трёх местах (`devices.ts:1546`, `houseplan-card.ts:11870`, `space-render.ts:301`) и существующие тесты #317 (`test/devices.test.mjs:1304-1426`) зовут функцию без нового параметра — расширение сигнатуры опциональным параметром не сломает эти вызовы, что подтверждает реалистичность «default-путь байт-в-байт» из AC4b |
|
||||
| L1 — термин «Доступные» вместо канонического «Доступны» (Low, правится при следующей правке текста, отдельного цикла не требовал) | Не исправлено единообразно | См. ниже, «Находки → L1 (сохраняется)» |
|
||||
|
||||
H1 и H2 закрыты по существу: факты исправлены, новый контракт/AC технически
|
||||
реализуем (проверено чтением кода, а не на слово автора). Но правка H1
|
||||
задела не весь документ — два места вне тронутого диапазона остались
|
||||
буквально с прежним (уже опровергнутым) утверждением. Это отдельная находка
|
||||
этого раунда, см. ниже.
|
||||
|
||||
## Находки
|
||||
|
||||
### M1 — Ревизия 3 не убрала два места, где документ прямо утверждает то, что H1 уже опроверг: причина будто появляется на «Скрытых»
|
||||
|
||||
**Файл:** `docs/specs/044-filter-grouping-policy.md`, «Риски» (строка 170) и
|
||||
«Принятые предположения» (строка 201).
|
||||
|
||||
**Что в документе сейчас:**
|
||||
- Строка 170 (не менялась при правке H1): «Реason-вкладка «Скрытые»
|
||||
получает новый класс записей — объём ограничен существующим механизмом
|
||||
вкладки; смок проверяет отсутствие дублей.»
|
||||
- Строка 201 (не менялась при правке H1): «Причина показывается на
|
||||
«Скрытых», НЕ в отдельной новой вкладке.»
|
||||
|
||||
**Почему это противоречие, а не стилистика:** ровно эта пара утверждений —
|
||||
причина оседает на «Скрытых» — и была фактической ошибкой H1 в r1
|
||||
(строка 183 ревизии 2, процитирована в SPEC-REVIEW-44-r1.md как то, что
|
||||
«маскирует» техническое противоречие). Ревизия 3 переписала «Что человек
|
||||
увидит», «Скоуп → 2» и AC4 так, что причина теперь однозначно остаётся на
|
||||
«Доступны» (строки 72-78, 138-140: «вкладка НЕ меняется … перенос категории
|
||||
… здесь не открывается»). Но правка не дотянулась до «Риски» и «Принятые
|
||||
предположения» — там буквально сохранился текст, отрицающий собственное
|
||||
исправление документа. Это не гипотетический риск: раздел «Риски» ссылается
|
||||
на несуществующий по факту «новый класс записей» на «Скрытых» и требует от
|
||||
смока проверки «отсутствия дублей» там, где по исправленному контракту
|
||||
вообще ничего нового не появляется; раздел «Принятые предположения»
|
||||
формулирует как «предположение» то, что теперь прямо противоречит принятому
|
||||
(и проверенному чтением) решению.
|
||||
|
||||
**Кому это мешает:** реализатор или автор смока, читающий «Риски»/
|
||||
«Предположения» отдельно от «Скоупа» (частый паттерн — эти разделы короче и
|
||||
их пробегают первыми), получит указание строить или проверять поведение
|
||||
вкладки «Скрытые», которого сама задача явно не создаёт. Ложное срабатывание
|
||||
дублей на «Скрытых» либо реализация лишней ветки категоризации — именно тот
|
||||
класс расхождения, который сделал r1 красным.
|
||||
|
||||
**Почему Medium, не High:** сами AC (AC4, AC4b) и нормативный «Контракт
|
||||
поведения» однозначны и не зависят от этих двух абзацев — блокирующей
|
||||
невыполнимости AC нет, в отличие от r1, где AC4 была буквально невыполнима
|
||||
как написана. Дефект — во внутренней непротиворечивости документа, задача
|
||||
которого — заново подтверждённые факты (H1). Это находка в скоупе ревизии,
|
||||
которую делает именно этот раунд (H1-правка), поэтому чинится тем же
|
||||
циклом, без отдельного issue (PROCESS.md, решение 2026-08-19, #202).
|
||||
|
||||
**Что нужно поправить:** строку 170 переписать/удалить (никакого нового
|
||||
класса записей на «Скрытых» нет — риск, который стоит сформулировать вместо
|
||||
неё, если он есть: неверно подставленный `{integration}` при отсутствующем
|
||||
platform-имени, откат на обобщённый текст); строку 201 удалить из
|
||||
«Принятых предположений» (это больше не предположение, а опровергнутый факт,
|
||||
раздел уже содержит корректную версию в теле ТЗ).
|
||||
|
||||
### L1 (сохраняется, Low, не блокирует) — Термин вкладки всё ещё «Доступные» в трёх местах, «Доступны» — в одном
|
||||
|
||||
**Файл:** строки 13, 49, 196 — «Доступные»; строка 73 (новый текст ревизии
|
||||
3, часть правки H1) и строка 138 (AC4) — «Доступны», канонический термин
|
||||
`docs/USER-GUIDE.ru.md:785` / `device_inbox.tab_available`.
|
||||
|
||||
Ревизия 3 внесла корректный термин именно там, где переписывала текст под
|
||||
H1, но не выровняла остальные вхождения — документ сейчас использует оба
|
||||
варианта параллельно. Как и в r1: не блокирует, правится автором в рабочем
|
||||
порядке при следующей правке текста; отдельного цикла не требует.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Без повторной проверки приняты (проверялись в SPEC-REVIEW-44-r1.md на
|
||||
коммите `3d4c5090`, продуктовый код с тех пор не менялся — только докс-
|
||||
коммиты `df1dbccf`, `ba568763`, `git log --oneline 3d4c5090..HEAD -- src`
|
||||
пуст):
|
||||
|
||||
- SCOPE.md: задача закрывает пункт J6/тезис issue («третий вариант»
|
||||
запрещён), конфликта со SCOPE нет.
|
||||
- Трек — полный (не `small`), файл ТЗ обязателен и существует; все
|
||||
обязательные разделы §7.1 присутствуют.
|
||||
- Резолвер `group_lights` (`devices.ts:1033/1098`) и `exclude_integrations`
|
||||
(`houseplan-card.ts:3924-3925`, `space-render.ts:245-246`) описаны точно,
|
||||
включая replace-семантику и `[]` как валидное «ничего не исключать».
|
||||
- `EXCLUDED_DOMAINS` (`rules.ts:12`, 13 доменов) совпадает с описанием.
|
||||
- `scripts/config-field-registry.mjs:42-70` — статусы `decision-required`,
|
||||
паспорта `allow-extra` обоих ключей, план перевода в `current` технически
|
||||
корректен и имеет прецедент (6 других полей).
|
||||
- `expected_rev`/транзакционный паттерн Сохранить, прецедент удаления ключа
|
||||
при возврате к дефолту (`settings.weather_entity`) — реальны.
|
||||
- `buildDevices(ctx)` — чистая функция, план AC6 (общий вход превью/боевого
|
||||
пути без копии логики) реализуем технически.
|
||||
|
||||
Эти пункты дельтой ревизии 3 не задеты (правка ограничена ровно тремя
|
||||
абзацами, перечисленными в «Скоуп ревью» выше), поэтому переносятся без
|
||||
повторной сверки кода.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Восстановлен SHA r1 (`3d4c5090`) и вердикт из
|
||||
`docs/reviews/SPEC-REVIEW-44-r1.md` (документ называет SHA явно — не
|
||||
находка).
|
||||
2. `git diff 3d4c5090..ba568763 -- docs/specs/044-filter-grouping-policy.md`
|
||||
— единственный источник дельты этого раунда.
|
||||
3. `git log --oneline 3d4c5090..HEAD -- src` — пусто, продуктовый код не
|
||||
менялся; гейты (`tsc`, `test`, `build`, `check-docs.mjs`) неприменимы,
|
||||
как и в r1 (чистый докс-коммит, `git show --stat ba568763` подтверждает
|
||||
единственный изменённый файл).
|
||||
4. Каждый факт из H1/H2-правок перепроверен чтением текущего кода (не на
|
||||
слово автора): `houseplan-editor-runtime.ts:7629-7642` (`reasonByBinding`,
|
||||
`integrationByBinding` уже существуют в одной структуре — плейсхолдер
|
||||
`{integration}` дёшев в реализации), `:11838-11851` (`row.integration`
|
||||
уже используется в соседней строке шаблона), `src/i18n/ru.json:484` /
|
||||
`en.json:484` (текущий обобщённый текст), `devices.ts:1435-1533`
|
||||
(`roomClimateMap`, ветка `optClimate:1488-1492`, вызовы `EXCLUDED_DOMAINS`
|
||||
— единственное прямое вхождение в `src/**`, перепроверено
|
||||
`grep -rn "EXCLUDED_DOMAINS\.has("`), три вызова `roomClimateMap`
|
||||
(`devices.ts:1546`, `houseplan-card.ts:11870`, `space-render.ts:301`),
|
||||
тесты `#317` (`test/devices.test.mjs:1304-1426, 2157-2166`) — сигнатура
|
||||
везде трёхаргументная, совместима с опциональным 4-м параметром.
|
||||
5. Полнотекстовый grep документа по «Доступны»/«Доступные»/«Скрыты» —
|
||||
выявил M1 и подтвердил L1.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Оба High из r1 закрыты по существу, не косметически: факты исправлены,
|
||||
новый контракт (1a) и AC4b реализуемы без структурных противоречий с
|
||||
существующим кодом (подтверждено чтением, включая совместимость с уже
|
||||
существующими тестами #317).
|
||||
- AC4 в новой формулировке однозначен и проверяем как написан (вкладка,
|
||||
текст причины, регресс-ветка на «значение reason не меняется»).
|
||||
- AC4b однозначен: три случая (интеграция вне действующего набора → климат
|
||||
участвует; интеграция в действующем наборе → не участвует;
|
||||
явный climate-opt-in побеждает) — тестируемы юнитом без реализации.
|
||||
- Ссылки на строки кода в новых абзацах (7637, 1491) точны, перепроверены
|
||||
заново на текущем дереве, а не унаследованы с доверием.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полный повторный разбор документа вне дельты — по правилу «дельта
|
||||
локальна», см. «Унаследовано из r1».
|
||||
- Гейты `tsc`/`test`/`build`/`check-docs.mjs`/смоки/golden/backend/perf —
|
||||
неприменимо: с r1 не менялось ничего в `src/**`, `test/**`, `demo/**`
|
||||
(проверено `git log --oneline 3d4c5090..HEAD -- src test demo` — пусто).
|
||||
- Не проверялась реализуемость точного способа проброса `{integration}` в
|
||||
`_t()` (нужен ли новый параметр вызова на строке 11849) — это вопрос
|
||||
кода на этапе реализации, не ТЗ; сам факт наличия данных (`row.integration`,
|
||||
`integrationByBinding`) подтверждён, механизм интерполяции `_t(key, params)`
|
||||
подтверждён существующими вызовами в другом месте того же файла.
|
||||
|
||||
## Итог
|
||||
|
||||
Вердикт: жёлтый. High-находок нет — оба High из r1 закрыты по существу и
|
||||
подтверждены чтением кода, а не на слово автора. Один Medium в скоупе (M1):
|
||||
ревизия 3 не убрала два места («Риски», «Принятые предположения»), которые
|
||||
буквально утверждают опровергнутый H1 факт («причина показывается на
|
||||
«Скрытых»») — внутреннее противоречие документа ровно на той оси, из-за
|
||||
которой r1 стал красным. Чинится без нового цикла (правка двух абзацев).
|
||||
Low (L1, термин вкладки) — сохраняется с r1, не блокирует.
|
||||
Reference in New Issue
Block a user