diff --git a/docs/reviews/SPEC-REVIEW-261-r1.md b/docs/reviews/SPEC-REVIEW-261-r1.md new file mode 100644 index 00000000..0a8c6a4d --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-261-r1.md @@ -0,0 +1,188 @@ +# SPEC-REVIEW-261-r1 + +- Issue: [#261](https://github.com/Matysh/houseplan-card/issues/261) — «Реальная причина белых клиньев в T-стыках (#258) не найдена — диагноз в #258 отозван владельцем» +- Этап: ТЗ на ревью (PROCESS.md §2.4), заход r1, блокирующих циклов израсходовано 0 из 4 +- ТЗ: `docs/specs/261-white-wedges-root-cause.md`, зафиксировано коммитом `96cd2f2b08be9a24b02dc13b9c7999a38ab96eac` +- Ревьюер: свежая сессия, без переписки с автором + +## Скоуп ревью + +Полный разбор — это первый заход по этому issue (§2.10 о разборе по дельте +относится к второму и последующим раундам, здесь неприменимо). Проверялись: +соответствие `docs/SCOPE.md`, обязательные разделы ТЗ по PROCESS.md §7.1, +однозначность и доказуемость каждого AC, отсутствие догадок под видом фактов, +соответствие технических утверждений реальному коду и существующим +тестам/фикстурам. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (полностью, включая + §2.4, §2.7, §2.9/§2.10, §7.1, §7.2, §8, §12). +2. Прочитано тело issue #261 и оба комментария (аналитика владельца с + исполненным расследованием на реальном экспорте, хендофф ТЗ). +3. Прочитан канонический `docs/WALL-THICKNESS.md` целиком. +4. ТЗ `docs/specs/261-white-wedges-root-cause.md` прочитано целиком (409 + строк, все 13 разделов). +5. Технические утверждения ТЗ сверены с текущим кодом `src/wall-thickness.ts`: + - подтверждено существование и сигнатуры `buildMultiWallNodeMap`, + `multiWallBevelTriangles`, `multiWallBevelTrianglesAt`, + `bevelMultiWallBody`, `bevelMultiWallPaper`, констант `MITRE_LIMIT = 4` и + `MULTI_WALL_JOIN_LIMIT = 1.25` (`src/wall-thickness.ts:46,49,1501,1969, + 2018,2024,2108`); + - подтверждено, что `node.limit = MULTI_WALL_JOIN_LIMIT * halfDepth` + (`src/wall-thickness.ts:1602`), то есть заявленный в ТЗ `R = 1.25 × H` + — не выдуманная величина, а существующая константа; + - прочитан код `bevelMultiWallBody`/`bevelMultiWallPaper` + (`src/wall-thickness.ts:2024-2128`) и подтверждён механизм, который ТЗ + описывает как причину дефекта: `boundedCurrent` получает полный + («от истока») cut через `multiWallBevelTrianglesAt(..., false)` без + привязки к `R`, а `localInside`/`preservedExterior` затем зависят от + `centre` (union комнат) — то есть внешняя половина физической полосы за + пределами room-centre действительно может быть потеряна ровно так, как + заявлено в §3 ТЗ. Диагноз воспроизводим по коду, а не декларативен. +6. Проверено существование каждого файла и теста, на который ссылается ТЗ: + `test/fixtures/197-junction-patch.json`, + `demo/smoke_junction_patch_resilience.mjs`, + `demo/smoke_multiwall_junction.mjs`, `demo/golden/matrix.mjs`, + `demo/golden/harness.mjs`, `demo/golden/run.mjs`, + `test/golden-matrix.test.mjs`, `scripts/mutation-gate.mjs` — все + присутствуют. +7. Проверен прецедент для AC5 (семантический golden-пробник): существующий + `discardedWedgeProbe` в сценарии `multiwall-junction-bevel-view-dark` + (`demo/golden/matrix.mjs:270-274`, обработка в + `demo/golden/harness.mjs:153-171,557-561`) — предлагаемый `retainedWedgeProbe` + для `junction-patch-resilience-*-dark` — тот же механизм, применённый к + новому месту; при этом подтверждено, что сегодня у + `junctionPatchResilience`-сценариев (`demo/golden/matrix.mjs:264,267`) такой + проверки ещё нет (`demo/golden/harness.mjs:153` — есть только базовая + обработка, без probe-проверки) — то есть AC5 закрывает реальный, а не + мнимый пробел (ровно тот, что назван в R4). +8. Проверено, что фикстура #197 действительно содержит узел в районе + заявленных координат: `test/fixtures/197-junction-patch.json` содержит + несколько записей стен с координатой `x = 0.8875` (что при + `coordScale = 1000` даёт `887.5` — совпадает с заявленным в ТЗ узлом + `(887.5, 550)`). +9. Проверено существующее покрытие #249 (`test/wall-thickness.test.mjs:758-956` + — фикстура `249-multiwall-junction.json`, table-driven order/direction/scale + independence), на которое ссылается AC2 — покрытие реально существует, ТЗ + не изобретает несуществующий тестовый базис. +10. Сверены обязательные разделы ТЗ по PROCESS.md §7.1 (см. таблицу ниже) и + раздел «Принятые технические предположения» (§13 ТЗ) на предмет догадок, + выданных за факт. + +## Находки + +### Low — «Что человек увидит до и после» смешивает продуктовое и техническое + +`docs/specs/261-white-wedges-root-cause.md`, §4. Правило PROCESS.md §7.1 +требует эту часть «одной фразой, без терминов реализации». Первые два +предложения («До» / первое предложение «После») этому правилу соответствуют +и достаточны как самостоятельное продуктовое описание: белые клинья исчезают, +кладка становится непрерывной. Но следом идут ещё два предложения с +терминами реализации — «mitre-зуб», «#249», точная формула `1.25 × H», +«не округляет стык» — то есть раздел не одна фраза и не свободен от +реализации. + +**Почему не блокирует:** первое предложение полностью самодостаточно и +однозначно передаёт продуктовый контракт; хвост — не догадка и не +противоречие, а дополнительная (верная и нужная) деталь geometric contract, +просто продублированная не в том разделе. Ничьё понимание AC или скоупа от +этого не страдает — §6 отдельно и подробно формулирует тот же геометрический +контракт. + +**Решение ревьюера:** снимается с записью, правки не требуется. Автору стоит +в следующих ТЗ держать implementation-термины только в §6 secure, но это не +дефект данного документа, блокирующий разработку. + +## Что проверено и корректно + +- **Соответствие `docs/SCOPE.md`:** задача восстанавливает J1 (план должен + правдиво показывать физические стены) и попутно обслуживает J6 (одна + каноническая геометрия для всех потребителей); никакого конфликта с + out-of-scope списком нет. Продуктовых вопросов владельцу не требуется — + ожидаемое поведение уже зафиксировано его собственным комментарием и + каноном `WALL-THICKNESS.md` (§6.1 задачи почти буквально повторяет уже + принятый контракт #249 «bounded overlap up to R», а не придумывает новый). +- **Обязательные разделы §7.1** — все присутствуют и содержательны: + + | Раздел §7.1 | Где в ТЗ | + |---|---| + | Сценарий/персона | §1 | + | Что человек увидит до/после | §4 (см. находку Low) | + | Проблема | §2–§3 (воспроизведение + причина) | + | Скоуп и не-скоуп | §5 | + | Контракт поведения | §6 | + | UX | §7 (явное «новых UI-контрактов нет») | + | Модель данных и миграция | §7 («config/layout и model_version не меняются») | + | i18n | §9 («новых i18n keys нет») | + | AC1…AC7 с доказательством | §8 | + | План автотестов | §8 (доказательства) + §9 (файлы) | + | Риски | §11 | + | Откат | §12 | + | Release-артефакты | §10 | + +- **Однозначность и доказуемость AC.** Каждый из AC1–AC7 называет конкретный + механизм доказательства (unit-файл, конкретный smoke, golden-сценарий с + геометрическим probe, review diff, конкретная команда + `model-invariants.mjs`) — нет ни одного AC вида «работает корректно» без + метода проверки. AC3 отдельно требует негативного мутанта («новый mutant + возвращает full-origin bevel cut... AC1 обязан на нём краснеть») — то есть + ТЗ заранее проектирует тест так, чтобы код-ревью могло убедиться, что тест + умеет падать (требование PROCESS.md §2.7/§18), а не полагается на голую + непустоту, как это уже один раз подвело регрессию (R4, честно признан как + риск). +- **Технический диагноз воспроизводим, а не декларативен.** Причинно- + следственная цепочка §3 (полный «от истока» cut в `bevelMultiWallPaper`/ + `bevelMultiWallBody`, затем клип к room-centre union) проверена по + реальному коду (см. «Как проверялось», п.5) и совпадает с тем, что видно в + `src/wall-thickness.ts:2024-2128`. Это не догадка, выданная за решение — это + находка, подтверждённая и автором (исполнением на реальном экспорте + владельца, см. комментарий-аналитику), и ревьюером (чтением кода). +- **Раздел догадок явный.** §13 «Принятые технические предположения» содержит + ровно те решения, которые PROCESS.md §7.1 разрешает автору принимать + самостоятельно (разбиение на helpers, достаточность существующей + анонимизированной фикстуры #197, выбор конкретной пробной точки, допустимое + расхождение paper-площади с beta.3 в пределах уже принятого бevel-бюджета, + статус нулевой общей грани). Ни одно утверждение о наблюдаемом поведении не + найдено вне этого раздела без опоры на существующий канон или на + исполненное автором расследование. +- **Трек и оценка процесса.** Задача корректно оставлена на обычном треке + (не `small`/`trivial`): сложность 7/10, общая физическая геометрия, + множество независимых потребителей (Full/Static/hidden Iso/paper/room + fill/hover/Glow/light barriers) — критерии лёгкого трека (§5 PROCESS.md, + «одна поверхность») здесь явно не выполняются, и автор сам это + проговаривает. +- **Ссылки на существующие тесты и фикстуры не выдуманы.** Каждый файл, + функция и тестовый сценарий, упомянутый в ТЗ, существует в дереве на SHA + ревью (проверено `grep`/`ls`, см. «Как проверялось» пп.6–9); ни один AC не + ссылается на несуществующий гейт или несуществующую фикстуру. +- **Откат, миграция, i18n, touch, performance** — по каждому пункту либо явное + «не меняется/не требуется» с обоснованием, либо конкретное ограничение + (§7: «Node map не строится повторно... запрещён новый полный `O(E²)` + обход»). Ничего не оставлено недосказанным. + +## Чего не проверял + +- **Полный количественный пересчёт геометрии** (площади, координаты + треугольников, вершины) из аналитики владельца — из тела issue взят как + факт, подтверждённый исполнением на реальном экспорте владельца + (не воспроизводился ревьюером с нуля, только структура вычислений в коде + прочитана и совпадает по смыслу с описанным механизмом). +- **Сам процесс `git bisect` до `29904df`** — принят со слов владельца/автора; + сверен только факт, что этот коммит существует и относится к #249 (не + проверял построчный diff этого коммита). +- **Golden/smoke прогон** — на этапе ревью ТЗ код ещё не написан, гейты + реализации (§8 PROCESS.md) неприменимы к этому раунду; они относятся к + код-ревью следующего этапа. +- **Точная формулировка будущего кода** (как именно будет переписан + `bevelMultiWallBody`/`bevelMultiWallPaper`) — ТЗ сознательно не фиксирует + разбиение на helpers (§13.1), это оставлено автору кода и будет предметом + код-ревью, а не ревью ТЗ. + +## Вердикт + +Зелёный. High-находок нет, Medium-находок нет. Единственная находка — Low, +снята решением ревьюера без правки (раздел «Находки» выше). + +Документ: `docs/reviews/SPEC-REVIEW-261-r1.md` (публикуется шагом конвейера +из данного файла).