houseplan-card/docs/reviews/SPEC-REVIEW-442-r1.md

SPEC-REVIEW-442-r1

• Issue: https://github.com/Matysh/houseplan-card/issues/442
• ТЗ: docs/specs/442-marker-write-rollback.md
• Этап: S4-spec-review, заход r1, блокирующих циклов израсходовано 0 из 4
• Ревьюер: Claude (роль «ревьюер ТЗ», отдельная сессия от автора)
• Материал ревью: ТЗ на коммите a8426268db90666920801f9e09554aa81343ceba (ветка issue/442-marker-write-rollback), тело issue #442 и все его комментарии на момент разбора (2026-09-03)

Скоуп разбора

Первый заход — разбор полный: продуктовая рамка (docs/SCOPE.md), процесс (PROCESS.md §2.4/§7.1, AGENTS.md), тело issue #442 и вся цепочка комментариев (аналитика → вопросы владельцу → решение владельца → готовое ТЗ), связанные issue #439 (guarded rollback общих настроек) и #441 (атомарный route CRUD), канонические docs/CONFIG-COMPATIBILITY.md и docs/VACUUM.md, терминология docs/USER-GUIDE.ru.md. Отдельно верифицированы факты о текущем коде, на которых ТЗ строит контракт: src/serialized-write-queue.ts, src/houseplan-editor-runtime.ts, src/editors/vacuum-maps-section.ts, custom_components/houseplan/validation.py, наличие названных тестов/смоков.

Как проверялось

1. Прочитаны docs/SCOPE.md, PROCESS.md (§1–§10), AGENTS.md полностью.
2. Прочитаны тело issue #442 и все 4 комментария (аналитика с оценками 8/10 польза · 9/10 разработка · 5/10 сложность · 5/10 риск · P2 · full track; пачка вопросов Q1/Q2 с default-вариантами; решение владельца, принявшее оба default; финальный комментарий со ссылкой на готовое ТЗ).
3. Прочитан весь текст docs/specs/442-marker-write-rollback.md (379 строк).
4. Прочитаны связанные issue #439 и #441 — оба уже приняты и описывают ровно тот прецедент (guarded rollback, атомарный route writer), на который ТЗ #442 ссылается как на эталон и явно не дублирует.
5. Прочитан docs/CONFIG-COMPATIBILITY.md целиком — не найдено противоречий между заявленным «revision + content fingerprint» guard'ом и уже задокументированным контрактом ревизий (#340), геометрии (#282/#306/#224) и маршрутов пылесоса (#162).
6. Прочитан docs/VACUUM.md целиком — раздел «Calibration» действительно не документирует порядок toast/подтверждение относительно ответа сервера, то есть заявленный в ТЗ release-артефакт (обновление VACUUM.md) не дублирует уже существующий текст, а закрывает реальный пробел.
7. Проверена терминология docs/USER-GUIDE.ru.md: «Редактор устройств», «Карты и этажи», «ручная подгонка», «Применить» — используемые в ТЗ термины (кроме одного, см. Low ниже) не расходятся с интерфейсом.
8. Делегирована фактологическая проверка кода отдельному агенту (Explore) по 9 конкретным пунктам — см. «Верификация фактов» ниже. Каждый пункт перепроверен по конкретным строкам файла, не на веру.
9. Самостоятельно прочитан src/serialized-write-queue.ts целиком (60 строк) — независимо подтверждён guard: host._cfgRev !== attempt.revision || (current !== attempt.attempted && fingerprint(current) !== attempt.attemptedFingerprint), то есть ревизия и контент-фингерпринт одновременно, включая ветку одного and того же object identity — именно то, что ТЗ требует мутационно проверить в AC2.
10. Самостоятельно найден _writeChain/expected_rev (src/houseplan-card.ts:7662-7730, src/houseplan-editor-runtime.ts:1175 и далее) — сериализация запросов, на которую ссылается контракт §2, существует и используется.

Верификация фактов ТЗ против текущего кода (HEAD a8426268)

