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, дельты не существует). Проверялось:
- Соответствие обязательным разделам §7.1 PROCESS.md.
- Однозначность и проверяемость AC1…AC9, наличие способа доказательства у каждого.
- Соответствие принятому владельцем решению по Q1 (комментарий владельца в issue) и дополнению про диагностическую видимость.
- Отсутствие «догадки, выданной за решение»: каждое фактическое утверждение о текущем поведении кода/документации сверено с реальным состоянием репозитория (
origin/dev, не веткой задачи — код там не менялся). - Соответствие
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-наблюдения сняты с записью, к возврату автору не ведут.