houseplan-card/docs/reviews/SPEC-REVIEW-625-r2.md

SPEC-REVIEW-625-r2

Issue: #625 — Backend: блокирующий I/O в import/apply под write_lock, экспорт держит лок на хэширование, нет инварианта уникальности markers[].id, квота upload после стриминга. Этап: S4-spec-review · заход r2 · трек: full (унаследовано из r1, не пересматривалось — обоснование не изменилось: несколько независимых backend- поверхностей, влияние на конкурентность/производительность, изменение контракта валидации конфигурации, сложность/риск 8/10). Ревьюер: независимая сессия, без контекста автора ТЗ и без контекста сессии r1. Материал: тело issue #625, раздел ## ТЗ, как оно читается на момент этого ревью (2026-09-23). Issue открыт, метка S4-spec-review.

---

Скоуп раунда — обзор по дельте (PROCESS.md §2.10)

Предыдущий раунд (r1, документ docs/reviews/SPEC-REVIEW-625-r1.md) вынес жёлтый вердикт: High 0, Medium 1 (M1, в скоупе), Low 2 (L1, L2 — обе сняты ревьюером r1 записью, без блокирующего цикла). Автор ответил комментарием владельца (Matysh, 2026-09-23T09:49:36Z): «ТЗ обновлено по Medium ревью r1. Принято недеструктивное delta-aware правило... Также явно зафиксированы i18n и touch/mobile как неприменимые поверхности.»

Объявление дельты. ТЗ живёт в теле issue, не в файле — формального git diff нет. Раунд r1 не зафиксировал сырой снимок тела в at-rest виде, пригодном для побайтового diff (блок якорей содержит только sha256 тела, не сам текст), поэтому дельта восстановлена по двум независимым источникам, которые согласуются между собой:

1. Дословные находки и цитаты кода из документа r1 (что именно было названо недостающим/сформулированным иначе);
2. Комментарий владельца, прямо перечисляющий, что изменено.

Контрольная проверка, что тело действительно правилось (а не то, что старый документ ошибочно интерпретировал неизменившийся текст): текущее тело не хэшируется в sha256 якоря r1 (c453dfbbd8e968467dd6567e6327005b625432bec7f0be055fdc6ac85700c910) ни в одной проверенной нормализации (сырой байтовый sha256 1cb98ccb309f4ba905a678b5d8cd7a989eebb9e2ed7456b1014d2c0738eae345, CRLF→LF 331e822cbb4523b5248c171b3d218184231c398181d3d7c33aae9c33ad2ffefb, с обрезкой пробелов f9d2f583a9c977bd535f6f14de24ad8c8757914db26218bea46875ad89fbaa67) — тело физически другое, редактирование подтверждено, а не заявлено на слово.

Дельта локальна: правка сосредоточена в разделе «Инвариант markers[].id и восстановление legacy-данных» (переименован и переписан) плюс две добавленные строки в «Принятых предположениях» (i18n, touch). Остальные разделы ТЗ (сценарии 1,2,4,5; «Не входит», кроме уже обсуждённого пункта; AC1, AC2, AC4–AC13; release-артефакты; риски вне M1) не тронуты — дельта не достигает новой подсистемы, не меняет track, не сопоставима по объёму с исходной задачей. Разбор по дельте оправдан по критерию §2.10.

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

