Files
houseplan-card/docs/reviews/SPEC-REVIEW-296-r1.md
2026-08-24 19:10:27 +00:00

13 KiB

SPEC-REVIEW-296-r1

  • Issue: https://github.com/Matysh/houseplan-card/issues/296
  • Этап: ТЗ на ревью (S4-spec-review), заход r1 (первый), блокирующих циклов израсходовано 0/4 (лимит обычного трека — 4)
  • Ревьюер: Claude, независимая сессия, без устных пояснений автора
  • Артефакт ТЗ: docs/specs/296-optimize-hidden-obstacles.md на ветке issue/296-optimize-hidden-obstacles, коммит d5e0e4f1
  • Диапазон: git diff origin/dev...HEAD — только docs/specs/296-optimize-hidden-obstacles.md (новый файл) и одна строка в docs/specs/README.md. Класс C (документация), кода нет — это ожидаемо для этапа spec.

Скоуп ревью

Полный разбор (это r1, дельты не существует). Проверялось:

  1. Соответствие обязательным разделам §7.1 PROCESS.md.
  2. Однозначность и проверяемость AC1…AC9, наличие способа доказательства у каждого.
  3. Соответствие принятому владельцем решению по Q1 (комментарий владельца в issue) и дополнению про диагностическую видимость.
  4. Отсутствие «догадки, выданной за решение»: каждое фактическое утверждение о текущем поведении кода/документации сверено с реальным состоянием репозитория (origin/dev, не веткой задачи — код там не менялся).
  5. Соответствие docs/SCOPE.md (job J6), docs/USER-GUIDE.ru.md (терминология), docs/CANVAS.md, docs/WALL-THICKNESS.md, docs/RESIZE.md.

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

Гейты кода (typecheck/test/build) не запускались — на этом этапе класса A/B изменений нет, диф чисто документационный (класс C), запускать их не по чему. Вместо этого каждое техническое утверждение ТЗ сверено чтением исходников на HEAD (= origin/dev по коду):

