mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,85 @@
|
||||
# SPEC-REVIEW-275-r3
|
||||
|
||||
- Issue: [#275](https://github.com/Matysh/houseplan-card/issues/275) — «Multi-wall bevel beta.6 вырезает реальные полосы стен: белые уголки, потеря толщины и крупные провалы»
|
||||
- Этап: ТЗ на ревью (PROCESS.md §2.4)
|
||||
- Артефакт ТЗ: `docs/specs/275-multiwall-strip-containment.md`, коммит `3bf5a1db26f43eee1d11e95ce8b6413386e46920` (ветка `issue/275-multiwall-strip-containment`)
|
||||
- Предыдущий заход: r2, `docs/reviews/SPEC-REVIEW-275-r2.md`, вердикт **жёлтый**, на коммите `62dd02d8234a8d5b9b629e2fd77b1c09f00932ab`
|
||||
- Заход: r3 · блокирующих циклов израсходовано 1 из 4 (жёлтый r2 списал единственный потраченный цикл; зелёный вердикт цикла не образует, PROCESS.md §4/#227)
|
||||
- Ревьюер: свежая сессия, без устных пояснений автора
|
||||
|
||||
## Почему разбор полный, а не только по находке r2
|
||||
|
||||
r2 вернул одну Medium-находку в скоупе: классификация узла целиком (rectilinear/non-rectilinear) оставляла ортогональную пару смешанного узла (T/X + диагональ) без защиты, потому что весь узел с хотя бы одной непрямой парой считался non-rectilinear и получал старый безусловный cut. Автор принял находку и переписал контракт: единица классификации — не узел, а **пара лучей** и **луч** («protected ray» — луч, у которого есть хотя бы один перпендикулярный партнёр в узле), а вычитаемое множество — не отдельный cut конкретной пары, а весь `pairwiseCuts` узла минус union protected-strips.
|
||||
|
||||
Это замена **инварианта классификации** (единица защиты «узел» → «луч через union protected strips, вычитаемый из всех cuts узла») — смена контракта поведения в смысле PROCESS.md §2.10 («разбор остаётся полным, если... смена контракта поведения»), а не косметическая правка одного предложения. Поэтому я заново, от первых принципов, проверил новый раздел 6 (6.1–6.7) целиком и все AC, которые он определяет (AC1, AC2, AC3, AC5, AC6, AC7), включая сверку с реальным кодом `bevelMultiWallBody`/`multiWallBevelCutsAt`, а не только текстовое сравнение с r2. Разделы, которых дельта не касается по существу (§1, §2, §7 кроме одной переформулированной строки, AC4, AC8, §9, §10, §12, §13 п.1/3/4/5), не разбирались повторно — см. «Унаследовано из r2».
|
||||
|
||||
## Дельта (`git diff 62dd02d..3bf5a1d` на `docs/specs/275-multiwall-strip-containment.md`)
|
||||
|
||||
159 строк (+93/−66 по факту сверки). Изменены: заголовок статуса; §3 (добавлен абзац «Spec review r2 указал на смешанный node...» и переформулирован предыдущий абзац с «rectilinear T/X» на «T/X nodes, где каждая потерянная ray входит хотя бы в одну перпендикулярную пару»); §4 («что человек увидит» переписано с уровня узла на уровень луча); §5 (Входит — классификация пар вместо классификации узла, добавлен явный пункт про смешанные узлы); §6 целиком (6.1 «Классификация rectilinear node» → «Классификация перпендикулярных ray pairs», 6.2 `requiredStripUnion` → `requiredOrthogonalStripUnion` по protected rays, 6.3 добавлена явная формула `effectiveCut = pairwiseCuts − requiredOrthogonalStripUnion(node)` и явный запрет частичной защиты диагональных cuts, 6.4/6.5/6.6/6.7 переименованы под «protected strip» вместо «rectilinear node»); §7 (одна строка переименована, смысл не изменён); AC1/AC2/AC3/AC5/AC6/AC7 (термины сужены/переименованы под per-ray protection, AC7 получил новый обязательный mixed-node fixture); §11 (первая строка риска переформулирована); §13 (пункт 2 переписан под per-ray логику). Продуктовый код не тронут: `git diff origin/dev...HEAD` за весь issue — три файла, все в `docs/` (класс C), включая публикацию `docs/reviews/SPEC-REVIEW-275-r{1,2}.md` — не авторская правка.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Перечитан диапазон `62dd02d..3bf5a1d` построчно (см. выше) и весь текущий `docs/specs/275-multiwall-strip-containment.md`.
|
||||
- Перечитаны `docs/reviews/SPEC-REVIEW-275-r{1,2}.md` целиком, включая их находки и то, что уже сверено с кодом.
|
||||
- Комментарии issue #275 прочитаны целиком: хендоффы и вердикты r1/r2, «Авторский контрпример», хендофф r3 («Замечание review r2 принято... 110 из 437... 100 из 382...»).
|
||||
- Заново, от первых принципов, проверена новая механика §6.1–6.3 против `src/wall-thickness.ts`, а не только текст:
|
||||
- `multiWallBevelCutsAt` (L2016–2095) строит cut **только для угловых соседей** в `node.rays`, отсортированных по углу (`rays.sort(... a.angle - b.angle)`, L1610; аналогично L839 для junction-детекции) — т.е. «pairwise cut» в коде это cyclic-adjacent пары, не все `C(n,2)`. Это существенно для проверки нового контракта: r3 явно требует защиты protected-ray strip **и от cut соседней диагональной пары того же узла**, а не только от cut между двумя ортогональными соседями — я подтвердил, что для смешанного узла (см. ниже) диагональные cuts действительно смежны с protected-лучом в угловом порядке и потому геометрически способны его задеть, если их не ограничить.
|
||||
- `bevelMultiWallBody` (L2125–2219): `local` — union прямоугольников `ray.supports` всех лучей узла (L2153–2174, без разбора на protected/unprotected — как и требует §6.2, «union полос **только** protected rays» относится к вычитаемому множеству, а не к построению `local`), затем `local = difference(local, retainedCuts)` (L2183), где `retainedCuts` — union **всех** angularly-adjacent pairwise cuts узла. Формула ТЗ `effectiveCut = pairwiseCuts − requiredOrthogonalStripUnion(node)` реализуема прямой заменой: `retainedCuts' = difference(retainedCuts, protectedUnion)` перед `difference(local, retainedCuts')` — то есть контракт не постулирует несуществующий в коде механизм, а описывает конкретную, выполнимую в существующей структуре модификацию одной строки.
|
||||
- Смоделирован эталонный смешанный узел из AC7 (`north(0°)/diagonal(45°)/east(90°)/south(180°)`, отсортирован по углу как N, diag, E, S): угловые соседние пары и их gap — `(N,diag)=45°`, `(diag,E)=45°`, `(E,S)=90°`, `(S,N)=180°` (последняя пропускается кодом, `gap >= π − 1e-9`). Скалярное произведение направлений: `dot(N,E)=0` → N и E ортогональны (оба protected); `dot(E,S)=0` (S=(−1,0), E=(0,1)) → E и S тоже ортогональны, так что S тоже protected через E, хотя N и S коллинеарны/противоположны и **не образуют** orthogonal pair между собой (согласовано с текстом §6.1: «параллельные/противоположные rays не образуют такую пару»). diag(45°) не перпендикулярен ни одному из N/E/S (`dot` ни с одним не близок к 0) → diag остаётся unprotected. Итог: cut `(E,S)` (реальный «спайк» классического T) целиком лежит внутри protected union (E и S оба protected) и по r3 полностью исключается из вычитания — именно это чинит основной баг. Cuts `(N,diag)` и `(diag,E)` частично перекрывают protected strips N и E соответственно (эти лучи protected независимо от того, с кем именно строится конкретный cut) — по r3 из этих cuts вычитается protected-часть, а неprotected часть у diagonal остаётся прежним bounded bevel #249. Это ровно тот сценарий, который r2 назвал невоспроизведённым при node-level классификации (там весь узел ушёл бы в non-rectilinear и E/S лишились бы защиты) — проверено вычислением, не переписыванием слов автора.
|
||||
- Проверено само определение «protected ray» как **узловое**, а не «по конкретной паре»: §6.1 «ray, участвующий хотя бы в одной orthogonal pair **этого node**» — то есть N и E защищены как лучи целиком независимо от того, какая конкретная угловая пара породила cut. Подтверждено, что это корректно устраняет находку r2: защита не привязана к тому, находится ли протектед-луч в угловой adjacency именно со своим ортогональным партнёром — она распространяется на любые cuts узла, задевающие его strip, включая cuts, порождённые его adjacency с диагональным лучом.
|
||||
- Проверено, что #249 (`test/fixtures/249-multiwall-junction.json`, углы лучей ~102.3°/45°/−27.7°, уже вычислено и подтверждено в r2 по фактическим координатам) не содержит ни одной пары с `|dot| ≈ 0` — union protected rays для этого узла пуст, `effectiveCut = pairwiseCuts − ∅ = pairwiseCuts`, т.е. §6.4/AC2 корректно утверждают «bevel #249 не меняется» — не текстовое заявление, а следствие формулы 6.3 при пустом protected-множестве.
|
||||
- Сверено содержание AC7: новый абзац «Отдельный mixed-node fixture содержит north/south/east rays и диагональный ray 45°» — это прямой регрессионный тест именно на находку r2 (если реализация случайно вернётся к классификации узла целиком, этот фикстур немедленно провалится, так как узел в целом не rectilinear из-за диагонали, а N/E/S обязаны остаться защищёнными). AC6-мутант («отключает protected-strip subtraction... в обычные **и смешанные** узлы») тоже явно расширен под этот случай.
|
||||
- Численный аргумент хендоффа r3 («защита только overlap перпендикулярной пары покрывала 110 из 437 samples для `1.json` и 100 из 382 для `2.json`») не проверялся напрямую (приватные данные), но он самосогласован с итоговым техническим решением: он объясняет, почему r3 защищает **весь union strip protected-луча от всех cuts узла**, а не только cut между двумя конкретно ортогональными угловыми соседями — более узкий вариант был опробован автором и эмпирически недостаточен. Это тот же класс данных, что процесс уже принимал в r1/r2 без независимой проверки.
|
||||
- Проверено, что §5 «Не входит», §7 (compatibility/UX/i18n/security/performance), §9 (ожидаемые файлы), §10 (release-артефакты), §12 (rollback) не разошлись по смыслу с r2 там, где дельта их не касается — точечно сверено построчно с diff, а не заново прочитано как новый текст.
|
||||
- Гейты `typecheck`/`test`/`build`/`check-docs` не запускались: `git diff origin/dev...HEAD` за весь issue (r1+r2+r3) содержит только файлы `docs/**` (класс C), продуктовый код не менялся ни в одном раунде — прогон этих гейтов не даёт сигнала о качестве ТЗ. То же основание, что в r1 и r2.
|
||||
|
||||
## Закрытие раунда r2
|
||||
|
||||
| Находка r2 | Чем закрыта в r3 | Где это видно |
|
||||
|---|---|---|
|
||||
| Medium: классификация узла целиком — смешанный узел (перпендикулярная пара + диагональ) целиком уходит в non-rectilinear, и его ортогональная пара теряет защиту, воспроизводя баг #275 | Классификация перенесена на уровень пары/луча: `protected ray` — луч с хотя бы одним перпендикулярным партнёром **в узле**, а не свойство узла целиком; `effectiveCut = pairwiseCuts − requiredOrthogonalStripUnion(node)` вычитает protected-union из **всех** cuts узла, включая cuts от диагональных пар. Независимо пересчитано на эталонном смешанном узле `N/diag(45°)/E/S`: N и E остаются protected и защищены от cut `(E,S)` и от protected-части cuts `(N,diag)`/`(diag,E)`, diag остаётся на контракте #249 | `docs/specs/275-...md` §6.1 («protected ray... этого node»), §6.3 (формула `effectiveCut`, явный запрет «недостаточно пропустить cut ровно между двумя перпендикулярными соседними rays»), AC7 (новый mixed-node fixture), AC6 (мутант расширен на «обычные и смешанные узлы») |
|
||||
| Low (снят ревьюером без правки документа): angle-epsilon в §6.1 не назван числом | Не затронуто дельтой по существу — эпсилон остаётся тем же «единым angle-epsilon», защищённым требуемым в AC7 negative boundary-тестом. Основание снятия не изменилось | `docs/specs/275-...md` §6.1 (формулировка сохранена), AC7 («Негативная angle-boundary matrix...») |
|
||||
| Low-1/Low-2 из r1 (канал доказательства AC3; план тестов не отдельным заголовком) | Не затронуты дельтой r2→r3, остаются в силе без изменений (уже сняты в r1) | `docs/reviews/SPEC-REVIEW-275-r1.md`, разделы «Low — 1/2» |
|
||||
|
||||
## Унаследовано из r2 (и транзитивно из r1)
|
||||
|
||||
Принято без повторной проверки — не задевается дельтой r2→r3:
|
||||
|
||||
- полнота обязательных разделов §7.1 PROCESS.md вне изменённых §3/§4/§5/§6/AC1-2-3-5-6-7/§11/§13п2 (сценарий/персона §1, план автотестов §8+9 как формат, release-артефакты §10, rollback §12, AC4 и AC8 текстуально не менялись с r1) — проверено в `SPEC-REVIEW-275-r1.md` на `762e9f4`;
|
||||
- существование инструментов, названных в §8–9 (`scripts/mutation-gate.mjs`, `demo/smoke_multiwall_junction.mjs`, `scripts/smoke-select.mjs`, `demo/golden/harness.mjs`, `demo/golden/matrix.mjs`) — проверено в r1;
|
||||
- статусы связанных issue (#271/#272/#273 CLOSED, #270 OPEN вне процесса) — проверено в r1, не менялось;
|
||||
- точность переноса числовых данных из тела issue в §2 (координаты, `cell_cm`, число rooms/walls/nodes) — §2 не тронут дельтой r2→r3, проверено в r1;
|
||||
- приватность данных (anonymized fixtures в Git, полные backups вне репозитория) — контракт не изменился, AC7 лишь добавил mixed-fixture того же класса;
|
||||
- геометрическая проверка fixture #249 (углы лучей ~102.3°/45°/−27.7°, ни одна пара не ортогональна) — вычислено в r2 на `test/fixtures/249-multiwall-junction.json`, дельта этот факт не меняет, только переименовывает термин «non-rectilinear» → «нет orthogonal pairs»;
|
||||
- геометрическая проверка перпендикулярного случая (`hit` лежит на границе union strip-прямоугольников, а не за ней) — доказана в r2 от первых принципов, остаётся основанием и для r3-формулировки того же механизма под новым именем;
|
||||
- продуктовый факт «сохранённая положительная полоса стены не может быть вырезана renderer-bevel» — зафиксирован владельцем в теле issue, подтверждён в r1, не переоткрывается.
|
||||
|
||||
## Находки
|
||||
|
||||
Ни одной High, ни одной Medium.
|
||||
|
||||
### Low — 1 (новая, снимаю без правки документа): числа «110 из 437» / «100 из 382» из хендоффа не отражены явно в тексте ТЗ
|
||||
|
||||
Хендофф-комментарий автора к r3 приводит конкретную эмпирическую причину, почему защита ограничивается не отдельным cut, а целым protected-union («she покрывала 110 из 437... для `1.json` и 100 из 382 для `2.json`»), но сам текст ТЗ (§3) не повторяет эти числа — там только качественное «недостаточно пропустить cut ровно между двумя перпендикулярными соседними rays». Формально это не создаёт непроверяемости ни одного AC: AC3 (exact-input lifecycle) проверяет оба полных backup файла целиком и потребует `0` нарушений после реализации независимо от того, названы ли эти промежуточные числа в тексте. Снимаю без правки: числа — обоснование выбора между двумя техническими вариантами (protect только смежный cut vs protect весь union), само решение уже зафиксировано в §6.3 однозначно и проверяемо, а не как догадка.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Новая пер-лучевая механика защиты (§6.1–6.3) технически обоснована независимым пересчётом на эталонном смешанном узле (`N/diag(45°)/E/S`), а не только текстом автора: подтверждено, что она защищает ортогональную пару смешанного узла от cuts как между собой, так и от соседних диагональных cuts — именно то, чего не хватало в r2.
|
||||
- Формула `effectiveCut = pairwiseCuts − requiredOrthogonalStripUnion(node)` реализуема прямой модификацией существующего кода (`bevelMultiWallBody`, L2183: `local = difference(local, retainedCuts)`) — не постулирует несуществующий в структуре кода механизм.
|
||||
- `multiWallBevelCutsAt` строит cuts только для угловых соседей отсортированных по углу лучей (подтверждено чтением сортировки на L1610/839) — это техническое свойство кода, релевантное для проверки того, что диагональные cuts действительно геометрически смежны с protected-лучами и потому нуждаются в защите по всему union, а не только «своей» паре.
|
||||
- #249 корректно остаётся вне изменений: пустой protected-union для его узла (нет ни одной ортогональной пары, подтверждено ранее в r2 по фактическим координатам) даёт `effectiveCut = pairwiseCuts`, то есть контракт формулы, а не отдельное текстовое исключение, сохраняет старое поведение.
|
||||
- AC7 получил прямой регрессионный тест именно на находку r2 (mixed-node fixture `north/south/east + 45°`), а AC6-мутант явно расширен на смешанные узлы — новый контракт не только описан, но и защищён тестом, способным упасть при откате к node-level классификации.
|
||||
- Не-scope (§5) не разошёлся: смешанные узлы добавлены явно во «Входит», ни одна соседняя подсистема не втянута.
|
||||
- Продуктовых вопросов владельцу нет и не должно быть: изменение — техническое уточнение уже принятого продуктового факта, различение per-ray protection не требует решения персоны/объёма видимых изменений.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `npm run typecheck`/`npm test`/`npm run build`/`node scripts/check-docs.mjs` — весь диапазон `origin/dev...HEAD` за issue #275 (r1+r2+r3) состоит из файлов `docs/**`, продуктовый код не менялся; прогон этих гейтов не даёт сигнала о качестве ТЗ. То же основание, что в r1/r2.
|
||||
- Не запрашивал и не пытался воспроизвести приватные `1.json`/`2.json` и числа «110/437», «100/382» из хендоффа — они не коммитятся по условию задачи; приняты на тех же основаниях, на которых процесс уже принимал числовые данные владельца/автора в #272 и предыдущих раундах #275. Если реальное распределение окажется иным, это не меняет того, что выбранный контракт (`effectiveCut` через union, а не через отдельные adjacency-пары) технически необходим уже для одного доказанного сценария (смешанный узел из находки r2), независимо от точных долей в приватных файлах.
|
||||
- Не оценивал производительность или сложность конкретной реализации разности `difference(retainedCuts, protectedUnion)` — §13 п.5 и §7 (perf) корректно оставляют это реализации и защищают prerelease performance gate; не предмет ревью ТЗ.
|
||||
- Не проверял повторно `docs/ARCHITECTURE.md`/`docs/USER-GUIDE.ru.md`/`docs/TESTING.md` — не тронуты дельтой, r1 уже подтвердил, что #275 не меняет их текущий контракт.
|
||||
- Не проверял `docs/reviews/SPEC-REVIEW-275-r{1,2}.md` как предмет этого раунда — использовал их только как источник для «Унаследовано» и «Закрытие r2».
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. High-находок нет, Medium-находок нет. Единственная Medium-находка r2 (защита на уровне узла целиком оставляла ортогональную пару смешанного узла без защиты) закрыта переносом единицы защиты на пару/луч и формулой `effectiveCut = pairwiseCuts − requiredOrthogonalStripUnion(node)`, что независимо пересчитано на эталонном смешанном узле и сверено с реальной структурой `bevelMultiWallBody`/`multiWallBevelCutsAt` (адрес угловых соседей, сортировка по углу), а не принято по одному текстовому заявлению автора. Новый AC7 mixed-node fixture и расширенный AC6-мутант — прямой регрессионный тест именно на находку r2. Одна новая Low-находка (числа хендоффа не повторены в тексте ТЗ) снята без правки документа: она обосновывает уже зафиксированное и проверяемое техническое решение, не создаёт непроверяемости ни одного AC.
|
||||
Reference in New Issue
Block a user