mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,174 @@
|
||||
# SPEC-REVIEW-262-r1
|
||||
|
||||
- Issue: [#262](https://github.com/Matysh/houseplan-card/issues/262) — Deleting entities prevents them from being added again later
|
||||
- ТЗ: [`docs/specs/262-readd-child-entity-after-device-delete.md`](https://github.com/Matysh/houseplan-card/blob/issue/262-readd-child-entity/docs/specs/262-readd-child-entity-after-device-delete.md)
|
||||
- Ветка: `issue/262-readd-child-entity`, коммит на ревью: `7b3ea37`
|
||||
- Этап: `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
|
||||
- Заход: r1 · блокирующих циклов израсходовано 0 из 4 (зелёный вердикт бюджет не тратит, #227)
|
||||
- Диф ревью: `git diff origin/dev...origin/issue/262-readd-child-entity` — только
|
||||
`docs/specs/262-readd-child-entity-after-device-delete.md` (новый файл) и одна
|
||||
строка в `docs/specs/README.md`. Продуктового кода нет — соответствует стадии
|
||||
`S3→S4` (Rule #1: код трогают только с `S5-ready`).
|
||||
|
||||
## Скоуп
|
||||
|
||||
Bug-репорт: после удаления HA-**устройства** с плана его отдельные дочерние
|
||||
**сущности** невозможно добавить обратно даже при включённом флаге «Показывать
|
||||
сущности» — можно вернуть только всё устройство целиком. Аналитика владельца
|
||||
(комментарий в issue) разложила репорт на три сценария и подтвердила, что
|
||||
именно третий («удалить device, добавить одну entity того же device») — баг,
|
||||
остальные два уже работают штатно. ТЗ фиксирует контракт на этот третий
|
||||
сценарий: exact live `entity:X` должна перевешивать exact/parent tombstone
|
||||
только для себя самой, не воскрешая ни родительское устройство, ни соседние
|
||||
сущности.
|
||||
|
||||
Соответствие `docs/SCOPE.md`: закрывает J6 («Keep the plan true as the home
|
||||
evolves») — часть про «поддерживать план в актуальном состоянии» и явный выбор
|
||||
нужной HA-привязки после удаления. Продуктовой рамке не противоречит, новых
|
||||
поверхностей не открывает.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью технической части ТЗ построено не на доверии тексту, а на сверке каждого
|
||||
фактического утверждения (§3, §4, §6, §8) с кодом на `origin/dev` — здесь
|
||||
именно тот случай, где «утверждение о поведении не в документах и не
|
||||
помеченное как предположение» было бы находкой, если бы не подтвердилось.
|
||||
|
||||
1. **§4.1 (picker разрешает только точный tombstone binding).** Прочитан
|
||||
`_bindingCandidates()` в `src/houseplan-card.ts` (ветка «Individual entities
|
||||
of devices — behind the show entities checkbox»): `removedBindings` — Set
|
||||
точных строк binding с `m.removed`; исключение `!removedBindings.has(v)`
|
||||
сравнивается с `v = 'entity:'+eid`, а не с `'device:'+parentId`. При
|
||||
`device:D` tombstone `isRemovedPlanEntity(h, eid, removed)` возвращает
|
||||
`true` (по `removed.devices.has(deviceId)`), `removedBindings.has('entity:'+eid)`
|
||||
— `false` ⇒ строка `continue` отбрасывает X из списка даже при включённом
|
||||
чекбоксе. Ровно то, что описывает ТЗ. Подтверждено чтением.
|
||||
2. **§4.2 (фикса списка недостаточно).** Прочитан `_saveMarker()`:
|
||||
`replacedRemovedIds = markers.filter(m => m.removed && m.binding === dlg.binding)`
|
||||
— снимает tombstone только с тем же exact binding (`entity:X`), родительский
|
||||
`device:D` tombstone переживает сохранение. Прочитан `buildDevices()`, ветка
|
||||
`kind === 'entity'`: `if (isRemovedPlanEntity(fullHass, ref, removed)) continue;`
|
||||
— уже сохранённый **живой** explicit marker `entity:X` всё равно отбрасывается,
|
||||
потому что `removed` строится из того же `markers[]`, где всё ещё лежит
|
||||
`device:D` tombstone. Оба утверждения ТЗ подтверждены чтением, не
|
||||
домыслены.
|
||||
3. **§8 (schema-valid combination).** Прочитан `validation.py`: схема маркера
|
||||
не запрещает сосуществование двух записей с разными `binding` и независимым
|
||||
`removed`; для `device:D removed:true` + живой `entity:X` нет constraint,
|
||||
который бы это отклонил. Утверждение «уже schema-valid» — не гипотеза.
|
||||
4. **§3 (воспроизведение).** Прочитан `demo/smoke_binding_picker.mjs` (добавлен
|
||||
в #263): блок 3 буквально помечает
|
||||
`o.knownDefect262ChildEntityBlocked = !(await offered(true)).includes('entity:'+childEntity)`
|
||||
с комментарием «почитают #262, смок покраснеет, потребовав перевернуть
|
||||
проверку» — соответствует плану автотестов ТЗ (AC1: «проверка known defect
|
||||
перевёрнута в положительную»).
|
||||
5. **Согласованность с #226.** Прочитан `docs/specs/226-entity-parent-dedup.md`
|
||||
(решения 4–5: «явная конфигурация сильнее автоматической», «tombstone не
|
||||
владеет сущностью») — контракт §6.4 ТЗ #262 («явная D не подавляет явную X»)
|
||||
этому не противоречит, использует уже принятую модель, не переопределяет её.
|
||||
6. **Согласованность с #104.** `docs/specs/104-opening-ha-reference-after-marker-delete.md`
|
||||
сам ссылается на `isRemovedPlanEntity`/`isRemovedPlanSource` — подтверждает
|
||||
заявление ТЗ, что точные ссылки opening contact/lock уже независимы от
|
||||
marker tombstones и не задеты этим исправлением.
|
||||
7. **Регистрация smoke-связи.** `scripts/smoke-links.mjs` уже связывает символы
|
||||
`removedPlanBindings`, `isRemovedPlanEntity`, `deletePlanMarkerRecords` со
|
||||
`smoke_binding_picker.mjs` (внесено в #263) — заявление §11 ТЗ («имя
|
||||
`_bindingCandidates` остаётся прямым совпадением, дублировать не требуется»)
|
||||
корректно, связь уже есть, ничего добавлять не нужно.
|
||||
8. **Терминология UI.** Флаг называется в ТЗ «Показывать сущности» —
|
||||
совпадает с формулировкой из `docs/USER-GUIDE.ru.md` (строки 657, 1587), а
|
||||
не изобретён. (Сам i18n-ключ `marker.show_entities` в коде переведён как
|
||||
«Отображать сущности» — расхождение между `USER-GUIDE.ru.md` и `ru.json`
|
||||
существует независимо от #262, автор ТЗ корректно взял термин из
|
||||
пользовательского гайда, как требует AGENTS.md; фиксирую это отдельно ниже
|
||||
как находку вне скоупа, не блокирующую этот заход.)
|
||||
9. **Обязательные разделы ТЗ (PROCESS.md §7.1).** Сверены построчно: сценарий
|
||||
и персона (§1) — есть, включая поверхность и момент; что человек видит
|
||||
до/после одной фразой без терминов реализации (§2) — есть, без упоминаний
|
||||
функций/tombstone; проблема (§3–4); скоуп и не-скоуп (§5); контракт
|
||||
поведения (§6); UX/touch/a11y (§7); данные/compatibility/миграция (§8);
|
||||
i18n/security/performance (§9); AC1…AC6 с доказательством (§10); план
|
||||
автотестов (§11); файлы (§12); release-артефакты (§13); риски (§14);
|
||||
откат (§15); блок принятых предположений (§16). Ничего не пропущено.
|
||||
10. **Проверка «не выдана ли догадка за факт».** Все технические утверждения о
|
||||
текущем поведении (§3, §4, §8) проверены чтением реального кода (пункты
|
||||
1–3 выше), а не приняты на слово. Пять пунктов §16 корректно помечены как
|
||||
предположения и являются техническими (форма реализации, позиция нового
|
||||
маркера, отсутствие whitelist для registry-hidden), а не спрятанными
|
||||
продуктовыми решениями — ни один не требовал вопроса владельцу.
|
||||
11. **AC на проверяемость и доказательства.** AC1–AC6 однозначны, у каждого
|
||||
указан способ доказательства (`smoke`/`unit`/«существующие
|
||||
unit/smoke #104/#161/#226»/gates), совпадающий с §11 и с уже
|
||||
зарегистрированными файлами/тестами.
|
||||
|
||||
## Находки
|
||||
|
||||
Блокирующих (High) и находок Medium в скоупе — нет.
|
||||
|
||||
### Medium — вне скоупа (не блокирует, заводится отдельным issue по §12 PROCESS.md)
|
||||
|
||||
Расхождение термина чекбокса между `docs/USER-GUIDE.ru.md` («Показывать
|
||||
сущности», строки 657 и 1587) и фактическим i18n-ключом `marker.show_entities`
|
||||
в `src/i18n/ru.json` («Отображать сущности»). Автор ТЗ #262 действовал верно —
|
||||
взял формулировку из канонического пользовательского гайда, как требует
|
||||
AGENTS.md, — но само расхождение живёт в репозитории независимо от #262 и не
|
||||
имеет отношения к его сценарию. Правка в этой ветке была бы посторонним
|
||||
скоупом (диалог маркера правит другой файл — `houseplan-card.ts`/`ru.json`, не
|
||||
затронутый ТЗ #262). Заведён отдельным issue — см. решение ниже.
|
||||
|
||||
Низких (Low) находок нет.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Диагноз двухчастного дефекта (picker exact-binding whitelist + runtime
|
||||
`isRemovedPlanEntity` в `buildDevices`) — точное соответствие коду `dev`, не
|
||||
гипотеза.
|
||||
- Контракт §6 (матрица Add picker, транзакция сохранения, runtime-приоритет,
|
||||
что остаётся удалённым) — самосогласован: список Add специально шире, чем
|
||||
runtime-построение (все дети D становятся видимыми в Add, но
|
||||
строится/рендерится только явно сохранённый marker) — это осмысленное
|
||||
разделение, а не противоречие.
|
||||
- Совместимость с уже принятыми контрактами #226, #161, #104 подтверждена по
|
||||
их собственным спекам и коду, не только заявлена.
|
||||
- Данные/миграция: комбинация `device:D removed:true` + живой `entity:X` уже
|
||||
schema-valid, новых полей и model bump нет — подтверждено чтением
|
||||
`validation.py`.
|
||||
- Воспроизведение (§3) подтверждено существующим browser smoke
|
||||
`demo/smoke_binding_picker.mjs` (#263), который уже фиксирует
|
||||
`knownDefect262ChildEntityBlocked: true` и явно рассчитан на переворот
|
||||
проверки этим исправлением.
|
||||
- Все обязательные разделы ТЗ (PROCESS.md §7.1) присутствуют и по существу, не
|
||||
формально.
|
||||
- Продуктовых вопросов владельцу нет, и это оправдано: ни одна неопределённость
|
||||
в ТЗ не задевает то, что видит или делает пользователь сверх уже описанного
|
||||
в §1–§2; сценарий закрыт исходным репортом и аналитическим комментарием
|
||||
владельца.
|
||||
- i18n-раздел корректен: новых ключей нет, использованная терминология взята
|
||||
из `docs/USER-GUIDE.ru.md`, а не изобретена.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты `typecheck`/`test`/`build`/`check-docs` не гонялись: этап — ревью ТЗ
|
||||
(`S3→S4`), продуктового и тестового кода в дифе нет (только новый
|
||||
`docs/specs/*.md` и строка в `docs/specs/README.md`), эти гейты относятся к
|
||||
этапу код-ревью (PROCESS.md §2.7, §8) и будут прогнаны там.
|
||||
- Не проверялась работоспособность будущей реализации — её ещё нет; предмет
|
||||
этого ревью — выполнимость и однозначность ТЗ, а не код.
|
||||
- Не запускал browser smoke `demo/smoke_binding_picker.mjs` — код на этой
|
||||
ветке не менялся относительно `dev`, поведение смока не могло измениться;
|
||||
чтения исходника было достаточно, чтобы подтвердить, что смок уже
|
||||
воспроизводит описанный в ТЗ дефект.
|
||||
- Не проверял golden/performance/mutation-gate — на этой стадии нет ни
|
||||
визуальных, ни мутационных изменений: диф ограничен документацией.
|
||||
|
||||
## Решение
|
||||
|
||||
Найдена ровно одна находка — Medium, вне скоупа задачи (терминологическое
|
||||
расхождение чекбокса, не создано и не усугублено этим ТЗ). Она не блокирует
|
||||
переход в `S5-ready` и заводится отдельным issue со ссылкой на #262, а не
|
||||
чинится в этой ветке.
|
||||
|
||||
**Вердикт: зелёный.** ТЗ выполнимо, однозначно, каждый AC проверяем и снабжён
|
||||
способом доказательства, технические утверждения о текущем поведении
|
||||
подтверждены чтением кода, а не приняты на веру, продуктовых вопросов
|
||||
владельцу нет.
|
||||
Reference in New Issue
Block a user