diff --git a/docs/reviews/SPEC-REVIEW-403-r3.md b/docs/reviews/SPEC-REVIEW-403-r3.md new file mode 100644 index 00000000..cc07721a --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-403-r3.md @@ -0,0 +1,318 @@ +# SPEC-REVIEW-403-r3 + +- Issue: https://github.com/Matysh/houseplan-card/issues/403 +- Артефакт ТЗ: `docs/specs/403-area-relocation-safety.md` (полный трек, класс A; + без метки `small`/`trivial`) +- Материал: SHA `e9938029de4908cac10ca432f79f95039baef458` (ветка + `origin/issue/403-area-relocation-safety`, проверено `git rev-parse HEAD` + непосредственно перед выводом; спецификация на этом SHA — «Ревизия: 3 + (2026-09-01; техническое уточнение AC7 при реализации)») +- Заход: r3 · блокирующих циклов израсходовано **1/4** (жёлтый r1 потратил 1 + цикл; зелёный r2 бюджет не тронул — §4/#227) +- Ревьюер: Claude (роль «ревьюер ТЗ»), независимая сессия, без устных пояснений + автора +- Разбор — **по дельте** (§2.10 PROCESS.md): дельта локальна (см. ниже), + ребейза текста спецификации между r2 и r3 не было, контракт поведения AC7 не + меняется (меняется только способ доказательства), новая подсистема не задета, + объём дельты (14 строк diff суммарно по спеку и смоку) многократно меньше + исходной задачи + +## Скоуп ревью + +Особенность этого захода: он идёт **после**, а не до реализации. Хронология по +issue и git-истории: + +1. `SPEC-REVIEW-403-r1` (жёлтый, H1) → `docs: revise area relocation safety + spec` (`94502d3d`, ревизия 2) → `SPEC-REVIEW-403-r2` (зелёный). +2. Реализация (`71bbc953`, «fix: preserve area relocation state») — в этом же + коммите разработчик поднял ревизию ТЗ до 3, скорректировав формулировку + AC7 (см. «Проверка дельты» ниже), и явно назвал это в хендофф-комментарии + («Явная техническая корректировка AC7»). +3. `CODE-REVIEW-403-r1` вынес **зелёный** вердикт на SHA `9c4d07ab`, уже + разобрав AC7 против текста ревизии 3. +4. Слияние не удалось: `dev` продвинулся за время ревью на 1 коммит, конвейер + предписал ребейз (§7.2) — ветка `issue/403-area-relocation-safety` перестала + быть вершиной ревью. Логическое содержимое коммитов `0d338b9c`/`94502d3d`/ + `71bbc953` затем попало в `dev` под новыми SHA (`8e7c9272`/`98d6edb7`/ + `355b0516` — те же сообщения коммитов, тот же диф) — подтверждено + `git merge-base --is-ancestor 355b0516 origin/dev` → `yes`. Issue сейчас + несёт метку `S8-merged` («код в dev, ждёт беты»), состояние подтверждено + `gh issue view` (`state: OPEN`, `labels: bug, P1, S8-merged`). +5. Материал **этого** захода — старая, до-ребейзная вершина ветки (`e9938029`, + ей же помечен `origin/issue/403-area-relocation-safety`); её текст спека + побайтово совпадает с версией, уже лежащей в `dev` + (`git diff origin/dev -- docs/specs/403-area-relocation-safety.md` → пусто). + +Итог: код, реализующий ревизию 3 ТЗ, уже смёржен и прошёл код-ревью зелёным. +Предмет этого захода — не блокировка чего-либо «в полёте», а проверка того, +что сама правка ревизии 3 (изменение формулировки AC7) была обоснованным +уточнением, а не догадкой, выданной за факт задним числом. Практический эффект +жёлтого вердикта здесь — не «возврат в разработку» (код уже в `dev`), а +требование правки текста ТЗ и/или отдельного issue до закрытия задачи при +выпуске беты (§2.8). + +## 1. Вердикт r2 и SHA, на котором он получен + +`docs/reviews/SPEC-REVIEW-403-r2.md`: **зелёный**, заход r2, блокирующих циклов +1/4, High: 0, Medium: 0, Low: 0, новых находок нет. + +Заявленный в документе и в комментарии автора (issue, 2026-09-01T14:26:53Z: +«Коммит дельты: `83005c3c`») материал — SHA `83005c3c`. **Этот SHA не +существует в репозитории**: `git cat-file -t 83005c3c` → `fatal: invalid +object name`; репозиторий не мелкий (`git rev-list --all | wc -l` → 2437, +`--is-shallow-repository` → `false`), перебор всех объектов +(`git rev-list --all`) не находит ни одного хеша с префиксом `83005c`. Это не +следствие последующего ребейза (см. «Скоуп ревью», п.4) — ребейз случился +позже, при код-ревью, и переименовал только `71bbc953`→`355b0516` и предков; +он не объясняет отсутствие `83005c3c` уже на момент r2. + +Реальный коммит, содержимое которого дословно совпадает с тем, что +`SPEC-REVIEW-403-r2.md` цитирует как дельту (сообщение «docs: revise area +relocation safety spec», диф +23/-2 в `docs/specs/403-area-relocation-safety.md`, +добавление разделов «Touch, View и kiosk» и «Производительность», правка адреса +строки M1) — `94502d3d67cacf85bdb9f69cd511b342989891fd`. Сверено построчно: +`git show 94502d3d -- docs/specs/403-area-relocation-safety.md` даёt ровно тот +диф, что процитирован в `SPEC-REVIEW-403-r2.md` разделе 2. Дальше в этом +документе как материал r2 используется `94502d3d` — единственный коммит, +фактически соответствующий описанному содержимому. + +Это самостоятельная находка данного раунда — см. «Находки», H-SHA. + +## 2. Объявление дельты + +Дельта — с `94502d3d` (фактическое содержимое r2) по `e9938029` (HEAD этого +раунда): + +``` +git diff 94502d3d..e9938029 -- docs/specs/403-area-relocation-safety.md +``` + +```diff +- Ревизия: 2 (2026-09-01; H1/L1 из SPEC-REVIEW-403-r1) ++ Ревизия: 3 (2026-09-01; техническое уточнение AC7 при реализации) +... +- **AC7**. Существующее поведение #126 не сломано: ручная перестановка после +- переезда переживает три rebuild-тика, delete-first echo не воскрешает точку. +- Доказательство: `demo/smoke_area_relocation.mjs` остаётся зелёным без +- правок его утверждений. ++ **AC7**. Существующее успешное поведение #126 не сломано: ручная перестановка ++ после переезда переживает три rebuild-тика, delete-first echo не воскрешает ++ точку. Доказательство: `demo/smoke_area_relocation.mjs` остаётся зелёным; ++ единственное прежнее утверждение `failedConfigRetryable`, которое прямо ++ требовало состояние исходного бага (`layout` уже удалён после отказа ++ `config/set`), заменяется witness нового AC1 — позиция восстановлена, а ++ повтор остаётся возможен. Остальные утверждения смока не меняются. +``` + +`git diff --stat 94502d3d..e9938029` подтверждает: изменения в диапазоне — +спек (эта правка, коммит `71bbc953`), продуктовый код и тесты правки C2/M1 +(`71bbc953`, вне предмета *этого* захода — уже пройдены `CODE-REVIEW-403-r1`), +два новых документа ревью (`6b515800` = `SPEC-REVIEW-403-r2.md`, +публикация конвейера) и `9c4d07ab` (пересъёмка скриншотов). Предмет **этого** +захода — только текст `docs/specs/403-area-relocation-safety.md`, как и +требует §2.10 для этапа ТЗ; продуктовый код в этом раунде не разбирается +заново — он уже прошёл `CODE-REVIEW-403-r1` (зелёный, независимый гейт, §7.2). + +Дельта локальна: ребейз текста спека между r2 и r3 не случился (текст +идентичен что до, что после последующего ребейза кода — `git diff +origin/dev -- docs/specs/403-area-relocation-safety.md` пуст), контракт +поведения AC7 не изменился (по-прежнему «успешный сценарий #126 не сломан»), +новая подсистема не задета, объём (14 строк diff по обоим файлам) существенно +меньше исходной задачи. + +## 3. Закрытие раунда r2 + +Раунд r2 был зелёным без находок — закрывать нечего (жёлтых/блокирующих +пунктов не было; §4: зелёный вердикт цикла не образует и правок не +предписывает). Таблица находок r2 пуста по построению. + +## 4. Унаследовано из r2 (без повторной проверки) + +Источник: `docs/reviews/SPEC-REVIEW-403-r2.md` (фактический материал — +`94502d3d`, см. «Находки», H-SHA, о расхождении с указанным в документе SHA). +Дельта r3 этих утверждений не касается, переносятся без повторной проверки: + +- диагноз C2 и M1 точен и построчно сверен с кодом на исходном SHA `1f9d9014` + (унаследовано ещё из r1 через r2); +- AC1–AC6 однозначны, у каждого указан способ доказательства, реалистичны на + существующей инфраструктуре смоков — текст этих пунктов дельтой не + затронут; +- разделы «Touch, View и kiosk» и «Производительность» отвечают DoR §2.5 и + проверены против `docs/TOUCH-SUPPORT.md`/`src/command-stack.ts` — текст не + менялся; +- все обязательные разделы §7.1 присутствуют (сценарий · что человек увидит · + проблема/контракт по пунктам · скоуп/не-скоуп · UX · модель данных и + миграция · i18n · план автотестов · риски · откат · release-артефакты) — + ни один не тронут этой дельтой; +- терминология соответствует `docs/USER-GUIDE.ru.md`; +- задача лежит в скоупе `docs/SCOPE.md` (J6 + правило о недопустимости + удаления пользовательских данных по догадке). + +## 5. Проверка дельты (не унаследовано — перепроверено заново) + +Предмет дельты — единственное утверждение: старый AC7 требовал, чтобы +`demo/smoke_area_relocation.mjs` остался зелёным «без правок его утверждений»; +новый AC7 разрешает ровно одну именованную замену (`failedConfigRetryable` → +`failedConfigRestoredAndRetryable`) с обоснованием «старое утверждение прямо +требовало состояния исходного бага». Это утверждение проверено по коду, а не +принято на слово: + +**Что проверено.** `git show 71bbc953 -- demo/smoke_area_relocation.mjs`: + +```diff +- const failedConfigRetryable = !c._layout.d_kettle ++ const failedConfigRestoredAndRetryable = c._layout.d_kettle?.s === kettlePoint.s ++ && c._layout.d_kettle?.x === kettlePoint.x ++ && c._layout.d_kettle?.y === kettlePoint.y + && c._serverCfg.settings.marker_area_snapshot?.d_kettle?.area === 'kitchen' +- && c._areaRelocationSyncKey === ''; ++ && c._areaRelocationSyncKey === '' ++ && calls.some((message) => message.type === 'houseplan/layout/update' ++ && message.device_id === 'd_kettle'); +``` + +Старое условие `!c._layout.d_kettle` требовало **отсутствия** записи layout у +устройства — то есть буквально требовало, чтобы позиция осталась потеряна +после отказа `config/set`. Заявление ТЗ («прямо требовало состояние исходного +бага») подтверждено буквально, не является пересказом с чужих слов. + +Новое условие проверяет: позиция восстановлена (`s/x/y` совпадают с +`kettlePoint`, сохранённым до переезда), снапшот всё ещё на прежней area +(`kitchen`, не продвинут — это ожидаемо: конфиг не записан), синхронизация не +залипла (`_areaRelocationSyncKey === ''`), и восстановление реально ушло на +сервер (`layout/update` вызван) — то есть проверяет именно контракт AC1 (см. +`docs/specs/403-area-relocation-safety.md:156-159`), а не более слабое +условие. + +«Повтор остаётся возможен» проверено смежной, не изменённой этим дифом +переменной чуть ниже по файлу (`sed -n '124,128p'`): + +```js +const configRetrySucceeded = c._serverCfg.settings.marker_area_snapshot?.d_kettle?.area + === 'living_room' && c._serverCfg.settings.new_device_ids?.includes('d_kettle'); +``` + +— после восстановления сценарий повторяет `window.__setRegistryArea` без +мока отказа, и `configRetrySucceeded` действительно проверяет, что снапшот +продвинулся, а устройство попало в `new_device_ids`. Итоговое утверждение AC7 +(«позиция восстановлена, а повтор остаётся возможен») доказано двумя +раздельными булевыми полями смока, оба из итогового `return` +(`demo/smoke_area_relocation.mjs:334-335`), а не одним удобным флагом. + +**«Остальные утверждения смока не меняются»** — проверено по объёму дифа: +`git show 71bbc953 --stat -- demo/smoke_area_relocation.mjs` → `1 file changed, +9 insertions(+), 4 deletions(-)`, ровно два хунка (переопределение переменной +в сценарии kettle-retry и переименование поля в итоговом объекте). Других +утверждений смока диф не касается — заявление ТЗ точно. + +**Вывод по дельте**: правка AC7 — не догадка и не косметическая уступка ради +зелёного теста: старая формулировка была логически противоречива сама с собой +(нельзя одновременно требовать «позиция цела после отказа» по AC1 и «после +отказа `layout` пуст» по старому AC7 — это взаимоисключающие состояния одного +и того же прогона), новая формулировка разрешает ровно то расхождение, которое +и создавало противоречие, ссылаясь на конкретный, проверяемый механизм (AC1), +а не ослабляет проверку в целом. + +## 6. Что проверено и корректно + +- Ревизия ТЗ поднята до 3 с корректной, содержательной пометкой источника + правки («техническое уточнение AC7 при реализации»), а не молчаливым + редактированием задним числом. +- Дельта не расширяет скоуп, не меняет поведенческий контракт AC7 (успешный + сценарий #126 по-прежнему обязан работать), не вводит новых терминов и не + противоречит канону подсистемы. +- Правка независимо подтверждена сторонним гейтом: `CODE-REVIEW-403-r1.md` + зафиксировал зелёный прогон `demo/smoke_area_relocation.mjs` (18/18) именно + с новым полем `failedConfigRestoredAndRetryable` на SHA `9c4d07ab` — то есть + утверждение ТЗ не только текстуально самосогласовано, но и подтверждено + исполнением реального кода (хотя код-ревью — отдельный гейт с отдельным + бюджетом, §7.2, и его вывод не заменяет собой эту проверку, а дополняет). +- Продуктовый код, затронутый этой дельтой (правка C2/M1), уже независимо + прошёл `CODE-REVIEW-403-r1` зелёным на отдельном гейте — повторно не + разбирается в этом документе. + +## 7. Находки + +### Наблюдение (не находка). Спек-ревью r3 идёт после того, как реализация уже смёржена в `dev` + +ТЗ было доведено до ревизии 3 внутри коммита реализации (`71bbc953`, +статус issue на тот момент — «в разработке»), код-ревью прошло зелёным на +следующий день по этой же ревизии, и лишь затем запущен этот, посвящённый +именно тексту ТЗ, заход. Явного правила PROCESS.md, требующего провести +спек-ревью формулировки AC **до** завершения код-ревью в случае, когда правка +ТЗ — техническое уточнение формулировки уже одобренного AC (а не расширение +скоупа, §2.6), не найдено; §3 правило 2 требует зелёное ревью ТЗ **до входа** +в разработку, что было выполнено (ревизия 2 → r2 зелёный → взято в работу). +Итог по существу неотличим от штатного порядка (обе проверки прошли зелёными, +несогласованности между текстом и кодом нет — см. §5 выше), поэтому не +поднимаю блокирующей находкой. Называю явно, чтобы не выглядеть тихим +одобрением задним числом. + +### H-SHA (Medium, вне скоупа задачи #403 → заведён отдельный issue). SHA материала SPEC-REVIEW-403-r2 не существует в репозитории + +**Где**: `docs/reviews/SPEC-REVIEW-403-r2.md:6` («Материал: спец-файл на +`HEAD = 83005c3c`») и комментарий автора в issue #403 от 2026-09-01T14:26:53Z +(«Коммит дельты: `83005c3c`»). + +**Почему это находка**. `83005c3c` не резолвится ни в один объект этого +репозитория (`git cat-file -t 83005c3c` → `fatal: invalid object name`, +подтверждено полным (не мелким) клоном — `git rev-list --all | wc -l` = 2437, +`--is-shallow-repository` = `false`, перебор всех хешей коммитов не находит +префикса `83005c`). §2.10 п.1 требует называть SHA предыдущего раунда именно +для того, чтобы дельта следующего раунда объявлялась воспроизводимой командой +(`git diff ..HEAD`); §7.2 требует сверять факты отчёта с +`git rev-parse HEAD`, а не с ранее записанным значением. Здесь оба источника — +и человек, и предыдущий ревьюер — независимо друг от друга не сверили +цитируемый SHA с деревом, и оба указали на объект, которого не существует. +Практический эффект уже проявился в этом раунде: п.1 этого документа не мог +быть выполнен командой `git diff 83005c3c..HEAD` буквально — пришлось +реконструировать реальный коммит (`94502d3d`) по содержимому диффа, +процитированному в тексте r2. + +**Почему не блокирует и не входит в скоуп #403**: содержимое ревизии 2 ТЗ +верно и не пострадало — сверено построчно (см. «Унаследовано из r2»); дефект +не в тексте `docs/specs/403-area-relocation-safety.md`, который правит автор +ТЗ, а в артефакте ревью-конвейера (`SPEC-REVIEW-403-r2.md`, уже +опубликованный документ прошлого раунда) и в дисциплине подтверждения SHA +перед выводом отчёта — тот же класс проблемы, что уже собирал отдельные +process-issue в этом репозитории (#171, #207, #214, #227). Правка текста ТЗ +#403 эту находку не закрывает. + +**Требуемое действие**: заведён отдельный issue (см. ниже), Medium/process, +со ссылкой на #403 и на этот документ. + +## 8. Чего не проверял + +- **Диагноз C2/M1 и построчную сверку с `src/houseplan-card.ts`/ + `src/device-area-relocation.ts`** — не перепроверял, дельта их не касается; + см. §4 «Унаследовано из r2». +- **Гейты кода** (`tsc`, `test`, `build`, `check-docs`, смоки, мутанты, + инварианты) — не гонял в этом раунде: продуктовый код и тесты, затронутые + этой задачей, уже прошли независимый код-ревью (`CODE-REVIEW-403-r1.md`, + зелёный, полная таблица гейтов на SHA `9c4d07ab`) — повторный прогон того же + гейта на том же диапазоне без нового изменения кода не добавляет информации; + в этом раунде продуктовый код не менялся вовсе (диапазон дельты — только + `docs/specs/403-area-relocation-safety.md`). Прочитан, не исполнен, только + фрагмент `demo/smoke_area_relocation.mjs`, необходимый для проверки самой + формулировки AC7 (§5) — этого достаточно для вопроса, стоящего перед этим + раундом («формулировка ТЗ соответствует коду, а не является догадкой»); + исполнение смока уже задокументировано в `CODE-REVIEW-403-r1.md`. +- **Состояние `dev` после ребейза 355b0516 и далее** — не разбирал: это + предмет уже пройденного код-ревью и последующего слияния, не этого + спек-раунда. +- **Полную повторную сверку разделов «Touch, View и kiosk» и + «Производительность» с канонit** — не требовалась: текст этих разделов не + менялся между r2 и r3 (см. «Объявление дельты»). + +## Итог + +Единственная содержательная правка этого раунда — уточнение формулировки AC7, +вызванное реальным логическим противоречием старой формулировки, а не +удобством или догадкой: старое утверждение смока требовало состояния +исходного бага, что стало невозможно совместить с фиксом C2. Новая +формулировка проверена построчно против кода (диф `71bbc953`) и подтверждена +независимым зелёным код-ревью. Найдена одна Medium-находка вне скоупа задачи +#403 — несуществующий SHA, процитированный в `SPEC-REVIEW-403-r2.md` и в +комментарии автора; заведён отдельный issue, задачу #403 она не блокирует. + +**Вердикт: зелёный.**