Files
houseplan-card/docs/reviews/SPEC-REVIEW-43-r2.md
2026-09-02 00:05:36 +00:00

15 KiB
Raw Permalink Blame History

SPEC-REVIEW-43-r2

  • Issue: https://github.com/Matysh/houseplan-card/issues/43
  • Ревьюер: Claude (роль «Ревьюер ТЗ», PROCESS.md §2.4)
  • Материал: docs/specs/043-private-support-report.md на коммите e25aa302 (ветка issue/43-help-feedback), дельта против r1 (docs diff 00b68450..e25aa302 -- docs/specs/043-private-support-report.md).
  • Заход: r2 · блокирующих циклов израсходовано 1 из 4.
  • Трек: полный (не изменился с r1).
  • Формат разбора: по дельте (PROCESS.md §2.9, issue #214) — обоснование ниже, в «Почему делаю по дельте, а не заново».

Предыдущий раунд

  • Вердикт r1: жёлтый · High 0 · Medium 1 (в скоупе) · Low 1 (снят без цикла).
  • Комментарий с вердиктом: issue #43, 2026-09-01T20:40:11Z.
  • Документ r1: docs/reviews/SPEC-REVIEW-43-r1.md, зафиксирован коммитом 4559b5cd.
  • SHA, на котором получен вердикт r1: 755fa4cf (упомянут в тексте вердикта и в шапке SPEC-REVIEW-43-r1.md). Этот SHA не резолвится в текущем дереве — автор пишет в хендоффе r2 «Ветка перебазирована на актуальный dev», и git rebase переписал хеши; коммит с тем же содержимым сегодня называется 00b68450 (git diff 00b68450..HEAD даёт содержательно тот самый и единственный дифф, который описывает хендофф r2).

Почему делаю по дельте, а не заново

Формально это ребейз на ушедший вперёд dev — сценарий, для которого инструкция требует полного разбора, если ребейз действительно сделал это «другим кодом» (§7.2). Проверил, что это не тот случай:

  • git merge-base HEAD origin/dev = 4fa670e8; коммиты dev между старой и новой базой (823fc316..4fa670e8: пин растеризации скриншотов, измерение дрейфа кадра, гейт стабильности) не трогают ни AGENTS.md, ни PROCESS.md, ни scripts/process-gate.mjs, ни docs/TOUCH-SUPPORT.md — git diff --stat по этим путям в диапазоне пуст. Это ровно те четыре документа, на которые опирались обе находки r1.
  • git diff 00b68450..HEAD --stat показывает три файла: добавленный документ ревью docs/reviews/SPEC-REVIEW-43-r1.md (артефакт публикации, не правка автора), docs/specs/README.md (1 строка, не содержательная), и docs/specs/043-private-support-report.md — 24 изменённые строки, все три правки локализованы в §9.1, §11 и §18 и построчно соответствуют двум находкам r1.
  • Ни договор поведения (§4/§6–8), ни AC1–AC17 (§13), ни privacy allowlist (§7.2/7.3), ни UX-контракт (§6) дельта не задевает.

Поэтому разбор r2 ограничен: (а) доказать закрытие двух находок r1 построчно, (б) проверить, что дельта не сломала ничего в соседних разделах, (в) унаследовать всё остальное из r1 без повторной проверки.

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

Находка r1 Чем закрыта Где это видно
Medium. support-relay/** как новый top-level каталог не попадает ни в класс A, ни в B scripts/process-gate.mjs::classify() → коммит, трогающий только этот каталог, проходит гейт без трейлера Issue: и без проверки статуса issue. §9.1 переписан: relay переехал под scripts/support-relay/**, что совпадает с CLASS_B (/^scripts\// в scripts/process-gate.mjs:59) без каких-либо правок AGENTS.md/PROCESS.md/process-gate.mjs. §18 добавляет обязательство держать runtime/manifest/tests/README relay целиком в этом поддереве и завести process-gate regression fixture, доказывающую, что relay-only коммит классифицируется как B. docs/specs/043-private-support-report.md:404-412 (§9.1), :621-623 (§18). Проверено запуском node scripts/process-gate.mjs на HEAD — «гейт пройден, предупреждений 0», путь scripts/support-relay/ реально попадает в CLASS_B по regex scripts/process-gate.mjs:59.
Low. §11 по содержанию реализует Touch editor: supported, но не содержит канонической фразы, которую требует docs/TOUCH-SUPPORT.md. В §11 первой строкой добавлено **Touch editor: supported.** с уточнением, что диалог доступен из View и всех трёх редакторов с одинаковым touch-контрактом, а kiosk по-прежнему его скрывает. docs/specs/043-private-support-report.md:465-467. Фраза дословно совпадает с канонической формой из docs/TOUCH-SUPPORT.md:165 (Touch editor: supported).

Обе находки закрыты текстом, а не заявлением: правка видна в дифф-контексте выше, а не только в комментарии автора.

Проверка дельты (не унаследовано — перепроверено заново)

  1. Согласованность пути relay по всему документу. grep -n "support-relay" docs/specs/043-private-support-report.md даёт ровно два совпадения (§9.1 заголовок и §18 release-артефакты) — оба уже указывают на scripts/support-relay/**, старого top-level упоминания support-relay/ без префикса нигде не осталось. Более широкий grep -n "relay" (60+ вхождений по всему файлу) показал, что остальные упоминания — это архитектурная роль сервиса («project-controlled relay», «relay secret», «relay fixture» и т.д.), а не файловый путь, поэтому переезд каталога их не касается — переименование сделано полностью, частичной правки нет.
  2. Не нарушает ли новое место relay границу HACS-пакета. hacs.json (zip_release: true, filename: houseplan.zip) не перечисляет содержимое явно — упаковка определяется отдельным релизным шагом, не process-gate.mjs; scripts/** и раньше не попадал в custom_components/houseplan/frontend/ (единственный источник HACS-фронтенда по CLASS_D). Утверждение «excluded from the HACS artifact» в §9.1 остаётся верным после переезда, это не новый риск, привнесённый дельтой.
  3. Не конфликтует ли scripts/support-relay/** с существующей ролью scripts/. tsconfig.json ограничивает typecheck src/**/*.ts; каталог scripts/ уже состоит из .mjs/.py-скриптов вне зоны компиляции TS — новый relay-код в том же дереве не меняет эту границу и не требует правки tsconfig*.json (что и означало бы A+B, а не только B, в терминах самого §9.1).
  4. Формулировка "a product commit that also changes src/** remains A+B и follows the stricter class-A flow" (§9.1) сверена с scripts/process-gate.mjs:137 (classes: new Set(files.map(classify))) и условиями classes.has('A') || classes.has('B') для строгих проверок — описание корректно: набор классов коммита — это множество классов всех его файлов, а не единственная метка.
  5. Трейлеры коммита дельты. git show e25aa302 -s → Issue: #43, User-Visible: no — верно для docs-only правки без видимого поведения.
  6. Гейты на этой дельте. Дифф 00b68450..e25aa302 строго класса C (только docs/specs/043-private-support-report.md). Прогнал:
    • node scripts/process-gate.mjs → «диапазон origin/dev..HEAD, коммитов 3; гейт пройден, предупреждений 0» (офлайн, без --issues);
    • Validate CI на e25aa302 зелёный (см. ссылку в хендоффе автора и в инструкции ревью) — покрывает tsc --noEmit/npm test/npm run build, предмета для которых у docs-only дельты и так нет. check-docs.mjs, browser-смоки, model-invariants, pytest tests_backend, golden — не прогонял: src/**, custom_components/** и геометрия/layout дельтой не тронуты, у этих гейтов нет предмета проверки на чисто документном коммите (то же обоснование, что в r1).

Унаследовано из r1 (без повторной проверки)

Документ: docs/reviews/SPEC-REVIEW-43-r1.md, зафиксирован на 4559b5cd, разбирал ТЗ на 00b68450 (тогда назывался 755fa4cf до ребейза). Дельта r1→r2 не задевает ни один из пунктов ниже, поэтому принимаю выводы r1 без повторной проверки:

  • Обязательные разделы §7.1 (сценарий, до/после, проблема, скоуп, контракт поведения, UX, модель данных/миграция, i18n, AC1–AC17, план автотестов, риски, откат, release-артефакты) — все на месте и однозначны.
  • Продуктовые вопросы Q1–Q5 корректно отделены от технических и закрыты владельцем в issue дословно, без додумывания.
  • Пять утверждений §3 о текущем состоянии кода проверены построчно по src/houseplan-card.ts, src/houseplan-editor-runtime.ts, custom_components/houseplan/{websocket_api,import_export,repairs}.py, src/i18n/*.json — расхождений не найдено.
  • Privacy allowlist/exclusion §7.2/7.3 внутренне непротиворечивы; точная геометрия включена осознанно как продуктовое решение (Q2), а не побочный эффект.
  • «Одно число — один источник» для preview/download/submit спроектировано контрактом (кешированный TTL-токен + mutation-gate §6.4/7.1/8.1/AC5/AC10 + §14.4), а не оставлено на волю реализации.
  • Технические допущения промаркированы в §20 как «assumed, change freely».
  • i18n RU/EN/DE/FR соответствует уже существующим локалям, список языков не расширяется произвольно.
  • Cross-links issue↔ТЗ↔docs/specs/README.md на месте.
  • DoR-зависимости §17 (relay deployment, mailbox, staging) корректно вынесены как внешние операционные факты с явным blocked-путём, а не спрятаны как допущения.

Чего не проверял (r2)

  • Полные npm test/npm run build/browser-смоки/model-invariants/pytest tests_backend — не прогонял отдельно: Validate CI зелёный на этом самом SHA (e25aa302), а дельта и так docs-only без предмета для этих гейтов.
  • Реальная развёртываемость relay, секреты, содержимое ещё не написанного scripts/support-relay/** — кода ещё нет, это заявленная DoR-зависимость §17/§18, появится на этапе реализации и code-review.
  • Golden/скриншоты, performance-профили — не относятся к докс-only дельте.
  • Process-gate regression fixture, которую §18 требует для доказательства классификации relay-only коммита как B — она появится вместе с кодом; на этапе ТЗ можно проверить только текст требования (проверено, есть).

Вывод

Обе находки r1 закрыты предметно, построчно и без побочных повреждений соседних разделов: relay-код явно и полностью перенесён в уже классифицированный класс-B каталог scripts/support-relay/** (подтверждено чтением scripts/process-gate.mjs и живым прогоном гейта), а §11 получил каноническую фразу Touch editor: supported, дословно совпадающую с требованием docs/TOUCH-SUPPORT.md. Ребейз на актуальный dev не изменил существа рассмотрения: коммиты dev, вошедшие в диапазон, не трогают ни один документ, на который опирались находки r1. Новых находок в дельте нет. Rec: зелёный, готово к переводу в S5-ready.