docs: review document for #310

Issue: #310
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-25 20:10:38 +00:00
parent 8d41651b1e
commit 9ec3a77ffa
+80
View File
@@ -0,0 +1,80 @@
# SPEC-REVIEW-310-r2
Issue: #310 «Парный острый стык: вернуть полное остриё, зубец торца толстой стены срезать по граням острия (follow-up #309)»
Этап: ревью ТЗ (PROCESS.md §2.4)
Документ ТЗ: `docs/specs/310-pair-apex.md` (ветка `issue/310-pair-apex`, ревизия 2, коммит `8d41651b`)
Заход: r2 · блокирующих циклов израсходовано 1 из 4 (потрачен r1 — жёлтый; сам r2 бюджет не тратит, если вердикт зелёный, §227)
Метка: `small` не установлена (labels: `bug`, `P2`, `polish`, `S4-spec-review`) → полный трек подтверждён, файл ТЗ обязателен и на месте — без изменений с r1.
## Скоуп
Раунд r1 (`docs/reviews/SPEC-REVIEW-310-r1.md`, база `c8d56b6c`) поставил один блокирующий High: AC5 и Риск №2 называли единственным гейтом `junctionContractHoles`, а тот структурно не видит узел-пару (`buildMultiWallNodeMap` отбрасывает узлы с < 3 канонических лучей — `src/wall-thickness.ts:1984,2032`), то есть главный риск задачи («дыры на границе нового среза») оставался без рабочей проверки именно там, где #310 вводит новую операцию.
Дельта r1→r2 — коммит `8d41651b` (`docs: spec #310 revision 2 — grid contract for two-ray nodes per review r1`), `docs/specs/310-pair-apex.md`, **+4/-4 строки**, единственный файл в диапазоне:
```
git diff c8d56b6c..8d41651b -- docs/specs/310-pair-apex.md
```
Затронуты ровно 4 места: строка статуса, Риск №2 (§5), AC5 (§7) и строка плана тестов (§8). Ни один другой раздел ТЗ не менялся — сценарий, решения владельца, контракты §2.1/§2.2/§2.3, скоуп/не-скоуп, UX/данные/i18n, риски 1 и 3, release-артефакты, AC1–AC4/AC6/AC7, откат идентичны байт-в-байт коммиту `c8d56b6c`, уже проверенному в r1.
Дельта затрагивает ровно тот AC, который r1 назвал дефектным, и ничего за его пределами — ни ребейза на ушедший вперёд `dev`, ни смены контракта поведения (§1/§2.1–2.3 не тронуты), ни новой подсистемы. Условие §2.10 «разбор по дельте, а не заново» применимо без оговорок: полный повторный разбор не требуется, разбор ограничен closure High-находки и проверкой, что правка не расшатала соседние AC.
Изменений продуктового кода в диапазоне нет (весь коммит — docs). Гейты `tsc`/`test`/`build`/`invariants` не запускал — на этапе ТЗ код не менялся, гонять нечего (то же основание, что и в r1).
## Как проверялось
1. Найден вердикт r1 и SHA, на котором он получен: `docs/reviews/SPEC-REVIEW-310-r1.md`, база `c8d56b6c` (сама r1 явно называет коммит в шапке — не находка, в отличие от типового случая, который описывает эта роль).
2. `git diff c8d56b6c..8d41651b -- docs/specs/310-pair-apex.md` — дельта объявлена и процитирована выше целиком (4 хунка).
3. По High-находке r1: прочитан новый текст §5 Риск №2 и §7 AC5 построчно, сверен с двумя вариантами правки, которые предлагала r1 («(а) отдельный способ доказательства для узла-пары — сэмплирование окрестности без фильтра по числу лучей» или «(б) расширить вход `junctionContractHoles`»). Определено, какой вариант выбран и закрывает ли он именно то, что было названо дефектом.
4. Перечитан код `buildMultiWallNodeMap` (`src/wall-thickness.ts:1984,2032`) и `linearWallJoinPatches` (парная ветка) повторно — не потому что он изменился (не менялся), а чтобы независимо проверить корректность нового текста AC5: действительно ли формула «полоса A ∪ полоса B ∪ mitre-патч − клинья» отражает то, как тело узла-пары фактически строится, и не является ли обоснование «клин лежит строго снаружи наружной грани соседа» голословным.
5. Проверена нотация: `h` в новом AC5 («радиус 3·max(h), шаг ≤ h/4») сверена с уже установленным в `docs/WALL-THICKNESS.md:207` употреблением «`VISUAL_MITRE_LIMIT = 1.5` maximal half-depths» (`h` = полутолщина) — та же нотация, что и в AC1 ТЗ («вылет больше 1.5·h»), не новое обозначение и не расходится по документу.
6. Проверено логически, различает ли новая эквивалентность (⇔, а не ⊇ как в общем инварианте `docs/WALL-THICKNESS.md:201`) все три мутанта §7.7 (a — фаска возвращена, b — торцевой трим отключён, c — трим не ограничен окрестностью): для (a)/(c) реальное тело теряет точки, которые формула требует как кладку → нарушение направления «формула ⇒ реальность»; для (b) реальное тело сохраняет зубец, которого формула (за вычетом клина) не предусматривает → нарушение направления «реальность ⇒ формула». Оба направления содержательны, эквивалентность — не избыточное усиление и не пропуск.
7. Прочитано тело issue #310 и последний комментарий владельца/автора — подтверждено, что ревизия 2 запушена именно в ответ на r1 и что автор просит перезапуск ревью, а не оспаривает находку.
8. Перепроверено, что низкая (Low) находка r1 (блок «принято предположительно») была снята самой r1 без требования правки — ревизия 2 её не касается, повторной проверки не требует.
## Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
| **[High]** AC5/Риск №2 называют детектором `junctionContractHoles`, который структурно не видит узел-пару (< 3 лучей отбрасываются `buildMultiWallNodeMap`) — главный риск задачи без рабочей проверки. | Риск №2 переписан: явно называет причину («детектор #302 узлы-двойки не видит: `buildMultiWallNodeMap` требует ≥3 лучей») и вводит раздельные гейты по типу узла. AC5 переписан: для узлов ≥3 лучей остаётся прежний `junctionContractHoles`; для узлов-двоек вводится независимый **парный сеточный контракт** — сэмплирование квадратной окрестности узла (радиус `3·max(h)`, шаг `≤ h/4`) на фактическом `physicalBodySet.geometry`, сверяемое с декларативной формулой `(полоса A ∪ полоса B ∪ mitre-патч) − клинья #310`, явно на узле `spike` фикстуры `309-junction-teeth` и на синтетических парах (острая/90°/почти-параллельная). Это в точности вариант (а), который r1 предлагала как приемлемое закрытие («прямая проверка через сэмплирование окрестности узла на `physicalBodySet.geometry`… без фильтра по числу лучей»). | `docs/specs/310-pair-apex.md` §5 (Риск №2) и §7 AC5, коммит `8d41651b`, `git diff c8d56b6c..8d41651b` (хунки 3 и 4). |
| **[Low]** Технические решения (фазировка нового трима) не оформлены явным блоком «принято предположительно» — снято самой r1 без требования правки. | Правки не требовалось; ревизия 2 раздел не касается. | Не применимо — находка закрыта решением ревьюера в r1, автор ничего не менял. |
## Унаследовано из r1
Без повторной проверки в r2 принято (документ и SHA: `docs/reviews/SPEC-REVIEW-310-r1.md`, база `c8d56b6c` — соответствующий текст ТЗ в этих разделах байт-в-байт идентичен `8d41651b`):
- **Продуктовая рамка (§0 сценарий, «до/после»).** Персона (Home admin, десктоп-редакторы), связь с `docs/SCOPE.md`, отсутствие расширения скоупа — точечный визуальный баг-фикс, follow-up #309.
- **Решения владельца перенесены точно (§1).** Буквальное совпадение с текстом обоих комментариев владельца в issue.
- **Контракт §2.1 (парная ветка mitre).** Построчно сверен с `wall-thickness.ts:1193-1197`; `MITRE_LIMIT`/`chamferApex` действительно не участвуют в парной ветке до этой задачи.
- **Контракт §2.2 (торцевой трим).** Обоснован реальной геометрией `linearWallBody` (строка 1062), а не догадкой.
- **Ручной пересчёт узлов фикстуры `309-junction-teeth`.** `step` — 4 луча, `spike` — 2, `hump` — 3; заявление «step/hump не трогаются» подтверждено.
- **Перечень потребителей парных патчей (§3).** Три вызывающих места `linearWallJoinPatches` и четыре сайта `physicalBodyParts(...)` в `houseplan-card.ts`/`space-render.ts` — полны.
- **Не-скоуп корректен.** `virtualJunctionPatches`/`unionJunctionPatches` и механизм #249 — независимы от узла-пары, исключение обоснованно.
- **AC1, AC3, AC4, AC6, AC7 однозначны**, называемые идентификаторы/тесты/golden-сцены существуют в коде.
- **Риск №1** (возврат «хвостов») — осознанное и уже принятое владельцем поведение.
- **Риск №3** (интерференция #271+#310) — адекватно закрыт юнитом на коротком толстом саппорте пары; степени узла (#271 требует ≥3 луча, #310 — ровно 2) взаимоисключающие, кроме случая разных концов одного отрезка — учтено тест-планом.
- **Release-артефакты, UX/данные/i18n, откат (§4, §6, §9)** — полны, без миграций и новых полей.
- **Трейлеры и класс изменения.** `Issue: #310`, `User-Visible: no` на месте и в `8d41651b`; класс C (документация) корректен для обоих коммитов ветки.
## Что проверено заново и корректно (дельта r2)
- **AC5 закрывает названный риск, а не переформулирует его.** Новый гейт для узла-пары сэмплирует реальную (`physicalBodySet.geometry`), а не повторно вычисленную кодом под тестом, геометрию — сравнение с независимо заданной формулой (полосы — простая прямоугольная геометрия стены, mitre-патч — то же `[node, pA, hit, pB]`, что и в AC1, клин — декларативное полупространство из §2.2) не тавтологично: ошибка именно в операции вычитания (риск №2) проявится как несовпадение формулы и факта, а не спрячется за общим кодом.
- **Направление проверки (⇔, не ⊇).** В отличие от общего инварианта `docs/WALL-THICKNESS.md:201` (`body ⊇ strips ∪ fans`, допускающего лишний материал у вееров), для пары выбрана более строгая эквивалентность — обоснованно: она одновременно ловит и недостачу (дыра, риск №2, мутанты a/c), и избыток (зубец не срезан, мутант b). Ослабление до ⊇ пропустило бы мутант (b).
- **Нотация `h` и радиус сэмплирования.** `h` = полутолщина, употребление совпадает с уже принятым в AC1 и `docs/WALL-THICKNESS.md:207`; радиус `3·max(h)` заведомо превышает зону вмешательства трима (`2·halfDepth(своей)` из §2.2) с запасом — окно сэмплирования покрывает всю область, где дыра в принципе может появиться, а не только формально названную.
- **Явное закрытие узла `spike`.** В отличие от r1-версии AC5 (обезличенная формулировка «на фикстуре и сценах»), новая формулировка называет именно узел `spike` фикстуры `309-junction-teeth` — тот самый двухлучевой узел, который r1 показала исключённым из `map.nodes`.
- **§8 план тестов согласован с §7 AC5** без противоречий: юнит парного сеточного контракта на `spike`-узле и синтетике плюс отдельный юнит детектора #302 на фикстуре для узлов ≥3 — разделение по типам узлов проведено последовательно во всех трёх местах (Риск №2, AC5, план тестов).
- **Дрейф формулировки «юнит + смок» → «юнит».** В r1-версии AC5 стояло «юнит + смок»; в r2 упоминание смока пропало у обеих ветвей гейта. Это не регрессия: `smoke_junction_holes` (`docs/WALL-THICKNESS.md`) — существующий wiring-проб именно для инварианта вееров ≥3-лучевых узлов, который в этой задаче не меняется (§2.3), а не для новой парной ветки; отдельного смока для пар никогда не существовало, и golden-сцена `junction-309-spike-dark` (AC2) закрывает ту же цель — «форма дошла до реального потребителя рендера» — визуально, через настоящий пайплайн. К AC5 претензий нет; выбор конкретных browser-смоков для запуска в этой задаче (если появятся) остаётся за код-ревью через `scripts/smoke-select.mjs` по фактическому диффу кода.
## Чего не проверял
- Полный обход `demo/golden/matrix.mjs` на прочие 2-лучевые сцены за пределами трёх названных `junction-309-*` — унаследовано из r1 (там же не проверялось), причина та же: код не менялся, гонять `npm run golden:verify` не на чем.
- Численную корректность формул (`chamferApex`, поведение при `sin ≈ 0` в парной ветке) — унаследовано из r1, это предмет код-ревью реализации.
- `npx tsc --noEmit` / `npm test` / `npm run build` / `node scripts/check-docs.mjs` — не запускал; диапазон `c8d56b6c..8d41651b` не содержит правок `src/**` и тестов, гонять нечего на этапе ТЗ.
- `npm run invariants` — задача не трогает модель данных (`layout`, записи толщины, `marker.space`, `open_spans`), только вычисляемую геометрию рендера; инвариант о ключе решёточного ребра неприменим — унаследовано из r1.
- Реализацию AC5 как теста (существует только как описание в ТЗ; тест ещё не написан) — предмет код-ревью, когда появится диапазон с кодом.
## Итог
Дельта r1→r2 — 4 строки в `docs/specs/310-pair-apex.md`, целиком внутри Риска №2, AC5 и строки плана тестов, то есть ровно там, где r1 поставила блокирующий High. Новый AC5 вводит для узла-пары независимый сеточный контракт «формула vs факт» на реальной геометрии, явно называет проблемный узел `spike`, использует установленную нотацию без новых неоднозначностей и по построению (проверено логически, направление ⇔) закрывает и риск «дыра», и все три мутанта §7.7 — то есть реализует вариант (а), который r1 сочла приемлемым. Низкая находка r1 закрыта ранее и ревизии не касалась. Новых находок нет.
**Вердикт: зелёный.**