docs: review document for #275

Issue: #275
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-23 22:02:29 +00:00
parent 762e9f4b32
commit e04e168ac4
+191
View File
@@ -0,0 +1,191 @@
# SPEC-REVIEW-275-r1
- 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`, коммит `762e9f4b323d9d00ae4b230e419b281cd3d82a9a` (ветка `issue/275-multiwall-strip-containment`)
- Заход: r1 · блокирующих циклов израсходовано 0 из 4 (первый заход — полный разбор по PROCESS.md §2.9)
- Ревьюер: свежая сессия, без устных пояснений автора
## Скоуп ревью
Не «согласиться», а найти, где ТЗ невыполнимо или непроверяемо. Проверено:
соответствие `docs/SCOPE.md`, обязательные разделы §7.1 PROCESS.md,
однозначность и доказательность каждого AC, отсутствие догадок, выданных за
факт, и совпадение технических утверждений о причине дефекта с реальным
`src/wall-thickness.ts`. Диапазон изменений `git diff origin/dev...HEAD`
содержит один файл: `docs/specs/275-multiwall-strip-containment.md` (364
строки, только добавление) — продуктовый код не тронут, что и ожидается на
этапе ревью ТЗ.
## Как проверялось
- Прочитаны `docs/SCOPE.md` целиком (J1/J6, персоны, lock-инвариант,
«никогда не удалять файл на догадке») и `AGENTS.md` (классы изменений,
трейлеры, окружения, лимит циклов, шаблон вердикта).
- Прочитан `PROCESS.md` §§1–14 целиком, включая §2.9 (объём повторного раунда
— не применим к r1) и §7.1/§7.2 (обязательные разделы ТЗ, формат вердикта).
- Прочитаны тело issue #275 (полный технический репорт с SHA-256 входов,
точными координатами узлов и числами potерянных sample) и комментарий
«ТЗ готово к ревью».
- Прочитан канонический `docs/WALL-THICKNESS.md` целиком и построчно сверен
с разделом 6 ТЗ (contract `R = 1.25 × H`, `MITRE_LIMIT = 4`,
`ray.support`/`halfDepth`, failure-isolation #197, opening association).
- Прочитан `src/wall-thickness.ts`:
- `MITRE_LIMIT = 4` (L46), `MULTI_WALL_JOIN_LIMIT = 1.25` (L49) — совпадают
с §6 ТЗ;
- `multiWallBevelCutsAt` (L2016–2095) — подтверждён механизм exterior-connector
из #272 (`connectToExterior`), который и создаёт «открытую белую выемку»,
описанную в разделе 3 ТЗ;
- `bevelMultiWallBody` (L2125–2219) — подтверждена буквально: `local` строится
как union прямоугольников `ray.supports` (L2153–2174), затем
`local = difference(local, retainedCuts)` (L2183) — ровно операция, которую
ТЗ называет источником дефекта («восстанавливает union… и снова вычитает
pairwise cut уже из этого физического union»);
- геометрия `sqrt(hA²+hB²)` для прямого угла: для равных `H`
`√2·H ≈ 1.414·H > 1.25·H = R` — арифметика ТЗ (раздел 3) верна.
- `grep` по `test/wall-thickness.test.mjs`, `test/golden-matrix.test.mjs`,
`demo/golden/harness.mjs`, `demo/golden/matrix.mjs` — подтверждено:
`enclosedHoles === 0` действительно единственный golden-инвариант (L399,
416–418 `golden-matrix.test.mjs`; L182, 567, 620 `harness.mjs`),
`retainedWedgeProbe`/`discardedWedgeProbe` из #249/#261 существуют
(L1549–1584 `wall-thickness.test.mjs`) — заявление ТЗ «hole inventory #272
остаётся дополнительной, не достаточной проверкой» и характеристика тестов
#272 в разделе 3 ТЗ точны, не выдуманы.
- Подтверждено существование всех инструментов, которые ТЗ называет в
разделах 8–9: `scripts/mutation-gate.mjs`, `demo/smoke_multiwall_junction.mjs`,
`scripts/smoke-select.mjs`, `demo/golden/harness.mjs`, `demo/golden/matrix.mjs`
— ни один не является изобретённым для этой задачи инструментом.
- Сверены статусы связанных issue: #271/#272/#273 — `CLOSED` (`bug`, `P1`),
#270 — `OPEN` (`tests`/`infra`/`tech-debt`, без статусного `S*`, т.е. вне
процесса разработки). ТЗ ссылается на них только в списке «Связанные
задачи» и не искажает их состояние.
- Сравнена структура ТЗ (13 разделов, нумерация, порядок) с ранее принятым
ТЗ той же подсистемы `docs/specs/272-no-multiwall-holes.md` — идентичный
каркас, включая обязательную строку «**Доказательство:**» на **каждом**
AC1–AC8 без исключения. Предыдущий раунд той же подсистемы
(`SPEC-REVIEW-272-r1`, жёлтый) поймал ровно два дефекта: (а) отсутствие
строки «Доказательство:» на 6 из 8 AC и (б) неопределённый термин
«объявленный exterior sector» в контракте. Проверено прицельно: в #275 оба
класса отсутствуют — AC1–AC8 несут явную строку доказательства, а
разграничение «что можно резать» дано формулой
`effectiveCut = pairwiseCut − requiredStripUnion(node)` (раздел 6.3), а не
словесно неопределённым понятием.
- Гейты этого этапа не прогонялись: код не менялся (диапазон — один
документ), прогон `typecheck`/`test`/`build`/`check-docs` на ревью ТЗ
бессмыслен и не даёт сигнала.
## Находки
Ни одной High и ни одной Medium-находки, блокирующей задачу.
### Low — 1: делегирование доказательства AC3 существующей конвенции хендоффа, не тексту ТЗ
`docs/specs/275-multiwall-strip-containment.md`, AC3. Доказательство описано
как «локальный отчёт с SHA-256 входов, machine-readable inventory и
PNG-crops», но раздел не называет явно канал передачи этого отчёта ревьюеру
кода — ревьюер (свежая сессия в CI) не имеет доступа ни к `C:\Temp\4\*.json`,
ни к локальной машине автора. Формально это тот же класс вопроса, который
ревью ТЗ обязано ловить («где ТЗ непроверяемо»), но по существу канал уже
задан на уровне процесса, а не этой задачи: PROCESS.md §7.2 требует в
хендофф-комментарии строку «Гейты: `<команда → результат>`» для каждого
прогнанного гейта, и тот же паттерн (прогон на приватных/локальных данных,
отчёт — командой и результатом в issue) уже используется для WSL/Windows
Playwright-прогонов (AGENTS.md, «Running the app / smoke suite»,
«`Verified` without a named command… is not evidence»). Отдельного
повторения этого правила в тексте ТЗ не требуется — снимаю без правки
документа, реализация обязана положить хеш/инвентарь/crops AC3 в
хендофф-комментарий по уже действующему шаблону §7.2, и это же придётся
неизбежно проверить на этапе код-ревью.
### Low — 2: план автотестов не выделен отдельным заголовком
Формально §7.1 PROCESS.md перечисляет «план автотестов» отдельным пунктом
обязательных разделов, а в ТЗ #275 он не оформлен отдельным заголовком —
содержание распределено между §8 (AC + «Доказательство:» на каждый) и §9
(«Ожидаемые файлы» с точным списком test/fixture/smoke/golden/mutation
файлов). Содержательно это ровно план автотестов, и точно тот же способ
подачи использован в принятом ранее ТЗ той же подсистемы
(`docs/specs/272-no-multiwall-holes.md`, разделы 8–9) без замечаний по этому
пункту в его собственном ревью. Снимаю без правки: расхождение чисто
формальное, не создаёт непроверяемости ни одного AC.
## Что проверено и корректно
- **Полнота обязательных разделов** (§7.1 PROCESS.md): сценарий/персона
(§1), «что человек увидит до/после» (§4, и кратко в конце §1), проблема
(§2–3), scope/не-scope (§5), геометрический контракт (§6),
compatibility/UX/i18n/touch/security/performance (§7 — все явно закрыты
словом «не меняется»/«нет»), AC1–AC8 с «Доказательство:» на каждом (§8),
ожидаемые файлы = план тестов (§9), release-артефакты (§10), риски (§11),
откат (§12), явный блок принятых технических предположений (§13).
- **Технические утверждения о коде не догадка.** Причина дефекта (раздел 3)
и геометрический контракт (раздел 6) построчно сверены с
`src/wall-thickness.ts:46,49,2016–2219` и с `docs/WALL-THICKNESS.md` —
совпадают буквально, включая формулу `sqrt(hA²+hB²)` и её сравнение с
`R = 1.25H`.
- **Утверждения об устаревшем покрытии тестов #272 точны**, а не преуменьшены
или преувеличены: `enclosedHoles === 0` — реальный и единственный
golden-инвариант, подтверждено grep'ом по `golden-matrix.test.mjs` и
`demo/golden/harness.mjs`.
- **Числовые данные не искажены при переносе из issue в ТЗ**: координаты
проблемных узлов, `cell_cm`, число rooms/walls/degree-3+ nodes и число
потерянных samples в разделе 2 ТЗ совпадают посимвольно с телом issue #275.
- **Не-scope сдерживает расползание**: явно исключены изменение persisted
данных, новая эвристика Optimize, отмена finite-ray endpoints #271,
изменение `R`/коэффициента, новый UI/i18n/backend/миграция — ни одна
соседняя подсистема не втянута в задачу.
- **Продуктовых вопросов владельцу нет и не должно быть.** Единственный
продуктовый факт («сохранённая положительная полоса стены не может быть
вырезана renderer-bevel») уже зафиксирован самим владельцем в тексте issue;
все оставшиеся пункты §13 — технические предположения, оспоримые
ревьюером, а не владельцем, что и есть верное место для них по PROCESS.md
§7.1.
- **Приватность данных не нарушена ни на этапе ТЗ, ни в описанном плане**:
ТЗ явно требует anonymized/minimized fixtures в Git (AC1) и запрещает
коммит полных backups/layout/имён комнат (AC7, раздел 2); формулировка
сохраняет прецедент, уже использованный для fixture #249
(`test/fixtures/249-multiwall-junction.json`).
- **AC6 (mutation) ссылается на реальный существующий инструмент**
`scripts/mutation-gate.mjs`, а не изобретённый для задачи механизм;
негативный мутант («безусловное вычитание pairwise cut из ray-strip union»)
сформулирован так, что обязан уронить именно AC1/AC5, а не только
косвенный признак.
- **Структура ТЗ идентична ранее принятой конвенции той же подсистемы**
(`docs/specs/272-no-multiwall-holes.md`) и не повторяет два конкретных
дефекта, пойманных прошлым раундом той же линии задач (`SPEC-REVIEW-272-r1`:
отсутствие строки «Доказательство:», неопределённый термин в контракте).
## Чего не проверял
- Не запускал `npm run typecheck`/`npm test`/`npm run build`/
`node scripts/check-docs.mjs` — на этапе ревью ТЗ продуктовый код не
менялся (диапазон `origin/dev...HEAD` — один файл спецификации), прогон
этих гейтов не даёт сигнала о качестве ТЗ.
- Не пытался воспроизвести или запросить приватные `1.json`/`2.json` — они
не коммитятся по условию задачи и по правилу `docs/SCOPE.md` (пользовательские
данные не публикуются); принял точные числа/координаты, приведённые
владельцем в теле issue, на тех же основаниях, на которых процесс уже
принимал их в ревью #272 (SPEC-REVIEW-272-r1, «Чего не проверял»).
- Не оценивал сложность самого geometric boolean алгоритма, который
напишет исполнитель, — ТЗ прямо и правильно оставляет конкретную реализацию
техническим решением (§13.2, «эквивалентный алгоритм допустим»), и это его
законное место, а не работа ревью ТЗ.
- Не проверял `docs/ARCHITECTURE.md`/`docs/USER-GUIDE.ru.md`/`docs/TESTING.md`
целиком — только подтвердил, что они существуют и что #275 не меняет их
текущий контракт, лишь дополнит тестовые/пользовательские артефакты, которые
эти файлы описывают на верхнем уровне.
- Не оценивал производительность предлагаемого алгоритма эмпирически —
раздел 7 ТЗ корректно называет риск и меру («cached local node work и
prerelease performance gate»), числовой бюджет уже существует вне этой
задачи и не является предметом ревью ТЗ.
## Вердикт
Зелёный. Обязательные разделы §7.1 присутствуют, каждый AC1–AC8 однозначен и
несёт явное доказательство, причина дефекта и геометрический контракт
построчно сверены с реальным кодом и каноническим `docs/WALL-THICKNESS.md` и
не являются догадкой, продуктовых вопросов владельцу нет и не должно быть,
scope/не-scope сдерживают задачу от расползания на соседние подсистемы. Обе
находки — Low, сняты решением ревьюера с записью (существующая конвенция
хендоффа §7.2 закрывает канал доказательства AC3; распределение плана
автотестов между §8/§9 — уже принятый прежде формат той же подсистемы).