Files
2026-09-26 09:04:16 +00:00

20 KiB
Raw Permalink Blame History

SPEC-REVIEW-663-r2

  • Issue: https://github.com/Matysh/houseplan-card/issues/663
  • Этап: S4-spec-review (ревью ТЗ, PROCESS.md §2.4)
  • Трек: полный (унаследовано из r1: «новая UX-сущность, новая persisted-модель, несколько поверхностей, межэтажные ссылки, touch и визуально-производительные риски»; r2 добавляет ещё одну persisted-геометрию (винтовая лестница) — трек остаётся полным a fortiori).
  • Материал: тело issue #663 (раздел «## ТЗ r2», sha256 тела 8c3631b53e07f2aebaef794681bb998a820afe35237f9c759db87c482e4aef58) + 3 новых комментария после SPEC-REVIEW-663-r1: (5) «ТЗ r2: правки после жёлтого ревью и новый винтовой тип» (High-1 fix + новый Q9), (6) решение владельца Q9 и готовность r2, (7) вердикт конвейера r1 (жёлтый, для полноты истории).
  • Заход: r2 · блокирующих циклов израсходовано 1 из 4
  • Роль: ревьюер ТЗ (не автор)

Скоуп ревью

Дельта r2 не локальна и разбирается полностью (PROCESS.md §2.10, «новая подсистема, объём сопоставим с задачей»): помимо точечного исправления High-1 из r1 (выбор единого continuous/мебельного контракта transform/magnet/Optimize), владелец добавил второй тип лестницы — винтовую, с круглым габаритом, задаваемым радиусом, tangent-магнитом к стене, дуговой стрелкой, радиальной разметкой ступеней с шагом 30 см по линии на 2/3 радиуса, отдельной discriminated-веткой модели данных и полной golden/unit/backend-матрицей. Это самостоятельная геометрическая подсистема, а не косметическая правка текста — разобрана целиком, а не только в части, закрывающей High-1.

SCOPE-проверка (docs/SCOPE.md): второй тип не меняет вывод r1. Винтовая лестница остаётся навигационной/визуальной сущностью (J1/J4/J6), явно и многократно отказывается от строительного расчёта (число ступеней по норме, уклон, площадки) и не сползает в «General CAD» либо «3D-модель» из списка «никогда не строить» — раздел 5 прямо перечисляет объёмный марш и составные лестницы как не в скоупе. Новой exception-строки в SCOPE.md эта задача не требует: второй тип — это второй визуальный вариант той же уже одобренной сущности, а не новая категория.

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

  1. Прочитаны docs/SCOPE.md, docs/process/REVIEWER.md (конспект + разделы PROCESS.md §2.4, §2.10, §4, §7.1, §7.2 по ссылкам), AGENTS.md.
  2. Прочитан документ и материал r1 (docs/reviews/SPEC-REVIEW-663-r1.md) целиком, включая блок «Материал раунда» — SHA тела a304102f2b7c…, вердикт жёлтый, High-1 + 2 Low (сняты без возврата).
  3. Прочитано тело issue #663 целиком (gh issue view 663 --json body,comments, ## ТЗ r2) и все 7 комментариев, включая новые (5)–(7).
  4. Объявлена дельта: сравнены разделы 6.2 и 8 ТЗ r1 (цитаты в SPEC-REVIEW-663-r1.md) с текущими §6.2/§8 ТЗ r2 построчно — правки High-1 найдены и подтверждены (см. «Закрытие раунда r1» ниже).
  5. Контракт continuous/lattice сверен не с одним лишь текстом канона, а с исполняемым кодом: src/coordinate-canonicalization.ts — DECOR_BOX_KINDS = ['rect', 'ellipse', 'furniture', 'image'] (import из src/editors/decor/types.ts:8), LATTICE_NOISE_STEPS = 1e-4 (coordinate-canonicalization.ts:18,65-69), latticeFields(decor, ['x','y','w','h']) и отдельно scalarFields(decor, ['angle']) для decor-box-родов (coordinate-canonicalization.ts:352-357). Это подтверждает канон docs/CANVAS.md:588-591 (грид-проход Optimize исключает furniture/image, их позиция/размер/угол — continuous authored values) и различает его от «persisted-coordinate canonicalisation» (docs/CANVAS.md:47-64, LATTICE_NOISE_STEPS) — тихой очистки IEEE-754 шума в пределах 1e-4 шага грида, которая формально применяется и к furniture-полям x/y/w/h, но не производит видимого сдвига (сработает только когда значение и так уже практически на узле). Формулировка нового §8 ТЗ («storage normalization может ограничивать незначащий численный шум, но не привязывает значения к grid/lattice») этому различию соответствует буквально.
  6. Разобраны новые §6.1/§6.2/§6.4/§7/§8/AC1-13/§11 построчно на предмет внутренней согласованности прямого и винтового типов, наличия открытых продуктовых вопросов (Q9 закрыт владельцем в комментарии 6) и однозначности способа доказательства каждого AC.
  7. git branch -a / git log --all --oneline | grep 663 — по-прежнему нет ветки/коммитов с кодом для #663; git diff origin/dev...HEAD пуст. Этап spec, продуктового кода нет — гейты (tsc, test, build, смоки, golden, инварианты) неприменимы, штатное состояние, не находка.
  8. Проверено docs/reviews/INDEX.md на предмет соседних прецедентов той же подсистемы (furniture wall magnet / lattice-канонизация): #223, #282, #313, #445, #447 — подтверждают, что спор «continuous vs grid-bound» для новых decor-подобных сущностей систематически разбирается на этом ревью и не является чем-то специфичным для #663.

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