Утверждение ТЗ Где проверено Результат
reconcileCoincidentPartitions/sameSegment требует полного совпадения одного интервала src/coincident-partitions.ts:1-100 подтверждено
alignAllToGrid фильтрует drafts по points?.length >= 2 src/align-grid.ts:256 подтверждено
_safe_optimize_partition_rehost требует покрытия ровно одним edge комнаты custom_components/houseplan/validation.py:148-246 подтверждено; расширение до piecewise технически естественно (та же структура доказательства, применённая к атомарным участкам)
MAX_PARTITIONS = 2000 src/houseplan-card.ts:625 подтверждено
Физический радиус узла 5 см, порядок отрисовки overlay #137 docs/CANVAS.md:660-680 подтверждено, диагностический слой явно описан как отдельный от #137
max(roomCm, partitionCm) — уже существующее правило, не новое docs/WALL-THICKNESS.md:412 подтверждено
Фикстуры real-plan-second-floor.json/real-plan-first-floor.json существуют test/fixtures/*.json существуют; partition-mt2on9ou-0 уже в фикстуре, partition-room-mt7ijuyq-0 и draft-mt7igts5 — ещё нет, фикстуру предстоит дополнить (это входит в план тестов §14.3, явных противоречий нет)
Существующие смоки smoke_optimize_coincident_partition.mjs, smoke_resize_* ls demo/smoke_*.mjs существуют, естественные кандидаты для §14.6
Терминология «осевые линии/узлы» уже знакома пользователю по #137 docs/USER-GUIDE.ru.md:373-390 подтверждено, новый раздел документации может опереться на неё
Ссылка issue ↔ ТЗ в обе стороны issue #296, docs/specs/README.md diff подтверждено

Продуктовая рамка

Задача закрывает J6 (docs/SCOPE.md): администратор поддерживает план в редактируемом состоянии. Явная команда «Оптимизировать» — единственная поверхность, диагностический слой ограничен редактором Плана и не протекает во View/kiosk (проверено по non-scope §5 и §9.2 — совпадает с принципом «View mode is the product», редакторские фичи не должны туда течь).

Открытый продуктовый вопрос был (Q1 — что вправе удалять Optimize из room_drafts), задан владельцу батчем с default-вариантом по формату §7.2, владелец принял default и добавил требование к диагностической видимости. ТЗ отражает оба решения текстуально (§7, §9) без residual несоответствий. Требование «не бывает сложной задачи без единого открытого вопроса» выполнено этой перепиской.

Находки

Ничего блокирующего (High/Medium) не найдено. Три Low-наблюдения, все сняты с записью (не требуют возврата автору):

L1 (снято). §2 «Что человек увидит» — несколько предложений с внутренними терминами (partition, room_draft, «канонические стены комнат»), а не одна фраза без терминов реализации, как в букве §7.1 PROCESS.md. Не поднимаю до Medium: терминология совпадает с той, которой сам владелец пользуется в теле issue (он написал partition/room_draft/duplicate-physical-wall от руки), то есть словарь общий с владельцем, а не изобретён автором ТЗ в одностороннем порядке; содержательно раздел ясен.

L2 (снято). §6.3/§12.3: «первый в каноническом порядке сохраняет исходный id» — «канонический порядок/направление» не определены явно даже в блоке предположений. Это техническая деталь («чего пользователь не наблюдает» — id внутренней записи), которую §7.1 PROCESS.md отдаёт автору/ревьюеру на усмотрение, а не владельцу; формально спор решается кодом на код-ревью. Оставляю как наблюдение для код-ревью: убедиться, что реализация фиксирует правило детерминированно (например, по возрастанию проекции вдоль исходного a→b) и AC2 (round-trip прямого/обратного направления) действительно проверяет это, а не подстраивается под то, что получилось.

L3 (снято). AC9/§14 п.7 ссылаются на бенчмарк coincident-partitions, которого сейчас не существует (demo/performance/ содержит только budgets-large-house-*.json, нет budgets-coincident-partitions.json). ТЗ не проговаривает явно, что профиль создаётся с нуля по образцу существующих budgets-large-house-*.json, либо что эта нагрузка меряется в рамках large-house-plan-snap. Технический выбор реализации, не продуктовый вопрос — не блокирует, но стоит явно решить на код-ревью, чтобы «утверждённые budgets» в AC9 не повисли без числового порога.

Проверка требований §7.1 (обязательные разделы)

Все присутствуют: сценарий (§1) · что увидит человек (§2, см. L1) · проблема/подтверждённая причина (§3) · scope/non-scope (§4/§5) · контракт поведения (§6–§10) · данные/i18n/accessibility (§11) · принятые предположения (§12) · AC1…AC9 с доказательством (§13) · план автотестов (§14) · перф/touch (§15) · release-артефакты (§16) · риски и откат (§17). Ничего не пропущено.

Каждый AC называет способ доказательства (unit/backend/smoke/golden/audit) и достаточно точен, чтобы автор код-ревью мог проверить «тест умеет падать» — большинство AC привязаны к конкретным числам из приложенного экспорта (30/30/30, partitionsReconciled >= 2, removedDrafts == 1) или к конкретным именованным функциям/файлам, а не к общим формулировкам.

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

  • Технические утверждения о текущем состоянии кода (проблема, §3) — все подтверждены чтением src/coincident-partitions.ts, src/align-grid.ts, custom_components/houseplan/validation.py.
  • Контракт поведения (§6–§10) не противоречит существующим канонам WALL-THICKNESS.md (max(roomCm, partitionCm) — уже принятое правило, не новое), CANVAS.md (диагностический слой явно отделён от snap-overlay #137, не меняет его приоритеты) и RESIZE.md (словарь причин duplicate-physical-wall/partial-shared/unequal-shared использован верно).
  • Non-scope корректно исключает миграцию схемы, автозапуск при загрузке, изменение snap/hit-testing #137, показ слоя вне Plan-редактора — совпадает с положением View/kiosk из docs/SCOPE.md.
  • i18n/accessibility (§11): новых ключей и ARIA-семантики не вводится, только правка существующей строки счётчика в обоих словарях — соответствует политике SCOPE.md («no ARIA labelling of the plan»).
  • Владельческие решения по Q1 и дополнению отражены в ТЗ без искажения и без остаточных противоречий с #173/#294 (двухточечная незамкнутая цепочка — законный объект, не подлежит удалению по числу точек).
  • Ссылка issue ↔ ТЗ в обе стороны на месте (docs/specs/README.md дополнен).

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

  • Реализацию — кода ещё нет, это этап ТЗ, а не код-ревью.
  • Полнотекстовое сравнение всех связанных issue (#137, #173, #276, #277, #280, #281, #282, #292, #294) построчно — проверены точечно те фрагменты, на которые ТЗ прямо ссылается фактическими утверждениями.
  • Фактическую производительность нового piecewise-прохода (§15) — бюджеты и порог регрессии не существуют, их скажет реализация на код-ревью (см. L3).
  • Golden/скриншоты — не применимо на этапе spec, это release-артефакт следующего этапа.

Вердикт

Зелёный. ТЗ полно по §7.1, продуктовый вопрос закрыт владельцем без остатка, технические утверждения проверены и подтверждены, AC проверяемы и привязаны к конкретным числам/файлам. Три Low-наблюдения сняты с записью, к возврату автору не ведут.