#	Утверждение ТЗ/issue	Результат
1	_saveMarker() мутирует cfg.markers до await _saveConfigNow(), catch только снимает busy и показывает toast	Подтверждено — src/houseplan-editor-runtime.ts:8270-8285 мутация, :8330 await, catch :8359-8366 без восстановления _serverCfg и без rebuild devices
2	(из тела issue, не из ТЗ) готовый guarded rollback применён «ровно к одному пути — общим настройкам (_updateDecorStyle)»	Неточность в тексте issue. Пара optimisticAttempt/rollbackOptimistic действительно встречается 3 раза (импорт + одна пара вызовов), но пара живёт в _saveSettingsDialog (:10231-10275, вызовы на :10260/:10271), а не в _updateDecorStyle (:10204) — та не содержит optimistic-логики вовсе и делегирует в _persistDecorStyle → обычный debounced _saveConfig(). Счётчик «18 вызовов _saveConfig()» корректен
3	_vacSaveMatrix() мутирует in-place, коллеры показывают success toast и закрывают UI до подтверждения записи сервером	Подтверждено — мутация m.vacuum, _maybeRebuildDevices(), невawait-нутый debounced _saveConfig(); toast.autocal_done/toast.cal_done показываются сразу после синхронного (boolean) возврата _vacSaveMatrix, без ожидания сервера
4	Route CRUD в vacuum-maps-section.ts уже использует optimisticAttempt/rollbackOptimistic (эталон #441)	Подтверждено — импорт :21, вызовы :105/:116
5	Guard сравнивает revision и content fingerprint; есть _writeChain/expected_rev	Подтверждено самостоятельно (см. «Как проверялось» п.9-10)
6	Четыре семантических валидатора маркера существуют под этими именами	Подтверждено ровно с этими именами (кроме generic «проверка controls», которую ТЗ и не называет точным именем) — validate_marker_value_badges (:811), validate_marker_vacuum_routes (:886), validate_marker_light_entities (:930), validate_marker_controls (:972, цикл — :1049) в custom_components/houseplan/validation.py
7	Названные в плане тестирования файлы существуют	Подтверждено — все 6: test/serialized-write-queue.test.mjs, demo/smoke_vacuum_route_draft.mjs, demo/smoke_vacuum_firstuse.mjs, demo/smoke_vacuum_multifloor.mjs, demo/smoke_vacuum.mjs, demo/smoke_dialog_zombie.mjs
8	docs/VACUUM.md ещё не документирует reject/success UX калибровки	Подтверждено — раздел «Calibration» молчит именно об этом
9	scripts/mutation-gate.mjs существует	Подтверждено

Итог верификации: единственное расхождение с кодом (пункт 2) находится в теле issue, не в тексте ТЗ — сам документ docs/specs/442-...md нигде не называет _updateDecorStyle и не строит контракт на этом имени; он ссылается на механизм обобщённо («общий writer», «уже готовый optimisticAttempt/ rollbackOptimistic»), что после проверки оказалось верным независимо от того, в какой именно функции этот механизм физически живёт. Так что дефект не проникает в исполнимость ТЗ — см. Low ниже, почему он всё равно стоит записи.

Соответствие §7.1

Все обязательные разделы присутствуют: Сценарий · Что человек увидит до и после · Подтверждённая проблема · Скоуп/Не-скоуп · Контракт поведения (6 подпунктов) · Данные и совместимость · Touch/клавиатура/доступность · Ошибки и крайние случаи · AC1–AC9 с доказательством (unit/smoke/test double у каждого) · План тестирования · Карта реализации · Риски и rollback · Release-артефакты · Принятые предположения.

Владелец отвечал только на продуктовые вопросы (Q1 — какие marker-пути входят в атомизацию; Q2 — что делать с calibration UI при отказе), оба с default-вариантом и явным принятием; технические решения (immutable candidate, где именно живёт guard, разбиение на pure-модуль) автор резервирует себе явным блоком «Принятые предположения» в конце — ровно так, как требует процесс.

Находки

Low-1. Неточное имя функции в теле issue (не в ТЗ)

Issue #442 в разделе «Проблема» называет уже реализованный guarded-rollback путь _updateDecorStyle; фактически это _saveSettingsDialog (src/houseplan-editor-runtime.ts:10231). _updateDecorStyle — соседняя функция без optimistic-логики. Ошибка не искажает контракт ТЗ (там имя функции не упоминается вовсе, только «общий writer» и сам механизм), поэтому не блокирует и не требует правки ТЗ. Фиксирую как заметку для реализации: если разработчик будет ориентироваться на текст issue при поиске «уже готового пути», неверное имя может стоить нескольких минут поиска не там — не более того.

Решение ревьюера: снимается без правки, запись сделана для истории раунда.

Low-2. Явного раздела «i18n» нет — контент есть, но не под заголовком

§7.1 перечисляет i18n отдельным обязательным разделом. В ТЗ #442 подтверждение «новых строк нет» разнесено по двум местам («Новых targets, жестов и строк i18n нет» в разделе Touch; «новых текстов ошибок... не добавляется» в Не-скоуп) без отдельного заголовка. По содержанию требование выполнено (эксплицитно и дважды непротиворечиво), поэтому не блокирует переход в S5-ready — DoR требует «i18n: ключи en+ru перечислены», а перечислять здесь нечего, что и сказано явно. Отмечаю как стилистическую придирку, не находку, которая должна кого-то останавливать.

Решение ревьюера: снимается без правки.

Что проверено и корректно

• Продуктовая рамка. Задача закрывает J6 (docs/SCOPE.md: «Keep the plan true as the home evolves» — «View не должен показывать конфигурацию, которую сервер отверг»), сценарий и «что человек увидит» — оба заполнены без терминов реализации первым предложением, персона (Home admin) и поверхность (Device editor + calibration overlay) названы.
• Скоуп/не-скоуп зеркалят принятые owner defaults Q1/Q2 дословно, без расширения и без урезания против решения владельца.
• Технические утверждения о текущем коде — 8 из 9 проверенных фактов подтверждены чтением реальных строк; единственное расхождение (Low-1) не влияет на сам контракт ТЗ.
• Контракт поведения опирается на реально существующий примитив (optimisticAttempt/rollbackOptimistic с двойным guard'ом ревизия+ фингерпринт, уже обкатанным в #441 и в _saveSettingsDialog), а не изобретает новый механизм с нуля — минимизирует риск.
• AC1–AC9 каждый указывает способ доказательства (unit / browser smoke / test double) и достаточно конкретен для написания теста без дополнительных решений: например, AC2 явно называет два обязательных мутанта (unconditional rollback; пропуск fingerprint для same identity), предвосхищая требование код-ревью §2.7 о «таблице чем краснеет». Формулировки допускают ровно одну трактовку каждая — не нашёл AC, который можно закрыть двумя разными реализациями с разным видимым поведением.
• Регрессионный периметр (AC8, таблица «Ошибки и крайние случаи», строка «route CRUD #441 reject/retry») явно защищает уже принятый атомарный route writer от повторной реализации или порчи — соответствует правилу «не подменять существующий механизм».
• Данные и совместимость: явно «миграции нет», persisted schema не меняется, что соответствует характеру задачи (только client-side транзакционность) и не противоречит ни одному разделу CONFIG-COMPATIBILITY.md.
• Touch/доступность: busy — настоящий disabled, а не только визуальный индикатор; фокус остаётся в открытом диалоге — соответствует TOUCH-SUPPORT.md (редакторы desktop-first, но контракт не деградирует специально под touch).
• Release-артефакты называют оба changelog, CONFIG-COMPATIBILITY.md, VACUUM.md (подтверждено выше, что там действительно есть пробел для заполнения), условие для docs-скриншотов и явное «golden не нужен, если подходящей канонической поверхности нет» — не оставляет открытых пунктов DoR.
• Догадки, выданные за решения: не найдено. Каждое место, где решение не следует напрямую из существующего документа, либо помечено как default-ответ владельца (Q1/Q2), либо вынесено в явный блок «Принятые предположения» в конце ТЗ, что ревьюер вправе (и не находит повода) оспорить.

Чего не проверял

• Не запускал npx tsc --noEmit/npm test/npm run build — на этапе ревью ТЗ кода ещё нет (это этап spec, не code), гейты §8 к этому документу неприменимы.
• Не проверял мутационные сценарии AC2 практически (мутант ещё не написан — это работа код-ревью, не спек-ревью); отметил только, что защитный примитив уже существует и уже используется в проде (#441).
• Не проверял браузерные смоки исполнением — только их наличие как файлов (см. таблицу верификации, п.7). Их фактическое покрытие AC — задача реализации и код-ревью.
• Не проверял docs/specs/README.md на предмет обязательной строки в таблице issue↔ТЗ — это релиз-артефакт автора ТЗ, не блокирует ревью.

Вердикт

Зелёный. High: 0, Medium: 0. Обе Low-находки сняты без правки текста ТЗ (записаны выше для истории раунда, не блокируют). ТЗ полно по §7.1, каждый AC однозначен и снабжён способом доказательства, продуктовые вопросы решены владельцем до написания ТЗ, а технические утверждения о существующем коде подтверждены независимым чтением, а не приняты на веру.

Материал раунда

• Ветка: issue/442-marker-write-rollback
• SHA ТЗ: a8426268db90666920801f9e09554aa81343ceba
• Путь ТЗ: docs/specs/442-marker-write-rollback.md
• Найти дерево: git log --all --format='%H %T' | grep <дерево HEAD>
• Найти блоб ТЗ: git log --all --find-object=<блоб> -- docs/specs/442-marker-write-rollback.md

---

Материал раунда

• Ветка: issue/442-marker-write-rollback, коммит a8426268db90 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 5d3fbe71add69e722a9f6d87b1ce04dcea586729

  git log --all --format='%H %T' | grep 5d3fbe71add6
  

• ТЗ docs/specs/442-marker-write-rollback.md, блоб a9dcca2ff269a1c309387902875a4937efbd4ebf

  git log --all --find-object=a9dcca2ff269a1c309387902875a4937efbd4ebf -- docs/specs/442-marker-write-rollback.md