Находка r1 Чем закрыта Где это видно
High-1 — §6.2 (wall snap, continuous) и §8 («тем же lattice-контрактом, что decor» + «Optimize не изменяет лестницы») взаимно исключают друг друга Оба раздела переписаны на единый continuous/мебельный контракт: §6.2 — «Обе лестницы используют continuous-контракт мебели, а не grid-bound контракт обычного декора»; «Результат magnet имеет приоритет над сеткой и после save/load остаётся в точной continuous-позиции. Послесохранительная численная нормализация не квантует позицию, размеры или угол лестницы на lattice»; «Optimize полностью исключает transform лестниц». §8 — «transform лестниц хранится и round-trip-ится как continuous authored values по контракту мебели; storage normalization может ограничивать незначащий численный шум, но не привязывает значения к grid/lattice и не разрушает точное примыкание». Формулировка соответствует реальному коду (см. «Как проверялось» п.5) Тело issue #663, §6.2 (абзацы 1 и 4), §8 (пункт «transform лестниц…»); AC2/AC9/AC11 дополнены явными regression-требованиями save/load/Optimize
Low-1 (снят без возврата) — AC7 называет 4 состояния, §6.3 — 3 Четвёртое слово убрано из терминологии: «Термин «недоступная цель» в UX означает одно из этих трёх состояний, а не отдельное четвёртое состояние» Тело issue #663, §6.3 последний абзац; AC7 переформулирован на «пустая, self и удалённая цель» — ровно три
Low-2 (снят без возврата) — AC13 не называет конкретный бюджет AC13 явно ссылается на существующий смок и запрещает ослаблять его порог: «проверяется существующим performance-смоком большого плана и его действующим бюджетом на момент реализации без ослабления порога» Тело issue #663, AC13 и §11 «Performance» — в репозитории такой смок реально существует (test/performance-budget.test.mjs, test/performance-baseline.test.mjs, test/performance-contract.test.mjs), ссылка не на фиктивный артефакт

Все три пункта r1 закрыты правкой текста, а не заявлением автора «сделано» — цитаты выше воспроизводимы построчно в текущем теле issue.

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

  • Полнота обязательных разделов §7.1 (1–15, теперь до 16) и их порядок — подтверждено в r1, структура разделов 1–15 не тронута правкой r2.
  • SCOPE-соответствие J1/J4/J6 для прямого типа, синтаксис Q1–Q8 → контракт — сверено в r1 построчно, r2 их не меняет (§16 ТЗ подтверждает то же самое: «Унаследовано из r1 без повторного пересмотра»).
  • Контракт навигации §6.3 (fixed-floor no-op, gesture guards, remembered view) — не изменён деltoй r2 (текст §6.3 идентичен r1 кроме удаления слова «недоступная» как четвёртого состояния, см. таблицу выше).
  • Контракт площади §6.4 для прямого типа, модель отката §13, i18n-принцип §9 «не конкатенировать» — не изменены по существу, только расширены на второй тип теми же словами.
  • Документ и материал: docs/reviews/SPEC-REVIEW-663-r1.md, тело issue r1 sha256 a304102f2b7c3f5aae7814eabe642e7aa679d5243df7f25ce579bc490959262c.

