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

19 KiB
Raw Permalink Blame History

SPEC-REVIEW-309-r3

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

Скоуп

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

Предмет r3 — ревизия 3 ТЗ (коммит 2ade4352), написанная в ответ на жёлтый вердикт r2: «§3.6 — явный не-скоуп механизма #249 …; §3.8a — touch-impact …» (комментарий владельца от 2026-08-25T18:13:49Z, IC_kwDOTOcLQM8AAAABQr2fJA — id 5414712356). Разбор по дельте (§2.10): единственная находка r2 состояла из двух подпунктов одного Medium, дельта — точечная правка того же файла (два новых абзаца), новую подсистему не задевает, контракт поведения (§3.1–3.5) не меняет → полный повторный разбор не требуется, проверяю дельту плюс всё, до чего она дотягивается.

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

  1. Нашёл вердикт предыдущего раунда и SHA, на котором он получен: комментарий 5414679506 (2026-08-25T18:11:18Z) — «Вердикт: жёлтый · заход r2 · блокирующих циклов 2/4 · High: 0 · Medium: 1 → в задаче», документ docs/reviews/SPEC-REVIEW-309-r2.md, получен на коммите спека f0ea376a (сам документ явно называет этот SHA в шапке — не находка, SHA назван).
  2. Объявляю дельту: git diff f0ea376a..2ade4352 -- docs/specs/309-junction-visual-limit.md — 10 вставленных строк, 1 изменённая (строка статуса), 0 удалённых. Все вставки: (a) новый абзац в §3.6 «Скоуп и не-скоуп» про механизм #249; (b) новый раздел §3.8a «Touch»; (c) новая строка AC «7a» (помечена в тексте как AC8) про не-скоуп #249. Разделы §0–§3.5, §4, AC1-6 и AC7 (мутанты), §6, §7 — байт-в-байт те же, что были на f0ea376a и уже проверены в r1/r2.
  3. По каждому из 2 подпунктов находки r2 построчно сверил новый текст с требованием (таблица «Закрытие раунда r2» ниже) и перепроверил фактическую точность заявлений — не на веру автору, чтением кода:
    • src/wall-thickness.ts:80 (MITRE_LIMIT = 4), :83 (MULTI_WALL_JOIN_LIMIT = 1.25, комментарий «#249» рядом в коде), :1996 (node.limit: MULTI_WALL_JOIN_LIMIT * halfDepth — единственное место присвоения), :1475 и :3875 (multiWallNodeAt(...)?.limit — оба сайта mitre контуров комнат), :2612-2696 (multiWallBevelCutsAt, вырез экстерьерного полотна/paper envelope через node.limit, вызовы на :2642-2666).
    • Построчно прошёл тела linearWallJoinPatches (:1082-…) и junctionNodeGeometry (:2032-…) целиком (awk по диапазону функции) — ни одна не ссылается на .limit/node.limit/MULTI_WALL_JOIN_LIMIT; обе считают собственный локальный limit = MITRE_LIMIT * max(halfDepth) (:1144, :2087). Это подтверждает заявление §3.6 «AC8» буквально — функции, которые правит #309, физически не читают node.limit.
    • src/wall-thickness.ts:129 (interface MultiWallNodeMap) и buildMultiWallNodeMap (:1809) — карта действительно общая (используется и веером junctionNodeGeometry, и room-contour/paper кодом), что подтверждает точность фразы §3.6 «MultiWallNodeMap используется совместно, но node.limit не переопределяется».
    • src/houseplan-card.ts:19724-19760 (touchStroke, physical-hit) и src/styles.ts:2256-2269 — hit-зоны рисуются отдельными <line>/<path class="physical-hit"> с фиксированной шириной обводки по осевой линии/грани стены, геометрически независимо от формы патчей стыков (linearWallJoinPatches/junctionNodeGeometry). Подтверждает §3.8a: правка формы кладки в узле не может задеть эти элементы.
  4. git diff origin/dev...HEAD --stat — только документация (3 файла: спек + оба предыдущих ревью), кода нет. Как и в r1/r2, npx tsc --noEmit / npm test / npm run build на этом этапе неприменимы — не гоняю по той же причине (нет изменений src/**), это не пропуск гейта, а отсутствие предмета для него.
  5. Прочитал docs/WALL-THICKNESS.md:100-230 повторно (тот же диапазон, что и в r2) для сверки статус-кво механизма #249/#279, поскольку новый текст §3.6 этот статус-кво пересказывает — совпадает.
  6. Прочитал docs/TOUCH-SUPPORT.md (контракт View/kiosk как touch-first, блокирующих поверхностей) — новый текст §3.8a не противоречит ему и корректно называет hit-зоны/жесты/панораму-зум как неизменные.
  7. git log -1 --format=%B 2ade4352 — трейлеры Issue: #309 / User-Visible: no на месте, корректны для документационного коммита.

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

Находка r2 (подпункт) Чем закрыта Где это видно
#249/node.limit не исключены явно из скоупа Добавлен абзац в §3.6: «Явный не-скоуп — механизм #249: MULTI_WALL_JOIN_LIMIT = 1.25 (:83) и его потребители — multiWallBevelCutsAt (paper envelope) и mitre контуров комнат (:1475/:3875) — не изменяются… MultiWallNodeMap используется совместно, но node.limit не переопределяется. AC8 закрепляет это диффом» docs/specs/309-junction-visual-limit.md:61
Touch-impact не назван явным пунктом DoR Добавлен §3.8a «Touch (docs/TOUCH-SUPPORT.md)»: «Touch-контракт не затрагивается: … hit-зоны (physical-hit, touchStroke), жесты, панорама/зум и порядок обработки касаний не изменяются. View/kiosk … получают ту же геометрию из общего структурного кэша без каких-либо новых слушателей» :71-73

Оба подпункта находки r2 закрыты полностью и по существу (не только по форме): проверено чтением кода (шаг 3 выше), что оба заявления фактически верны, а не правдоподобная догадка, выданная за факт.

Унаследовано из r2 (и транзитивно из r1)

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

  • Продуктовая рамка/скоуп-соответствие SCOPE.md J1/J6 («Скоуп» r1).
  • §0 «Сценарий» и «до/после» — персона, поверхности, момент (закрытие 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, прямые/тупые углы не задеты.
  • Остальная часть §3.6 «Скоуп и не-скоуп» (модель данных/конфиг, hit-зоны/снап, политика #302 решения №5, #308) — не менялась дельтой r3, закрыта в r1/r2.
  • §3.7 UX, §3.8 «Модель данных и миграция», §3.9 i18n, §3.10 Риски (4), §3.11 Release-артефакты — закрыты в r2, дельтой r3 не тронуты.
  • AC1–6 и AC7 (мутанты a-d) (§5) — однозначны, у каждого указано доказательство (юнит/golden/смок), проверены в r1.
  • План автотестов (§6) и откат (§7) — полны, проверены в r1.
  • Low-находка r1 (расхождение «16» vs «15+1» junction-сцен) — снята ревьюером без правки в r1, не проверяю повторно: не влияет ни на один AC.
  • Трейлеры документационных коммитов (Issue: #309, User-Visible: no) — корректны для ff623cd4 и f0ea376a, проверено в r1/r2; для 2ade4352 та же схема (см. «Как проверялось», шаг 7).

Находки

Low (не блокирует, отмечаю для полноты)

Нумерация AC в §5 разъезжается: новая строка помечена «7a», но названа в прозе «AC8», а следующая (мутанты) осталась «7».

Фактическая последовательность в ## 5. AC: 1, 2, 3, 4, 5, 6, 7a, 7. Новая строка, добавленная этой ревизией — 7a. **Не-скоуп #249 (AC8):** … — стоит перед уже существующим пунктом 7. **Мутанты:** …, хотя в тексте §3.6 та же строка называется «AC8» («AC8 закрепляет это диффом»). Формально у документа теперь два разных обозначения одного и того же критерия (7a как заголовок списка и AC8 как ссылка на него из другого раздела), а «Мутанты» — фактически восьмой по счёту критерий — сохранил старый номер «7». DoR (PROCESS.md §2.5) требует «AC1…ACn — пронумерованные проверяемые критерии» — строго говоря, 7a не входит в последовательность 1..n.

Не блокирует: содержание однозначно (единственный кандидат на «AC8» — строка 7a, спутать её с чем-то другим невозможно), ни один AC не теряет доказуемости, а на код-ревью не может возникнуть двух прочтений. Рекомендация для следующей правки документа (не обязательно в этом раунде): перенумеровать подряд 1…8 (не-скоуп #249 = AC7, мутанты = AC8) или убрать скобку «(AC8)» и называть критерий по его фактическому заголовку «AC 7a» везде. Снимаю без блокировки: чисто редакционный дефект, введённый этой же дельтой, не влияет на проверяемость.

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

  • Оба подпункта находки r2 (#249/node.limit не исключён явно; touch-impact не назван) закрыты полностью и по существу — не декларацией, а текстом, который я перепроверил чтением кода и который оказался технически точным (см. «Как проверялось», шаг 3): linearWallJoinPatches и junctionNodeGeometry действительно не читают node.limit/ MULTI_WALL_JOIN_LIMIT, а hit-зоны действительно нарисованы отдельными DOM-элементами с фиксированной шириной, не зависящей от формы патча стыка.
  • §3.6 корректно указывает конкретные строки-потребители #249 (multiWallBevelCutsAt, :1475, :3875) — все три существуют и делают ровно то, что написано.
  • §3.8a корректно называет три поверхности, которые получают одну и ту же геометрию из структурного кэша (View/kiosk touch-first по docs/TOUCH-SUPPORT.md), не изобретая новый touch-термин.
  • Новая строка AC (7a/«AC8») методологически корректна: критерий проверяем («не входят в дифф») и способ доказательства указан («ревью кода по диффу + существующие тесты») — тот же формат, что у остальных AC.
  • Статус-строка (Статус: ревизия 3 (после SPEC-REVIEW-309-r2: …)) корректно ссылается на r2 — трассируемость ревизии на месте.
  • Трейлеры коммита 2ade4352: Issue: #309, User-Visible: no — корректны для документационной правки.
  • Технический контракт (§3.1-3.5), AC1-7 (мутанты), план тестов, откат — не менялись дельтой r3, наследуются из r1/r2 без повторной проверки (см. раздел выше).

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

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

Вердикт

Зелёный. High-находок нет, Medium-находок нет. Оба подпункта Medium-находки r2 (явный не-скоуп механизма #249/node.limit, явный touch-impact) закрыты полностью и технически точно — проверено чтением кода, не на веру автору. Одна Low-находка (нумерация нового AC «7a»/«AC8» разъезжается с соседним «7») не блокирует: содержание однозначно, ни один AC не теряет доказуемости, дефект чисто редакционный. Технический контракт (§3.1-3.5), AC1-7 (мутанты), план автотестов и откат не менялись дельтой r3 и наследуются из r1/r2 без повторной проверки. ТЗ готово к DoR (§2.5): все обязательные разделы присутствуют, критерии приёмки однозначны и снабжены способом доказательства, открытых продуктовых вопросов нет.