mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,189 @@
|
||||
# SPEC-REVIEW-543-r1
|
||||
|
||||
Issue: #543 · Этап: spec (PROCESS.md §2.4) · Заход: r1 · блокирующих циклов 0/4
|
||||
|
||||
Материал: тело issue #543, раздел `## ТЗ`, на момент ревью (метка `S4-spec-review`,
|
||||
два предыдущих комментария — S2-аналитика и передача ТЗ на ревью, оба того же
|
||||
автора, без предыдущих раундов ревью). Это первый заход, разбор полный, без
|
||||
раздела «Унаследовано».
|
||||
|
||||
Продуктовая постановка (аудиторский блок `## Пользовательская проблема` …
|
||||
`## Состояние при заведении`) и `## ТЗ` — согласованы: ТЗ прямо цитирует те же
|
||||
строки кода и тот же механизм (`_reloadConfigOnly` / `adoptAuthoritativeGated`),
|
||||
без расхождений.
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Проверялось: полнота обязательных разделов §7.1, однозначность и доказуемость
|
||||
каждого AC, соответствие продуктовой рамке `docs/SCOPE.md` (J1 «актуальное
|
||||
состояние», J6 «согласованность нескольких клиентов»), фактическая проверка
|
||||
технических утверждений ТЗ по коду на точном SHA `6d2facc2e37724a6e6e1a96c5e026aa24b595a07`
|
||||
(рабочая копия репозитория стоит на этом коммите — сверено `git rev-parse HEAD`).
|
||||
|
||||
Продуктовых вопросов к владельцу нет и не требовалось: сценарий и видимое
|
||||
поведение зафиксированы однозначно («До/После»), а всё техническое оформлено
|
||||
отдельным блоком «Принятые технические предположения — можно менять на
|
||||
ревью». Технического спора с автором, требующего отдельного цикла из-за
|
||||
разногласия, нет — единственная находка ниже касается точности ссылки на
|
||||
тестовый артефакт, а не архитектуры решения.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2.4/§2.9/§7.1/§7.2/§4.
|
||||
2. Прочитано тело issue #543 целиком, включая исходный аудиторский блок
|
||||
(источник, воспроизведение, причина по коду, ожидаемый результат) и оба
|
||||
комментария (S2-аналитика с приоритетом P2 и передача ТЗ на ревью).
|
||||
3. Прочитан текущий код на названном SHA и построчно сверен с ключевыми
|
||||
техническими утверждениями ТЗ:
|
||||
- `src/houseplan-card.ts:4417,4466-4501` — `_reloadConfigOnly`: `observedRev`
|
||||
сравнивается с `_cfgRev` только на входе (`:4473`), дальше идёт
|
||||
`await this._getAuthoritativeConfig()` и `await this._adoptAuthoritative(...)`
|
||||
без повторной проверки владения после первого await — подтверждает
|
||||
«Подтверждённую причину» ТЗ дословно;
|
||||
- `src/config-adoption.ts:428-460` — `adoptAuthoritativeGated`: `await
|
||||
host._signer.prepareImage(...)` (`:439`) идёт до `adoptStructuralResponses`
|
||||
(`:451`) без промежуточной проверки заявки — подтверждает К2 буквально;
|
||||
- все шесть вызывающих `_adoptAuthoritative`/`adoptAuthoritativeGated` мест
|
||||
(`houseplan-card.ts:4306,4485`, `summary-panel-runtime-loaded.ts:630`,
|
||||
`houseplan-onboarding-runtime.ts:477`, `houseplan-editor-runtime.ts:8693,9577,9737`)
|
||||
уже проверяют `status !== 'adopted'`, то есть новый статус `superseded`
|
||||
естественно поглощается существующей веткой без правки самих call sites —
|
||||
подтверждает заявление ТЗ «менять call sites только если явная обработка
|
||||
нужна»;
|
||||
- все `_reloadConfigOnly(true)` call sites (`houseplan-card.ts:5267,7933`,
|
||||
`summary-panel-runtime-loaded.ts:671`, `houseplan-onboarding-runtime.ts:287,410,497`,
|
||||
`space-copy-runtime.ts:224,248`, `houseplan-editor-runtime.ts:1842,8623,8703,9552`) —
|
||||
подтверждает, что «force/restore» путей на практике много, К7 не
|
||||
придумана под единственный случай;
|
||||
- `docs/ARCHITECTURE.md:1375-1393` (#500) — «Config/layout identity has one
|
||||
owner», единая точка `adoptAuthoritativeGated` — центр, в котором ТЗ
|
||||
предлагает разместить общую проверку заявки, соответствует уже
|
||||
установленному инварианту, а не вводит второй путь принятия;
|
||||
- прецедент generation-guard уже существует в кодовой базе:
|
||||
`_liveSyncGeneration`/`_liveSyncConnection` (`houseplan-card.ts:948-949,
|
||||
4376-4405`) — подтверждает, что предлагаемый механизм claim/generation не
|
||||
изобретение с нуля, а расширение уже принятого паттерна;
|
||||
- «route leave» — установленный в коде термин (`houseplan-card.ts:2059,2752,
|
||||
7447`), а не придуманный ТЗ;
|
||||
- `test/config-adoption-ownership.test.mjs` — прочитан целиком: это
|
||||
статический regex-сканер по `src/**` (`IDENTITY_WRITE`, `BODY_WRITE`),
|
||||
ратчет-тест другого инварианта #500 («только `ConfigAdoption` пишет
|
||||
`_cfgRev`/`_layoutRev`/fingerprint»), см. находку ниже;
|
||||
- `demo/smoke_save_race.mjs` — образец существующего паттерна: смок вызывает
|
||||
приватный `c._reloadConfigOnly()` напрямую через `page.evaluate` и
|
||||
подменяет `hass.callWS`; подтверждает техническую реализуемость AC2/AC4
|
||||
без новых инструментов;
|
||||
- `package.json` — `gate:small`, `invariants`, `docs:accept`, `golden:verify`
|
||||
существуют как названо в плане автотестов;
|
||||
- `demo/smoke_plan_upload_race.mjs`, `smoke_save_race.mjs`,
|
||||
`smoke_ws_resilience.mjs`, `smoke_version_recovery.mjs` — существуют, как
|
||||
названо в плане автотестов п.6.
|
||||
4. Продуктового кода к задаче ещё нет (этап — ревью ТЗ), поэтому гейты
|
||||
typecheck/test/build/golden не прогонялись и не требовались.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — двусмысленная ссылка на тестовый артефакт в AC7 и списке файлов
|
||||
|
||||
`AC7` и раздел «Ожидаемые файлы и модули» называют `test/config-adoption-ownership.test.mjs`
|
||||
как один из свидетелей порядка `compare → asset readiness → continuity → adopt
|
||||
→ reload tail → caller hook`. Этот файл уже существует и уже документирован
|
||||
(`docs/ARCHITECTURE.md:1375-1393`, докстринг файла) как нечто другое: чисто
|
||||
статический regex-сканер исходников, доказывающий не поведенческий порядок
|
||||
операций, а то, что `_cfgRev`/`_layoutRev`/fingerprint пишет только
|
||||
`ConfigAdoption` (ратчет-инвариант #500, счётчики `BODY_STAGING_ALLOWLIST`
|
||||
могут только уменьшаться). Он не выполняет ни одного `_reloadConfigOnly`,
|
||||
не мокает `prepareImage`, не видит async-порядка вообще — механически не
|
||||
способен подтвердить то, что ему приписывает AC7.
|
||||
|
||||
**Сценарий проявления:** разработчик, ориентируясь на прямое упоминание в
|
||||
ТЗ, либо (а) добавляет поведенческие async-тесты заявки/generation в этот
|
||||
файл, ломая его текущую единственную задачу и докстринг («one owner for
|
||||
config/layout identity»), либо (b) недоумевает и оставляет AC7 доказанным
|
||||
только `test/config-adoption.test.mjs`, а «ownership tests» в ревью кода
|
||||
считается недоказанным пунктом AC. Оба исхода — трение, которого не было бы
|
||||
при точной ссылке.
|
||||
|
||||
Это не блокирует ТЗ целиком: основной свидетель AC7 —
|
||||
`test/config-adoption.test.mjs`, который действительно предназначен для
|
||||
поведенческих проверок (план автотестов п.1, deferred-promise матрица) и
|
||||
способен нести эту нагрузку в одиночку. Также не пострадала ни одна связь с
|
||||
`docs/CONFIG-COMPATIBILITY.md` или другим каноном — это чисто internal
|
||||
naming/reference вопрос.
|
||||
|
||||
**Правка:** в AC7 и в «Ожидаемых файлах» либо явно развести два разных смысла
|
||||
слова «ownership» (например, назвать новый файл/describe-блок иначе — `test/config-adoption-attempt.test.mjs`
|
||||
или новый describe в `config-adoption.test.mjs`), либо прямо указать, что
|
||||
`config-adoption-ownership.test.mjs` в этой задаче не трогается и упомянут по
|
||||
ошибке. Без правки автор может честно закрыть AC7 текстом «доказано
|
||||
`config-adoption.test.mjs`», но текущая формулировка ТЗ вводит в заблуждение.
|
||||
|
||||
## Проверка обязательных разделов (PROCESS.md §7.1)
|
||||
|
||||
Все обязательные разделы на месте и содержательны: сценарий · что человек
|
||||
увидит до/после · проблема (аудиторский блок) · скоуп и не-скоуп · контракт
|
||||
поведения (К1–К9) · UX (пусто, обоснованно) · модель данных и миграция (нет
|
||||
изменений схемы, обоснованно) · i18n (нет) · критерии приёмки AC1–AC9 с
|
||||
доказательством (`unit`/`smoke`/`lifecycle matrix`/`force/reset`/`adoption
|
||||
regression`/`mutation`/`gates`) · план автотестов · риски · откат ·
|
||||
release-артефакты.
|
||||
|
||||
- **Сценарий/видимое поведение** — персона-центричны (SCOPE J1/J6), не
|
||||
используют внутренних терминов реализации сверх необходимого; «claim»,
|
||||
«generation», «high-water» вводятся только в контракте (К1–К9), не в
|
||||
сценарии.
|
||||
- **AC однозначны и проверяемы**, у AC1/AC7/AC8 явно назван отрицательный
|
||||
свидетель («тест умеет падать» — уже в самом ТЗ, а не отложено на код-ревью).
|
||||
AC2/AC5 требуют реального host/lifecycle-прогона, а не последовательного
|
||||
unit-теста — соответствует явному требованию «Ожидаемого результата» из
|
||||
исходного аудиторского блока issue.
|
||||
- **Технические предположения** оформлены отдельным блоком «можно менять на
|
||||
ревью», 5 пунктов, каждый — реальное техническое решение (кто выигрывает
|
||||
гонку, что считается новым baseline, поведение `superseded`, судьба
|
||||
signer-кэша просроченной попытки, исключение layout reload), ни один не
|
||||
маскирует продуктовый вопрос.
|
||||
- **Скоуп/не-скоуп** согласован с явным исключением протокола backend/WS,
|
||||
глобальной сериализации reload и правила «никогда не принимать меньшую
|
||||
revision» — совпадает с «Ожидаемым результатом» аудита («не вводить
|
||||
безусловный вечный запрет»).
|
||||
- **Риски** — шесть пунктов, каждый называет конкретный способ, которым
|
||||
наивная реализация могла бы сломать defense (не декоративный список).
|
||||
- **Полный трек оправдан явно**: названы нарушенные критерии `small`
|
||||
(multi-client state contract, несколько lifecycle-границ, асинхронный asset
|
||||
gate, риск для continuity/cache) — соответствует §5.
|
||||
|
||||
## Что не проверялось и почему
|
||||
|
||||
- Гейты `typecheck`/`test`/`build`/`golden`/`invariants` — не прогонялись:
|
||||
продуктового кода ещё нет, предмет этапа — текст ТЗ.
|
||||
- Реализация `request-owner/coordinator` модуля, будущего мутанта AC8 и
|
||||
содержимого `demo/smoke_config_reload_race.mjs` — не существуют на этом
|
||||
этапе, их проверит код-ревью.
|
||||
- Частота гонки на реальном HA в проде — сам аудит и ТЗ прямо признают это
|
||||
неизмеренным; для приоритета P2 и полного трека этого достаточно, отдельно
|
||||
не перепроверялось.
|
||||
- Совместимость с бэкендом/WS-протоколом — вне скоупа задачи по её же
|
||||
формулировке, не проверялась.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый: единственная находка — Medium в скоупе задачи (двусмысленная ссылка
|
||||
на `test/config-adoption-ownership.test.mjs` в AC7 и списке файлов). High нет.
|
||||
Технический контракт (К1–К9), причина по коду и большинство AC подтверждены
|
||||
прямым чтением кода на названном SHA, а не приняты на веру. Автор правит
|
||||
ссылку на тестовый артефакт в теле issue, изменение проходит повторный цикл
|
||||
ревью ТЗ по дельте (PROCESS.md §2.10).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `6d2facc2e377` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `bd3acd278b0c6dd3d0cc572096f8c0d2c7e3b38c`
|
||||
```
|
||||
git log --all --format='%H %T' | grep bd3acd278b0c
|
||||
```
|
||||
- Тело issue: `06cae6bc0c1c834e055a858a18d406231ff7a696f929be70277a82a675f6779a`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user