diff --git a/docs/reviews/SPEC-REVIEW-397-r2.md b/docs/reviews/SPEC-REVIEW-397-r2.md new file mode 100644 index 00000000..ea7fb58d --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-397-r2.md @@ -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.