Files
houseplan-card/docs/reviews/SPEC-REVIEW-309-r2.md
2026-08-25 18:11:25 +00:00

20 KiB
Raw Permalink Blame History

SPEC-REVIEW-309-r2

Issue: https://github.com/Matysh/houseplan-card/issues/309 «Стыковочные узлы: визуальный лимит mitre и устранение паразитных парных патчей» Этап: spec (обычный трек — сложность/риск владельца 6/10 и 7/10, small невозможен) Заход: r2 · блокирующих циклов израсходовано 1 из 4 (до этого раунда) Ревьюер: Claude (сессия ревью ТЗ), артефакт: docs/specs/309-junction-visual-limit.md, коммит f0ea376a (ветка issue/309-junction-visual-limit). git diff origin/dev...HEAD --stat — 2 файла документации (спек + SPEC-REVIEW-309-r1.md), кода нет.

Скоуп

Продуктовая рамка не изменилась со времён r1 — рендер кладки в узлах стыка стен (J1/J6 SCOPE.md), см. SPEC-REVIEW-309-r1.md, раздел «Скоуп»: соответствие подтверждено там, дельта её не задевает, наследую без повторной проверки.

Предмет r2 — ревизия 2 ТЗ (коммит f0ea376a), написанная в ответ на жёлтый вердикт r1: «Взял: … добавлены Сценарий, „до/после“, Скоуп/не-скоуп, UX, Модель данных (нет миграции), i18n (нет строк), Риски (4), Release-артефакты» (комментарий владельца от 2026-08-25T18:02:38Z). Разбор по дельте (§2.10): единственная находка r1 была одна (Medium, 8 пунктов одним списком), делта — точечная правка того же файла, новую подсистему не задевает, контракт поведения не меняет → полный повторный разбор не требуется, проверяю дельту плюс всё, до чего она дотягивается.

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

  1. Нашёл вердикт предыдущего раунда и SHA, на котором он получен: комментарий IC_kwDOTOcLQM8AAAABQrvNFQ (2026-08-25T18:01:51Z) — «Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 1 → в задаче», документ docs/reviews/SPEC-REVIEW-309-r1.md, получен на коммите спека ff623cd4 (сам документ это явно называет в шапке — не находка, SHA назван).
  2. Объявляю дельту: git diff ff623cd4..f0ea376a -- docs/specs/309-junction-visual-limit.md — 36 вставленных строк, 1 изменённая (строка статуса), 0 удалённых из существующего текста. Все вставки — новые разделы §0 (Сценарий + до/после) и §3.6–§3.11 (Скоуп/не-скоуп, UX, Модель данных, i18n, Риски, Release-артефакты). Разделы §1–§3.5 (проблема, решения владельца, контракт 3.1–3.5), §4 (поверхности), §5 (AC1–7), §6 (план тестов), §7 (откат) — байт-в-байт те же, что были на ff623cd4 и уже проверены в r1.
  3. По каждому из 8 пунктов находки r1 построчно сверил новый текст с требованием (таблица «Закрытие раунда r1» ниже).
  4. git diff origin/dev...HEAD --stat — только документация (docs/specs/309-…md, docs/reviews/SPEC-REVIEW-309-r1.md), кода нет. Как и в r1, npx tsc --noEmit / npm test / npm run build на этом этапе неприменимы — не гоняю по той же причине, что и r1 (нет изменений src/**), это не пропуск гейта, а отсутствие предмета для него.
  5. Проверял НЕ на веру автору полноту новых разделов, а построчно читал код и канонический документ подсистемы для проверки: src/wall-thickness.ts — константы MITRE_LIMIT = 4 (:79), MULTI_WALL_JOIN_LIMIT = 1.25 (:83, «#249»), их использования (:1144 парный mitre, :2087 веер, :1475/:2642-2666/ :3311/:3875 — node.limit/multiWallBevelCutsAt); docs/WALL-THICKNESS.md §3 (:100-230) — механизм #249/#279 и его текущий статус; docs/TOUCH-SUPPORT.md (:1-40) — контракт touch для View/kiosk; PROCESS.md §2.5 (DoR) — полный список обязательных пунктов выхода в «Готово к разработке».

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

Находка r1 (подпункт Medium) Чем закрыта Где это видно
Сценарий отсутствует Добавлен §0 «Сценарий»: персона (владелец дома), поверхности (Plan-редактор для правки, View/kiosk/Static/hidden-Iso для просмотра — с явной ссылкой на общий структурный кэш), момент (схождение стен под острыми углами/у толстых стен/смешанных толщин) docs/specs/309-junction-visual-limit.md:7-9
«Что человек увидит до/после» отсутствует Добавлено в §0: «До: … клинья и пики длиной до 4 полутолщин …» / «После: … вершины не дальше 1.5 полутолщины …» :11-12
Скоуп и не-скоуп не выделены явным разделом Добавлен §3.6 «Скоуп и не-скоуп» :56-59 — закрыта частично, см. находку ниже
UX отсутствует Добавлен §3.7 «UX»: «никаких новых контролов… форма превью и персиста совпадает» :61-63
Модель данных и миграция не названы явно Добавлен §3.8: «Нет. Чистая рендер-геометрия…» :65-67
i18n не названо явно Добавлен §3.9: «Нет новых строк» :69-71
Риски не собраны в раздел Добавлен §3.10, 4 пронумерованных риска (дыры на фаске, регресс golden, скрытые сектора при удалении пар, перф) :73-78
Release-артефакты не выделены Добавлен §3.11: changelog RU+EN, обновление docs/WALL-THICKNESS.md §3, screenshot-fingerprint :80-82

7 из 8 подпунктов закрыты полностью и корректно. Один («Скоуп и не-скоуп») закрыт только формально — раздел появился, но не содержит именно то, что r1 просил назвать явно, — см. «Находки».

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

Без повторной проверки, по документу docs/reviews/SPEC-REVIEW-309-r1.md на SHA ff623cd4 (дельта эти разделы не касается — сверено побайтово diff'ом из шага 2):

  • Продуктовая рамка/скоуп-соответствие SCOPE.md J1/J6 («Скоуп» r1).
  • Контракт §3.1–3.5: VISUAL_MITRE_LIMIT = 1.5, парная фаска (§3.2), фильтр не-соседних пар в узлах ≥3 лучей (§3.3), правило вееров (§3.4), инварианты «без дыр»/«без фантомов»/превью=персист (§3.5) — построчно проверены на src/wall-thickness.ts:80,1082-1150,2087, src/physical-geometry.ts:242 в r1.
  • Математика порога: для равных толщин под 90° вылет h·√2 ≈ 1.41h < 1.5h — проверено в r1, прямые/тупые углы не задеты.
  • AC1–7 (§5) — однозначны, у каждого указано доказательство (юнит/golden/смок), мутанты (a)-(d) целятся в разные пункты контракта и не дублируют друг друга.
  • План автотестов (§6) и откат (§7) — полны.
  • Low-находка r1 (расхождение «16» vs «15+1» junction-сцен в docs/WALL-THICKNESS.md/ТЗ) — снята ревьюером без правки в r1, не проверяю повторно: не влияет ни на один AC.
  • Трейлеры документационных коммитов (Issue: #309, User-Visible: no) — корректны, проверено в r1 для ff623cd4; для f0ea376a та же схема (см. «Что проверено» ниже).

Находки

Medium (в скоупе задачи — правится в этом же документе)

Раздел «Скоуп и не-скоуп» (§3.6) не закрывает находку r1 полностью, и одна из двух его частей — не мелочь, а связана с реальным соседним механизмом кода. DoR (§2.5) также требует явного пункта про touch, которого до сих пор нет.

  1. #249/#279 не исключены явно, хотя r1 просил именно это. r1 дословно: «не сказано explicitly, что #249-шеврон для near-orthogonal узлов (#279) … не пересматривается». В §3.6 не-скоуп перечисляет модель данных, hit-зоны, «политику полного mitre в допустимом пороге (#302 решение №5)» и #308 — но не называет #249/#279 вовсе.

    Это не формальность: в кодовой базе есть второй, самостоятельный лимит — MULTI_WALL_JOIN_LIMIT = 1.25 (src/wall-thickness.ts:83, «#249»), применяемый через node.limit в multiWallBevelCutsAt (:2642-2666, вырез лишнего клина у экстерьерного полотна/paper envelope) и в mitre-логике контуров комнат (:1475, :3875, multiWallNodeAt(...)?.limit). Число 1.25 лежит прямо рядом с новым VISUAL_MITRE_LIMIT = 1.5, который вводит §3.1. docs/WALL-THICKNESS.md:189 подтверждает статус-кво: «A degree-3+ node closes with a FULL mitre… the #249 chamfer is retired» — то есть для вееров кладки (то, что правит #309) #249-лимит действительно не участвует (ретировался вместе с #302), но :120-126 там же говорит, что «the same bounded rule [R = 1.25·H] applies to the exterior half-wall and paper envelope» и к mitre контуров комнат — то есть механизм жив в двух других местах, которые используют ТУ ЖЕ структуру MultiWallNodeMap/node.limit, что и веер, который правит #309.

    Технически ТЗ, видимо, действительно не задевает эти два места (§3.2/§3.4 правят только linearWallJoinPatches и junctionNodeGeometry), но сам факт, что рядом с новым порогом 1.5h лежит старый порог 1.25h на той же карте узлов в соседнем коде — ровно тот случай, где реализатору легко решить «раз уж я вижу node.limit, поправлю заодно» и задеть paper/room-contour рендер, которого AC не касаются и golden для него не планируется. Один явный пункт в §3.6 («#249/node.limit — вырез экстерьерного полотна и mitre контуров комнат — не пересматривается, использует свой независимый R = 1.25·H») закрывает находку r1 по существу, а не только по форме.

  2. Touch impact не назван нигде явным пунктом. DoR (PROCESS.md §2.5) требует явно: «влияние на touch по docs/TOUCH-SUPPORT.md (View и киоск — блокирующие)» — как отдельный обязательный пункт очереди «Готово к разработке», не только как часть §7.1. docs/TOUCH-SUPPORT.md фиксирует View и kiosk как touch-first, полностью поддерживаемые поверхности — и ровно они входят в скоуп по §0 (общий структурный кэш). §3.7 UX ограничивается «никаких новых контролов», что не то же самое, что явный ответ по touch. Ответ тривиален («чистая рендер-геометрия, никаких интерактивных элементов не меняется — идентична на touch и mouse/keyboard»), но раздел должен произносить его явно, а не подразумевать — тот же принцип, которым в r1 обосновано требование явного i18n/миграции при тривиальном ответе.

Оба пункта решаются автором самостоятельно, без вопроса владельцу: ни один не требует продуктового решения — #249/#279 факт выводится из уже читаемого кода и канонического документа, touch — прямое следствие «нет новых интерактивных элементов», уже фактически сказанного в §3.7. Без High-находок это жёлтый вердикт: два предложения добавляются в §3.6, следующий заход проверяется по дельте этих двух правок.

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

  • 7 из 8 пунктов находки r1 закрыты полностью, текстом, который отвечает требованию r1 буквально (см. таблицу «Закрытие раунда r1»).
  • §0 «До/после» использует ту же величину («полутолщина» = h = halfDepth), что и контракт §3.1-3.2 и математика AC4 (1.41h < 1.5h) — согласовано, не разъезжается с техническим текстом.
  • §3.8/§3.9 («нет модели данных», «нет i18n») корректны по существу: изменение чисто рендер-геометрическое, конфиг/экспорт/импорт не трогает ни один AC.
  • §3.10 риск 3 («удаление парных патчей в узлах ≥3 лучей может вскрыть непокрытые сектора») корректно ссылается на §3.3, где то же самое уже названо как решаемое измерением — согласовано, не новое обещание без опоры.
  • §3.11 release-артефакты (changelog RU+EN, docs/WALL-THICKNESS.md §3, screenshot-fingerprint) — полный список по требованию docs/specs/README.md; способ ревью golden (--reviewed) уже назван в неизменённом §3.5, повторно дублировать в §3.11 не обязательно.
  • Статус-строка (Статус: ревизия 2 (после SPEC-REVIEW-309-r1: …)) корректно ссылается на r1 — трассируемость ревизии на месте.
  • Трейлеры коммита f0ea376a: Issue: #309, User-Visible: no — корректны для документационной правки (не меняет поведение, меняет только ТЗ).

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

  • npx tsc --noEmit / npm test / npm run build — на этом этапе снова нет изменений src/** (git diff origin/dev...HEAD — только 2 файла документации), гейты кода относятся к этапу код-ревью (§2.7), как и в r1.
  • npm run golden:capture/golden:verify и фикстуру test/fixtures/309-junction-teeth.json — фикстура ещё не создана, план тестов не менялся дельтой, предмет реализации/код-ревью.
  • Реальное поведение multiWallBevelCutsAt/room-polygon mitre на конкретном экспорте владельца — находка выше про #249/#279 основана на чтении кода и docs/WALL-THICKNESS.md, не на исполнении; для проверки корректности самого этого утверждения о статус-кво это достаточно (речь о том, что ТЗ должно сказать, а не о поведении будущей реализации), но не является заменой код-ревью после реализации.
  • docs/USER-GUIDE.ru.md — как и в r1, не переоценивал: задача не вводит пользовательских терминов интерфейса (чистая геометрия рендера).

Вердикт

Жёлтый. High-находок нет. Один Medium в скоупе задачи: раздел «Скоуп и не-скоуп» (§3.6), добавленный в ревизии 2, закрывает находку r1 только частично — не называет явно, что #249/node.limit (вырез экстерьерного полотна/paper envelope и mitre контуров комнат, R = 1.25·H, живой и независимый от нового VISUAL_MITRE_LIMIT = 1.5 механизм в соседнем коде той же карты узлов) не пересматривается; отдельно не назван обязательный по DoR (PROCESS.md §2.5) пункт про touch-impact. Оба пункта решаются автором самостоятельно, ответы тривиальны, правка — два предложения в том же файле. Остальные 7 из 8 пунктов находки r1 закрыты полностью и корректно; технический контракт (§3.1-3.5), AC1-7, план тестов и откат не менялись дельтой и наследуются из r1 без повторной проверки.