1. Перечитал docs/SCOPE.md, AGENTS.md, PROCESS.md §1–§9 (полностью, не по памяти — трек, DoR, лимит циклов, формат вердикта, §2.10 по пунктам).
2. Перечитал документ r1 целиком, включая таблицу построчной проверки кода, находки M1/L1/L2, разделы «Что проверено» и «Чего не проверял».
3. Получил текущее тело issue (gh issue view 625 --json body,comments) и сверил каждую фразу изменённого раздела 3 против того, что M1 требовал закрыть — построчно, см. таблицу «Закрытие раунда r1» ниже.
4. Перепроверил внутреннюю согласованность изменённого раздела с непосредственно соседними разделами, которые он логически обязан не нарушить: «Не входит» (запрет авто-удаления/слияния), «Данные и совместимость» (нет новых полей/version bump), AC3 (свидетели), «Принятые предположения» (определение «структурно неизменённой группы»).
5. Продуктовым рассуждением проверил сценарий отказа из M1 — переживает ли администратор с уже испорченной конфигурацией первую же несвязанную правку после выката; проверил, остаётся ли путь починки достижимым существующим редактором (без новых UI-элементов, которые ТЗ прямо исключает).
6. Гейты (typecheck/test/build) не запускал: на стадии S4-spec-review продуктовый код не менялся (Rule #1) — это подтверждено чтением git log рабочей копии (последние коммиты — документы ревью #625/#639, ни один не касается custom_components/**/*.py или src/**); гонять гейты означало бы проверять пустое множество, тот же принцип что в r1 и в SPEC-REVIEW-89-r1.

Закрытие раунда r1

Находка r1	Чем закрыта	Где это видно
M1 (Medium, в скоупе) — новый инвариант markers[].id не учитывает уже испорченную хранимую конфигурацию: любая последующая несвязанная правка (config/set, layout/set) после выката начала бы падать vol.Invalid, «Не входит» запрещал единственный автоматический выход, а явного решения владельца в тексте не было	Раздел переписан на явное delta-aware правило с previous config: «новый duplicate active marker id всегда отклоняется; неизменённая группа активных legacy-дубликатов может пройти через несвязанное сохранение без автоматической потери данных; добавление, удаление или изменение любой записи в такой группе допустимо только если candidate оставляет для id не более одного активного маркера». Это выбор варианта (в) из трёх, предложенных r1 («сузить инвариант так, чтобы он не блокировал уже существующие несвязанные записи»), явно принятый автором-владельцем issue, а не тишиной. Путь починки не требует новых UI-элементов: «существующий редактор/экспорт остаётся доступен» — обычное удаление/правка одного из дублирующих маркеров через уже существующий редактор оставляет ≤1 активный и проходит по тому же правилу	Тело issue, раздел «Инвариант markers[].id и восстановление legacy-данных» (весь раздел, включая «CONFIG_SCHEMA сохраняет permissive read-совместимость... целевой invariant применяется previous-aware валидатором на write/import boundaries»); AC3 переписан в тех же терминах («разрешают несвязанную правку при структурно неизменённой legacy-группе»); «Данные и совместимость»: «Несвязанное сохранение разрешено, пока конфликтующая группа структурно сохранена»; «Принятые предположения»: точное определение «структурно неизменённой группы» (перестановка массива без изменения записей — не новый конфликт); комментарий владельца от 2026-09-23T09:49:36Z подтверждает это как явное продуктовое решение
L1 (Low, снята r1 без цикла) — сценарии не называют персону/поверхность явно, смешивают результат с терминами реализации	Не требовала правки (r1 сняла её как не блокирующую); правки текста в этом разделе нет. Инвариант не нарушен: все пять сценариев по-прежнему однозначно читаются как относящиеся к Home admin, персона называется в комментарии S2-analysis, а не в тексте сценариев	Наследуется без повторной проверки — см. раздел ниже
L2 (Low, снята r1 без цикла, с рекомендацией добавить явные строки)	Автор выполнил рекомендацию сверх минимально требуемого: добавлены явные строки «i18n: новых пользовательских строк и ключей локализации нет» и «Touch/keyboard/mobile UX: неприменимо, интерактивная поверхность не меняется»	Тело issue, раздел «Принятые предположения», последние две строки

Унаследовано из r1

Принято без повторной проверки в этом раунде — дельта их не задевает:

• Все двенадцать пунктов «текущего поведения» (построчная проверка кода: import_export.py, websocket_api.py, validation.py, http_api.py, virtual_lights.py, diagnostics.py, trails.py, quality_scale.yaml) — документ r1, раздел «Как проверялось», таблица; материал — docs/reviews/SPEC-REVIEW-625-r1.md, тело issue на дереве b40b99307d3bc80bd28ac73ffa0e097c6451a3f6 (блоб тела c453dfbbd8e968467dd6567e6327005b625432bec7f0be055fdc6ac85700c910).
• AC1, AC2, AC4–AC13 — однозначны, доказательство названо, изменение к ним не относится (изменённый раздел 3 задевает только формулировку и обоснование AC3). Документ r1, раздел «Что проверено и корректно».
• AC4 (ws_layout_update): условие «удалённым считается marker id только когда для него нет активной записи» разобрано и признано корректным (не задевает соседнюю orphan_virtual-ветку). Документ r1.
• Track = full — обоснование не изменилось и не пересматривалось.
• «Не входит» (кроме прочитанного заново пункта про авто-удаление, который проверен в этом раунде как согласованный с новым правилом) — не изменилось.
• «Данные и совместимость»: отсутствие новых persisted-полей/version bump — документ r1 подтвердил это независимо от M1; в этом раунде дополнительно проверено, что новая формулировка данного раздела («несвязанное сохранение разрешено, пока группа структурно сохранена») не вводит скрытого нового поля — не вводит.
• Release artifacts (User-Visible: yes, оба changelog, issue не закрывается вручную) — не менялись, не перепроверялись.
• Механизм AC3 без version bump реализуем технически (прецедент _space_geometry_invariants) — вывод r1 не оспаривается.

Находки

Блокирующих находок нет.

Low (необязательное наблюдение, не блокирует, не требует цикла). Раздел «Инвариант markers[].id...» формулирует путь выхода из состояния конфликта только общим правилом («изменение допустимо, если оставляет ≤1 активный»), но не называет прямо, что для администратора это означает: удалить или пометить удалённым лишний дублирующий маркер через уже существующий редактор плана — это первое действие, которое он, скорее всего, попробует, и оно сработает. Текст сейчас не гарантирует этого читателю явно, хотя механика следует из общего правила однозначно и не оставляет двух прочтений. Это вопрос удобства чтения ТЗ будущим автором кода/тестов, а не открытая продуктовая развилка (в отличие от M1, здесь нет неоднозначности исхода). Снимаю без цикла; можно поправить одной фразой при следующей правке текста, если она случится по другой причине.

Что проверено и корректно (относится к дельте этого раунда)

• Новое правило раздела 3 внутренне согласовано с «Не входит»: запрет автоматического удаления/слияния не противоречит тому, что явную правку администратора (не автоматику), оставляющую ≤1 активный, система принимает — это не «слияние без потери», а обычная сохраняемая правка.
• Новое правило покрывает путь kind=full (import/restore), который r1 явно оставил неразобранным как «не требуется для вердикта, но открыт»: раздел теперь прямо требует «доказуемую previous-группу» для полного восстановления и отклоняет и восстановление нового конфликта, и импорт, создающий конфликт с целевым документом — гэп из «Чего не проверял» r1 закрыт текстом, а не оставлен молча.
• «Preview/revalidate и apply используют одно правило» — предупреждает класс дефекта «подтверждённый preview разошёлся с apply»; это самостоятельно корректное усиление, не запрошенное явно в M1, но релевантное тому же риску.
• Ошибка duplicate active marker id без пользовательского id согласована с «Наблюдаемость и ошибки»: «без вывода персональных marker-данных» — раздел не противоречит себе после правки.
• AC3 в новой редакции покрывает ровно четыре сценария (новый дубликат, несвязанная неизменная группа, изменение оставляющее два active, разные id
  • tombstone+один active) плюс отдельный тест permissive-read против строгой full-import границы — тестируемо и однозначно, конкретных открытых развилок не оставляет.
• Определение «структурно неизменённой группы» в «Принятых предположениях» (перестановка массива без изменения записей — не новый конфликт) — это ровно тот технический вопрос, который §7.1 оставляет на усмотрение агентов («где хранится состояние... механика миграции»), правильно оформлен как предположение, а не как скрытая догадка.

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

• Не перепроверял AC1, AC2, AC5–AC13 и построчные факты «текущего поведения» — дельта их не задевает (см. «Унаследовано из r1»).
• Не запускал typecheck/test/build — на этой стадии нет диапазона кода класса A/B; см. «Как проверялось», п.6.
• Не оценивал заново предположения о производительности export-snapshot и concurrency-лимите upload (AC2, AC6) — не касается дельты, будет предметом исполняемых тестов на код-ревью.
• Не искал побайтовый diff тела issue между r1 и r2 (GitHub REST/timeline API для этого issue не отдаёт событий edited с changes.body — проверено запросом gh api .../issues/625/timeline, событий такого типа нет). Дельта восстановлена по цитатам находок r1 + описанию правки в комментарии владельца + хеш-несовпадению тела с якорем r1 (см. «Скоуп раунда» выше) — этого достаточно, чтобы показать, чем закрыта M1, но не даёт формального побайтового diff остальных разделов; для них опора — отсутствие упоминания правки в комментарии владельца и совпадение содержания с цитатами r1.

Вердикт

Зелёный. High: 0. Medium: 0 (M1 из r1 закрыт явным продуктовым решением и его текстовым воплощением, разобрано выше). Low: 1, снята записью в этом же документе, цикла не образует.

ТЗ готово к переходу в «Готово к разработке» (S5-ready): DoR (§2.5) по состоянию на этот раунд — ревью ТЗ зелёное, AC1–AC13 пронумерованы и доказательство названо, i18n и touch явно «нет», миграция/compatibility решены («новых полей и version bump нет»), release-артефакты названы, открытых продуктовых вопросов не осталось, откат — один revert терминального коммита.

---

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

• Материал: тело issue #625, раздел ## ТЗ, состояние на момент этого ревью (2026-09-23); предыдущий раунд — docs/reviews/SPEC-REVIEW-625-r1.md, вердикт жёлтый, материал на дереве b40b99307d3bc80bd28ac73ffa0e097c6451a3f6 / блоб тела c453dfbbd8e968467dd6567e6327005b625432bec7f0be055fdc6ac85700c910.
• Текущее тело не совпадает ни с одной проверенной нормализацией этого блоба (см. «Скоуп раунда» — три варианта хеша приведены) — редактирование подтверждено технически, а не только по заявлению комментария.
• Код продукта не менялся (стадия S4-spec-review), диапазон коммитов для этой стадии отсутствует.

---

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

• Ветка: issue/625-backend-io-invariants, коммит d607198e2dbc — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: f73a83d6ff61d567c75d9fbd9a0c1d4487eb6928

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

• Тело issue: f9d2f583a9c977bd535f6f14de24ad8c8757914db26218bea46875ad89fbaa67
• Вердикт конвейера: green · High 0