mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,168 @@
|
||||
# SPEC-REVIEW-397-r2
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/397
|
||||
- ТЗ: `docs/specs/397-device-position-echo.md` (класс A, полный трек)
|
||||
- Заход: r2 · блокирующих циклов израсходовано 1 из 4 (r1 — жёлтый, потратил
|
||||
цикл; этот раунд бюджет не тратит, см. вердикт)
|
||||
- SHA r1: `83692e79` (найден в документе `docs/reviews/SPEC-REVIEW-397-r1.md`,
|
||||
раздел "Скоуп"; **в комментарии-вердикте r1 SHA не назван** — это находка
|
||||
формата, зафиксирована ниже, не блокирует)
|
||||
- SHA этого раунда: `6edcde01`
|
||||
|
||||
## Скоуп раунда
|
||||
|
||||
Дельта — только `docs/specs/397-device-position-echo.md`:
|
||||
|
||||
```
|
||||
git diff 83692e79..6edcde01 -- docs/specs/397-device-position-echo.md
|
||||
1 file changed, 23 insertions(+), 7 deletions(-)
|
||||
```
|
||||
|
||||
(второй файл в общем диапазоне, `docs/reviews/SPEC-REVIEW-397-r1.md`, —
|
||||
публикация документа предыдущего раунда, не предмет ревью.)
|
||||
|
||||
Правка автора — точечный ответ на вердикт r1: один Medium (AC5б/AC7 без
|
||||
способа доказательства) и два Low (диапазон строк, отсутствие фразы
|
||||
"Доказательство" у AC3). Продуктовый код не тронут (`src/**` не в диффе) —
|
||||
ожидаемо для этапа "ТЗ на ревью". Дельта локальна: не ребейз, контракт
|
||||
поведения не меняется, новая подсистема не задета, объём дельты (23 строки)
|
||||
несопоставим с объёмом исходной задачи (174 строки). Разбор по дельте
|
||||
достаточен; полный повторный прогон не требуется.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где видно |
|
||||
|---|---|---|
|
||||
| **Medium**: AC7 и вторая половина AC5 без способа доказательства | Добавлены пункты 7 (AC5б: удаление → честный reload → `canUndo` остаётся `true`) и 8 (AC7: запись «в полёте», сервер отвечает старой позицией → в `_layout` остаётся отправленная) в "План автотестов"; у AC5 и AC7 добавлена фраза "Доказательство: …" со ссылкой на эти пункты | `docs/specs/397-device-position-echo.md:121–138` (AC), `:157–160` (план) |
|
||||
| **Low-1**: диапазон `_persistDevicePlacement` указан как `:5226-5258`, тело функции длиннее | Исправлено на `:5226-5261` | `docs/specs/397-device-position-echo.md:28` |
|
||||
| **Low-2**: у AC3 нет явной фразы "Доказательство" | Добавлено "Доказательство: смок, пункт 5 плана — …" | `docs/specs/397-device-position-echo.md:114–117` |
|
||||
|
||||
Все три закрытия проверены не на слово автора, а построчным чтением текущего
|
||||
`src/houseplan-card.ts` и `src/device-position-history.ts` на SHA `6edcde01`
|
||||
(продуктовый код с r1 не менялся, но заново прочитан, так как новые
|
||||
формулировки AC5/AC7 делают новые технические утверждения о его поведении —
|
||||
это предмет дельты, не наследуется):
|
||||
|
||||
- **Диапазон функции.** `awk` по файлу подтверждает: `private async
|
||||
_persistDevicePlacement(` начинается на `:5226`, закрывающая скобка метода —
|
||||
на `:5261` (после `this._persistLocalLayout(); }`). Новый диапазон точен.
|
||||
- **AC5, обоснование раздельного доказательства.** Ветка удаления
|
||||
(`placement === null`, `pending = null`, `houseplan/layout/delete`)
|
||||
подтверждена буквально на `:5236-5242`. `applyDevicePlacement`
|
||||
(`src/device-position-history.ts:45-55`) при `placement === null`
|
||||
действительно **удаляет ключ** (`delete next[deviceId]`), а не
|
||||
"заменяет значение" — рационале AC5 в ТЗ технически верно, не догадка.
|
||||
- **AC7, механизм "в полёте побеждает".** `_reloadLayoutOnly`
|
||||
(`:4904-4948`) собирает `mine` из `_sentPos` (`:4918`), затем накладывает
|
||||
поверх ответа сервера (`:4922-4926`: `for (const [id, pos] of mine) …
|
||||
merged[id] = pos`) **до** сравнения отпечатков (`:4927-4928`). Формулировка
|
||||
AC7 и сценарий плана (пункт 8) описывают ровно этот код, не предположение.
|
||||
- Смок для пунктов 7–8 ложится в тот же файл фикстуры
|
||||
(`demo/smoke_device_position_history.mjs`), что и пункты 4–6 — не пересекается
|
||||
с `demo/smoke_layout_sync.mjs:87`, который (по находке r1) проверяет только
|
||||
дренаж `_sentPos`, а не победу при слиянии; новый пункт закрывает именно тот
|
||||
пробел, который был назван.
|
||||
|
||||
Все три закрытия — не косметика, а корректные по содержанию правки: заявленное
|
||||
в тексте совпадает с тем, что делает код.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Без повторной проверки в этом раунде принято (документ
|
||||
`docs/reviews/SPEC-REVIEW-397-r1.md`, SHA `83692e79`, дельта их не задевает):
|
||||
|
||||
- Обязательные разделы §7.1 присутствуют полностью, в правильном порядке;
|
||||
сценарий и "что человек увидит" отвечают на оба вопроса без терминов
|
||||
реализации; связь со SCOPE.md (J6, "Keep the plan true as the home evolves")
|
||||
— не изменились в дельте r2.
|
||||
- Технический разбор B3 (некорректная запись в `_layout`, нежеканонический
|
||||
отпечаток) и M1 (подстроенная фикстура смока) — воспроизведены построчно
|
||||
в r1, дельта r2 текст находок не меняет (кроме номера строки, отдельно
|
||||
перепроверен выше).
|
||||
- Граница скоупа: путь `_savePos`/`_persistLayout` (подписи комнат) багом не
|
||||
затронут — раздел "Скоуп / не-скоуп" не менялся.
|
||||
- Цитата AC10 ТЗ #74 сверена с `docs/specs/074-device-position-undo.md:254` —
|
||||
не менялась.
|
||||
- Модель данных/миграция, i18n, откат, release-артефакты — разделы не
|
||||
затронуты дельтой.
|
||||
- AC1, AC2, AC4, AC6 — однозначны, доказательство названо и прослеживается
|
||||
до плана автотестов; текст этих AC дельтой не менялся.
|
||||
- Гейты `npx tsc --noEmit`, `npm test` (1654/1655, 1 известный skip),
|
||||
`npm run build` были зелёными на `83692e79`; на `6edcde01` полный набор
|
||||
подтверждён Validate (см. ниже) — независимая проверка, не перенос старого
|
||||
результата.
|
||||
|
||||
## Как проверялось в этом раунде
|
||||
|
||||
1. Найден вердикт r1 в комментариях issue (жёлтый, Medium в скоупе) и SHA
|
||||
ревью — из тела документа `SPEC-REVIEW-397-r1.md` (в самом
|
||||
комментарии-вердикте SHA не указан, см. находку формата ниже).
|
||||
2. `git diff 83692e79..6edcde01 -- docs/specs/397-device-position-echo.md`
|
||||
— вся дельта прочитана целиком.
|
||||
3. Каждое из трёх закрытий сверено с текущим кодом на `6edcde01` (см. таблицу
|
||||
выше) — не принято на слово.
|
||||
4. Перечитаны обязательные разделы §7.1 в местах, которые правка не касалась,
|
||||
ровно настолько, чтобы убедиться в отсутствии противоречий между новым
|
||||
текстом AC5/AC7/AC3 и остальным документом (сценарий, скоуп, риски) —
|
||||
противоречий нет: раздел "Риски" уже (с r1) называл гонку с `_sentPos`
|
||||
и порядок записи как явные риски, новый AC7 их формализует, не
|
||||
противоречит.
|
||||
5. Гейты: Validate на `6edcde01` зелёный (см. ссылку в задании на этот
|
||||
раунд) — `npx tsc --noEmit`, `npm test`, `npm run build` не перегонялись
|
||||
повторно, приняты по этой ссылке.
|
||||
|
||||
## Находки
|
||||
|
||||
### Low (формат, не блокирует) — SHA не назван в комментарии-вердикте r1
|
||||
|
||||
Комментарий-вердикт r1 в issue (2026-08-30T23:19:27Z) не содержит SHA, на
|
||||
котором получен результат — вопреки практике, которой сам документ ревью
|
||||
r1 следует (`SHA ревью: 83692e79` в теле документа). SHA пришлось
|
||||
восстанавливать через документ, а не через комментарий. Не блокирует эту
|
||||
задачу (SHA найден и подтверждён), но стоит перенести в шаблон
|
||||
комментария-вердикта на будущее, чтобы не полагаться на то, что документ
|
||||
ревью всегда будет открыт вместе с комментарием.
|
||||
|
||||
Новых находок по содержанию ТЗ в дельте r2 нет: оба Low и один Medium из r1
|
||||
закрыты корректно и по существу (см. таблицу выше), новых недоказанных
|
||||
утверждений или расхождений с кодом дельта не вносит.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все три пункта закрытия r1 (Medium AC5б/AC7, Low диапазон строк, Low фраза
|
||||
"Доказательство" у AC3) — не косметические переформулировки, а точные по
|
||||
содержанию правки, проверенные построчным чтением текущего исходника.
|
||||
- Новые технические утверждения, добавленные дельтой (ветка удаления снимает
|
||||
ключ, а не заменяет значение; `_sentPos` побеждает при слиянии в
|
||||
`_reloadLayoutOnly` до сравнения отпечатков), верны и не являются
|
||||
непомеченными догадками.
|
||||
- Новые пункты плана автотестов (7, 8) целятся в код, который дельта не
|
||||
трогала, но который граничит с фиксом B3 (тот же риск, что и в r1) —
|
||||
теперь у обеих веток есть названный, воспроизводимый способ доказательства.
|
||||
- Документ внутренне непротиворечив: новый текст AC не противоречит разделам
|
||||
"Риски", "Скоуп/не-скоуп", "Модель данных".
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- `npx tsc --noEmit`, `npm test`, `npm run build` — не прогонял повторно:
|
||||
Validate на `6edcde01` зелёный (ссылка дана в задании на раунд), диф этого
|
||||
раунда не тронул код после последнего зелёного прогона.
|
||||
- `node scripts/check-docs.mjs` — не прогонялся: диф не трогает `src/**`.
|
||||
- `npm run invariants -- --config …` — не прогонялся: диф не меняет
|
||||
геометрию/`layout`/`marker.space`/`open_spans`.
|
||||
- `demo/smoke_*.mjs`, `scripts/smoke-select.mjs` — не прогонялись: продуктовый
|
||||
код и демо-фикстуры не менялись ни в r1, ни в этой дельте (это по-прежнему
|
||||
этап ТЗ, реализации ещё нет).
|
||||
- `npm run golden:verify`, `python -m pytest tests_backend -q`,
|
||||
performance-профили, `bundle:sync`/`bundle:budget` — не применимо: дельта
|
||||
документационная, ни рендер, ни `custom_components/**/*.py`, ни `dist/**`
|
||||
не затронуты.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Один Medium и два Low из r1 закрыты корректно, проверено построчно против
|
||||
кода на текущем SHA, новых High/Medium в дельте не найдено. Один Low
|
||||
(формат — SHA не назван в комментарии-вердикте r1) не блокирует и не в
|
||||
скоупе этой задачи.
|
||||
|
||||
**Зелёный.** ТЗ готово к DoR/S5-ready.
|
||||
Reference in New Issue
Block a user