Находки

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

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

  • Continuous-контракт устранил противоречие r1 и подтверждён исполняемым кодом, а не только текстом канона (см. «Как проверялось» п.5) — при реализации «по контракту мебели» лестница действительно не попадёт под Optimize-грид-проход и переживёт минимальную post-save численную очистку (≤1e-4 шага грида) без визуального сдвига магнита.
  • Второй тип (spiral) внутренне согласован: круглый габарит (radius, диаметр 2×radius), единый радиальный resize без независимого ширина/высота, tangent-magnet к стене «без принудительного изменения угла» (логично — angle для spiral означает не ориентацию габарита, а нижний начальный луч разметки, а не точку касания), stair-snap описан для всех трёх комбинаций габаритов (прямоугольник–прямоугольник/круг, круг–круг).
  • Разметка ступеней spiral (§6.1, Q9) однозначна: один полный оборот, нижний луч = поворот, направление по/против часовой стрелки, шаг 30 см по линии на 2/3 радиуса, остаток — перед верхним лучом, не растягивается; AC3 численно завершает контракт для обоих типов раздельно.
  • Type conversion (§7: straight→spiral — радиус = половина большей стороны; spiral→straight — длина и ширина равны диаметру) детерминирован, не оставляет зазора для «а как насчёт …», участвует в undo/redo.
  • Модель данных (§8) — discriminated union по kind с общими полями (id/позиция/angle/двухзначное направление/target_space_id) и типоспецифичными размерами; backend-валидация (bounds, kind, лимит, уникальные id) описана для обоих типов без особого случая, оставленного открытым.
  • AC1–AC13 переформулированы под оба типа без потери однозначности; способ доказательства указан для каждого (unit/smoke/golden/backend/ geometry parity/performance+ревью кода) — требование DoR §2.5 выполнено.
  • План автотестов (§11) обновлён: TS unit явно называет «rectangle/circle intersections» и «snap candidates для всех пар габаритов», golden — «матрица AC12» (оба типа/оба направления/оба режима 2D-2.5D), mutation witnesses явно перечисляют «шаг ступеней, зависящий от zoom» — новый в r2, закрывает единственный визуально рискованный кейс §6.1 «на очень малом zoom рендер может скрыть часть внутренних линий».
  • Release-артефакты (§14) прямо называют «light/dark 2D/2.5D screenshots обоих типов» и пользовательскую заметку про оба типа — не забыт второй тип ни в одном из обязательных пунктов.
  • Открытый вопрос Q9 (единственный, добавленный r2) закрыт владельцем в комментарии 6 до отправки на ревью, дословно перенесён в §6.1/§7/AC3 — открытых продуктовых вопросов не осталось.
  • Q1–Q8 (r1) остаются закрытыми, второй тип не переоткрывает ни один из них (проверено построчно в r1, не изменено r2 — см. «Унаследовано»).

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

  • Гейты (tsc --noEmit, npm test, npm run build, check-docs.mjs, смоки, golden:verify, инварианты) — не прогонял: этап spec, кода для #663 нет, git diff origin/dev...HEAD пуст. Предмет код-ревью после реализации.
  • Не пересчитывал буквально геометрию 2/3-радиуса для конкретных числовых примеров радиуса/шага — контракт §6.1 полон и не создаёт двух прочтений вне зависимости от конкретных чисел (совпадение начального и конечного луча после ровно одного оборота — ожидаемое, физически корректное поведение винтовой лестницы, а не дефект спецификации).
  • Не проверял конкретные i18n-строки RU/EN построчно — раздел 9 перечисляет смысловые группы, конкретные строки — реализация (то же ограничение, что и в r1).
  • Не оспаривал сами решения владельца Q9 (единственный полный оборот, измерение шага на 2/3 радиуса) — продуктовый выбор владельца, не предмет спора ревьюера.
  • Не проверял осуществимость будущего объёмного 2.5D-этапа для spiral сверх текста ТЗ — явно вне скоупа этой версии (§5, §15).

Вердикт

Единственная блокирующая находка r1 (High-1, противоречие continuous/lattice контрактов) закрыта явной и однозначной правкой текста, подтверждённой не только каноном (docs/CANVAS.md §9.4–9.5), но и исполняемым кодом (src/coordinate-canonicalization.ts) — реализация «по контракту мебели» физически не может унаследовать баг «съезжает после сохранения», который ловил High-1. Оба Low r1 сняты правкой (не просто заявлением). Существенное расширение скоупа — второй тип лестницы (винтовая) — специфицировано так же строго, что и первый: геометрия, magnet, площадь, 2.5D, модель данных, i18n, AC и план автотестов покрывают оба типа без зазора, единственный новый продуктовый вопрос (Q9) закрыт владельцем до отправки на ревью. Новых блокирующих или в-скоупных находок нет.

Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0 → в задаче



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

  • Ветка: dev, коммит dc83a875f4d4 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 4c8e25550d7cae5d16a2843c2828929e17169296
    git log --all --format='%H %T' | grep 4c8e25550d7c
    
  • Тело issue: f3f0ee8408eb76ccec4f3ec50a9048ff1e29b2967259f0d4c4d333d8c68157e4
  • Вердикт конвейера: green · High 0