mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,151 @@
|
||||
# SPEC-REVIEW #440 — r1
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/440
|
||||
- Этап: spec (PROCESS.md §2.4)
|
||||
- ТЗ: `docs/specs/440-v171-beta2-polish.md`, коммит `735710f1`
|
||||
- Заход r1 · блокирующих циклов израсходовано 0/4
|
||||
- Вердикт: **зелёный**
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Полный разбор — заход первый, дельты нет. ТЗ покрывает семь пунктов аудита
|
||||
v1.71.0-beta.2 (§3.3): (а) зависание verifier на не-обычном файле, (б) потеря
|
||||
pointer-modality на room pointermove при выключенном tooltip, (в) getter с
|
||||
побочным эффектом `_dangerConfirmLocaleGate`, (г) TOCTOU в физической
|
||||
инвентаризации ассетов + неверный HTTP 507, (д) source-regex свидетель AC5
|
||||
#434, (е) отсутствие ретроактивных review-документов для #429/#430, (ж)
|
||||
`importorskip("homeassistant")`, скрывающий чистые Python-тесты канонизации.
|
||||
|
||||
Читал в заданном порядке: `docs/SCOPE.md`, `PROCESS.md` §1–§8, тело issue
|
||||
#440 и оба комментария (аналитика + сдача ТЗ), само ТЗ целиком,
|
||||
`docs/TOUCH-SUPPORT.md` §«Pointer modality and hover ownership».
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью ТЗ — не код-ревью, но каждое фактическое утверждение ТЗ о текущем
|
||||
поведении кода я сверил с реальным деревом на `origin/issue/440-audit-polish`
|
||||
(= `735710f1`), чтобы отличить обоснованный контракт от догадки, выданной за
|
||||
факт:
|
||||
|
||||
| Пункт ТЗ | Файл:строка | Что проверено |
|
||||
|---|---|---|
|
||||
| (а) verifier/FIFO | `custom_components/houseplan/asset_integrity.py:42-129` | `_signature()` = голый `path.stat()` (проходит на FIFO/device), `_stream_sha256()` открывает `open("rb")` без таймаута, followers ждут `event.wait()` без границы. Совпадает с описанием ТЗ дословно |
|
||||
| (б) pointer modality | `src/houseplan-card.ts:11589-11675`, `:7186-7193`, `:7291-7304` | `tip(e)` (bound на `@pointermove`) при `!showRoomTooltipOf(...)` возвращается на 11592-11593 **до** вызова `_notePointer`; `enterRoom` (bound на `@pointerenter`) вызывает `_notePointer` всегда. Подтверждает узкую щель: смена pointer type внутри уже наведённой комнаты не долетает до `_notePointer`, когда tooltip выключен |
|
||||
| (в) locale getter | `src/houseplan-card.ts:2160-2166,2188,4193`, `src/i18n/language-runtime.ts:105-131` | Геттер `_dangerConfirmLocaleGate` вызывает `languageRenderGate()`, которая мутирует `host.inert`, `aria-busy`, `lang`, два `WeakSet`, планирует `requestUpdate()` — вызывается из `_confirmDanger()` и `updated()`, вне рендера. Подтверждено |
|
||||
| (г) TOCTOU/507 | `custom_components/houseplan/decor_assets.py:376-394`, `http_api.py:262-347` | `physical_asset_blobs` не оборачивает `path.stat(follow_symlinks=False)` внутри цикла `iterdir()` — исчезновение даёт необработанный `OSError`, который поднимается через `physical_asset_usage` → `_store()` → перехватывается только на `except OSError` (500). Отдельно `except DecorAssetError: … status=507` на строке 344 ловит **любой** `DecorAssetError` из `_store()`, включая `invalid_image` (273, 289) — сейчас всегда 507. Оба факта подтверждены |
|
||||
| (д) AC5 regex | `test/space-card-audit-lows.test.mjs:28-39` | Ровно пять `assert.match` по тексту `src/space-card.ts`/`src/config-store.ts`, как описано в ТЗ |
|
||||
| (ж) importorskip | `tests_backend/test_coordinate_canonicalization.py:1-40` | Модульный `pytest.importorskip("homeassistant")` стоит выше импорта `coordinate_canonicalization` и `DECOR_BOX_KINDS` — чистые тесты (`test_decor_box_catalog_matches_shared_contract`, `test_all_4801_lattice_nodes_and_nine_decimal_forms_share_exact_bits`) реально скипаются без HA |
|
||||
| AC6 инфраструктура | `scripts/mutation-gate.mjs:2402-2450` | Мутанты `image-box-python-canonicalization-omitted` и `all_4801_lattice_nodes...` уже указывают на `tests_backend/test_coordinate_canonicalization.py` через `backend-test-guard.mjs` — перенаправление на новый модуль механически осуществимо, путь не изобретён |
|
||||
| TOUCH-SUPPORT.md | `docs/TOUCH-SUPPORT.md:43-57` | «Touch and pen input immediately clear transient room and device hover, including tooltips» — контракт п.2 ТЗ («независимо от настройки») не изобретён, а прямая цитата канона |
|
||||
| i18n | `src/i18n/{en,ru,de,fr}.json` | Все четыре словаря существуют, ТЗ верно перечисляет «не меняются» |
|
||||
| Смоки в «Затронутых модулях» | `ls demo/smoke_danger_confirm_branches.mjs demo/smoke_room_tooltip_toggle.mjs demo/smoke_space_card_decor_capability.mjs` | Все три файла существуют |
|
||||
| `docs/specs/README.md` | `git show HEAD -- docs/specs/README.md` | Строка на #440 добавлена в том же коммите, ссылка на файл корректна |
|
||||
|
||||
Проверка кодом не подтверждает и не опровергает пункт (е) — это решение
|
||||
процесса, а не факт кода; ТЗ фиксирует его как аналитический вывод без
|
||||
кодовых изменений, что соответствует PROCESS.md §1 (инфраструктурные задачи
|
||||
без файлов класса A идут без ТЗ/код-ревью).
|
||||
|
||||
## §7.1 — обязательные разделы
|
||||
|
||||
Все присутствуют: Сценарий · Что человек увидит до и после · Проблема и
|
||||
подтверждённые причины · Скоуп/Не-скоуп · Контракт поведения (7 пунктов) ·
|
||||
Touch и доступность (UX) · Модель данных, совместимость и i18n · Производительность
|
||||
и безопасность · Затронутые модули · Критерии приёмки AC1–AC8 с доказательством
|
||||
· таблица «чем краснеет» для AC1–AC6 · План автотестов · Риски · Откат ·
|
||||
Release-артефакты · блок принятых технических предположений. Трек (`full`)
|
||||
обоснован явно названным нарушенным критерием лёгкого трека (несколько
|
||||
поверхностей, TS+Python, touch-контракт) — соответствует PROCESS.md §5.
|
||||
|
||||
## Догадки, выданные за факт
|
||||
|
||||
Не найдены. Каждое утверждение о текущем поведении сверено с кодом (таблица
|
||||
выше) и совпадает буквально. Технические решения, не вытекающие однозначно из
|
||||
существующего контракта (точное значение follower-таймаута, точное имя
|
||||
command-метода locale-gate, единство/раздельность test harness), явно
|
||||
вынесены в раздел «Принятые технические предположения» с пометкой «может быть
|
||||
свободно скорректировано ревьюером» — я эту пометку принимаю без правок: ни
|
||||
одно из шести предположений не меняет продуктовый контракт и не требует
|
||||
продуктового решения владельца.
|
||||
|
||||
## Продуктовые вопросы владельцу
|
||||
|
||||
Нет. Ни один AC не требует решения о том, что человек видит или делает —
|
||||
задача исключительно про hardening существующих контрактов (пункты 1–4, 6) и
|
||||
про качество тестового свидетеля (пункты 5, 7), без нового UX. Раздел
|
||||
«Аналитика» в issue уже верно это фиксирует.
|
||||
|
||||
## Находки
|
||||
|
||||
Ни одной блокирующей (High) или требующей возврата (Medium) находки.
|
||||
|
||||
**Low (снимается без правки, с записью).** Формулировка контракта AC4/§4
|
||||
«Если root либо отдельная entry исчезла… этот кандидат пропускается» лексически
|
||||
объединяет два разных случая: исчезновение одной записи между `iterdir()` и
|
||||
`stat()` (ровно то, что описывает аудит и что покрывает план автотестов п.4) и
|
||||
исчезновение самого каталога `root` целиком (у которого нет «кандидата» —
|
||||
пропадать нечему, весь скан просто должен дать `count=0, bytes=0` без
|
||||
исключения). План автотестов раздела «План автотестов» п.4 явно строит только
|
||||
первый случай (подмена stat одного кандидата), про второй не говорит. Это не
|
||||
блокирует: реализация свободна закрыть оба случая одним try/except вокруг
|
||||
`iterdir()`-цикла, а второй случай на порядок более гипотетичен (нужно, чтобы
|
||||
директория ассетов исчезла во время активного апдейта HA) и не был частью
|
||||
исходной находки аудита (г), которая специально про TOCTOU на отдельной
|
||||
записи. Снимаю без возврата автору — реализация и ревьюер кода в состоянии
|
||||
решить механику по контексту; если разработчик по факту не покроет
|
||||
root-vanishing веткой теста, это не расхождение с ТЗ, а вопрос полноты теста
|
||||
для код-ревью.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все семь пунктов аудита в ТЗ имеют прямое соответствие в коде на текущем
|
||||
SHA — ни один не является пересказом чужих слов без проверки.
|
||||
- AC1–AC8 однозначны, у каждого указан способ доказательства; AC1–AC6
|
||||
дополнительно снабжены таблицей «чем краснеет» — это избыточно для этапа
|
||||
spec (обязательно только для код-ревью, §2.7), но облегчает будущий цикл.
|
||||
- Скоуп/не-скоуп разделены чётко, включая явный отказ трогать API/схему/
|
||||
golden/performance-baseline.
|
||||
- Откат описан как atomic revert с оговоркой про частичный откат по
|
||||
компонентам — соответствует стилю других ТЗ пакетных полишей.
|
||||
- Touch-контракт п.2 — прямая цитата `docs/TOUCH-SUPPORT.md`, не изобретение.
|
||||
- i18n/release-артефакты корректны по факту дерева (все 4 локали существуют,
|
||||
правок нет; `docs/specs/README.md` обновлён в том же коммите).
|
||||
- Названные для редактирования тестовые/смок-файлы существуют в дереве.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал автотесты/гейты — ревью ТЗ не требует прогона (нет кода для
|
||||
тестирования, реализация ещё не начата).
|
||||
- Не проверял `custom_components/houseplan/http_api.py` целиком построчно —
|
||||
только участки, относящиеся к пунктам (а) и (г).
|
||||
- Не проверял `docs/ARCHITECTURE.md`/`docs/TESTING.md` на точное текущее
|
||||
содержание тех разделов, которые ТЗ предлагает обновить «если требуют» —
|
||||
условная формулировка допустима на этапе ТЗ (детали редактирования
|
||||
документации решает исполнитель).
|
||||
- Не оценивал реализуемость точного числового значения follower-таймаута —
|
||||
ТЗ прямо помечает это как техническое предположение, свободное для
|
||||
корректировки, поэтому численное значение не является предметом ревью ТЗ.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- SHA ТЗ: `735710f1` (ветка `origin/issue/440-audit-polish`, HEAD на момент
|
||||
ревью).
|
||||
- Дерево материала: рабочая копия на `735710f1`, `git status` чист.
|
||||
- Файл ТЗ: `docs/specs/440-v171-beta2-polish.md` (422 строки, добавлен этим
|
||||
коммитом вместе с записью в `docs/specs/README.md`).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/440-audit-polish`, коммит `735710f16b8f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `927a83664db31061974ea9103581848a0cc8470e`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 927a83664db3
|
||||
```
|
||||
- ТЗ `docs/specs/440-v171-beta2-polish.md`, блоб `0de292e2f2dc734f08a9f3eb4d37d2a6734da3e4`
|
||||
```
|
||||
git log --all --find-object=0de292e2f2dc734f08a9f3eb4d37d2a6734da3e4 -- docs/specs/440-v171-beta2-polish.md
|
||||
```
|
||||
Reference in New Issue
Block a user