mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 13:18:58 +00:00
@@ -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 <SHA>..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 она не блокирует.
|
||||
|
||||
**Вердикт: зелёный.**
|
||||
Reference in New Issue
Block a user