mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
committed by
Sergey Matyunin
parent
dd4ebb7401
commit
f3d4a79787
@@ -0,0 +1,179 @@
|
||||
# SPEC-REVIEW-272-r1
|
||||
|
||||
- Issue: [#272](https://github.com/Matysh/houseplan-card/issues/272) — «Multi-wall стыки beta.5 всё ещё оставляют белые треугольные отверстия вне probe #261»
|
||||
- Этап: ТЗ на ревью (PROCESS.md §2.4)
|
||||
- Заход: r1 · блокирующих циклов израсходовано 0 из 4 (первый заход, полный разбор)
|
||||
- Артефакт ТЗ: `docs/specs/272-no-multiwall-holes.md`, коммит `9e4a9470b63718d75cb6e11b13717ccc24cbe4e6` (ветка `issue/272-no-multiwall-holes`)
|
||||
- Ревьюер: свежая сессия, без устных пояснений автора
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Не «согласиться», а найти, где ТЗ невыполнимо или непроверяемо. Проверено:
|
||||
соответствие `docs/SCOPE.md`, обязательные разделы §7.1 PROCESS.md, однозначность
|
||||
и доказательность каждого AC, отсутствие догадок, выданных за факт, и совпадение
|
||||
технических утверждений о коде с реальным `src/wall-thickness.ts`.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§§1–14 целиком) —
|
||||
подтверждён формат вердикта, лимит циклов, обязательные разделы ТЗ.
|
||||
- Прочитаны тело issue #272 и оба комментария (аналитика владельца, «ТЗ готово
|
||||
к ревью»). Issue не помечен `small` → ТЗ обязано жить в `docs/specs/`, что и
|
||||
сделано.
|
||||
- Прочитан канонический `docs/WALL-THICKNESS.md` целиком и сверен построчно с
|
||||
утверждениями ТЗ о контракте multi-wall bevel (`R = 1.25 × H`, `MITRE_LIMIT`,
|
||||
#249/#261/#197 инварианты).
|
||||
- Прочитаны связанные issue #249, #258, #261 (закрыты), #270, #271 (открыты,
|
||||
`S4-spec-review`, без метки `small`) — сверено, что ТЗ #272 не искажает их
|
||||
историю и корректно описывает состояние #270 (не слито, без семантики
|
||||
`enclosedHoles` в коде).
|
||||
- Прочитан `src/wall-thickness.ts`: константы `MITRE_LIMIT`/`MULTI_WALL_JOIN_LIMIT`
|
||||
(строки 46, 49), `multiWallBevelTrianglesAt`, `bevelMultiWallBody`,
|
||||
`bevelMultiWallPaper` (строки 1969–2130) — построчно сверены с разделом 3 ТЗ
|
||||
(«вычитает full-origin pairwise triangles», «union-ит ray rectangles»,
|
||||
«добавляет tiny core», «клипует к envelope»). Совпадает.
|
||||
- `grep` по `demo/golden/matrix.mjs`, `demo/golden/harness.mjs`,
|
||||
`test/golden-matrix.test.mjs`, `test/wall-thickness.test.mjs` — подтверждено
|
||||
существование `discardedWedgeProbe`, `retainedWedgeProbe`,
|
||||
`multiWallJunction`; подтверждено отсутствие `enclosedHoles` где-либо в
|
||||
дереве (только в тексте issue #270, не в коде) — заявление ТЗ о статусе #270
|
||||
точное.
|
||||
- Сверен формат ТЗ с предыдущим ТЗ того же автора по той же подсистеме
|
||||
(`docs/specs/261-white-wedges-root-cause.md`) как образец принятого прежде
|
||||
документа — раздел «Acceptance criteria и доказательства» там даёт явную
|
||||
строку «**Доказательство:**» на каждый без исключения AC (AC1…AC7).
|
||||
- Гейты этого этапа (ревью ТЗ) не подразумевают прогон кода — код не менялся.
|
||||
`typecheck`/`test`/`build` не прогонялись, так как это ревью документа, а не
|
||||
реализации; продуктовый код изменений не содержит (диапазон `origin/dev..HEAD`
|
||||
— один документ).
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе) — 1: у большинства AC нет явной строки «Доказательство»
|
||||
|
||||
`docs/specs/272-no-multiwall-holes.md`, раздел 8, AC2–AC7. Только AC1 несёт
|
||||
явную строку `**Доказательство:** test/wall-thickness.test.mjs и анонимная
|
||||
fixture.`. AC2 («#249 остаётся ограниченным внешним bevel»), AC3, AC5, AC6, AC7
|
||||
описывают ожидаемое поведение и упоминают артефакты в свободной прозе
|
||||
(`discardedWedgeProbe`, `retainedWedgeProbe`, golden-сцену, `mutation`), но не
|
||||
называют явно способ доказательства в терминах §2.5 PROCESS.md
|
||||
(`unit`/`backend`/`smoke`/`golden`/«ревью кода»). AC4 называет метод
|
||||
(«Targeted production-bundle smoke») в тексте, но тоже без выделенной строки.
|
||||
|
||||
**Почему это находка, а не стиль.** PROCESS.md §2.5 (DoR) требует буквально:
|
||||
«у каждого [AC] указано, чем он доказывается». Собственный прецедент того же
|
||||
автора по той же подсистеме, `docs/specs/261-white-wedges-root-cause.md`
|
||||
(предыдущее принятое ТЗ по multi-wall bevel), даёт явную строку
|
||||
«**Доказательство:**» на каждый без исключения AC — AC1…AC7. #272 отступает от
|
||||
уже установленной и принятой конвенции без объяснения. Без явной строки
|
||||
«Готово к разработке» (§2.5) невозможно механически сверить, что каждый AC
|
||||
имеет названное доказательство — а не восстановить его по смыслу абзаца, что и
|
||||
есть определение непроверяемости, которое ревью ТЗ обязано ловить.
|
||||
|
||||
**Воспроизведение:** сравнить `docs/specs/272-no-multiwall-holes.md:186-245`
|
||||
(AC1–AC7) с `docs/specs/261-white-wedges-root-cause.md:211-291` (AC1–AC7 того
|
||||
же автора, та же подсистема) — расхождение в формате видно построчно.
|
||||
|
||||
**Исправление:** добавить к AC2, AC3, AC4, AC5, AC6, AC7 явную строку
|
||||
«**Доказательство:** …», называющую конкретный тест-файл/гейт (по образцу AC1 и
|
||||
по образцу #261), не переписывая уже верное содержание критериев.
|
||||
|
||||
### Medium (в скоупе) — 2: «объявленный exterior sector» не определён алгоритмически
|
||||
|
||||
`docs/specs/272-no-multiwall-holes.md`, §6.3, пункт 3: «пустые samples,
|
||||
связанные с границей окна **или с объявленным exterior sector**, считаются
|
||||
внешним фоном». Раздел 6.3 — это ядро контракта: он формально отделяет
|
||||
законную внешнюю пустоту (после bevel #249) от настоящей дыры внутри стыка,
|
||||
то есть именно то различие, ошибка в котором была причиной, по которой #261 и
|
||||
старый golden-порог пропустили дефект (см. #270). Первая часть критерия
|
||||
(«связаны с границей окна») — детерминированный flood-fill, вычислимый
|
||||
геометрически и без произвола. Вторая часть, «объявленный exterior sector», не
|
||||
определена: неясно, вычисляется ли она автоматически из угловой геометрии
|
||||
узла (`node.rays`, разрыв между соседними лучами за пределами `R` — то, что
|
||||
уже строит `multiWallBevelTrianglesAt()`), или назначается вручную на fixture,
|
||||
по аналогии с уже существующими вручную объявленными `discardedWedgeProbe` /
|
||||
`retainedWedgeProbe`.
|
||||
|
||||
**Почему это важно именно здесь.** Если «объявленный сектор» — вручную
|
||||
назначаемый параметр теста, то AC1 (автоматический vector inventory по
|
||||
minimized fixtures из настоящих экспортов с 12–14 узлами) либо не масштабируется
|
||||
без ручной разметки каждого узла, либо тест может «объявить» настоящую дыру
|
||||
законным внешним сектором и молча её принять — то есть повторить ровно тот
|
||||
класс слепоты, который эта задача должна закрыть (мера #270: seen-but-under-
|
||||
threshold). Если это вычисляемая величина — соответствующее правило (например,
|
||||
«угловой разрыв между соседними лучами за пределами retained-overlap `R`»)
|
||||
стоит явно назвать в тексте контракта, а не оставлять читателю выбирать между
|
||||
двумя разными по надёжности реализациями.
|
||||
|
||||
**Исправление:** одна фраза в §6.3.3, фиксирующая происхождение «exterior
|
||||
sector» как вычисляемой из геометрии узла величины (или прямая ссылка на то же
|
||||
правило связности, что уже сформулировано в §6.1), либо явное решение
|
||||
исполнителя занести в блок §13 «принято предположительно» с формулировкой,
|
||||
исключающей произвольное ручное объявление на реальных, не golden, fixtures.
|
||||
|
||||
### Low — 1: неполное попадание в Core user jobs (снято, без правки документа)
|
||||
|
||||
Аналитика владельца и §1 ТЗ ссылаются на J1 **и** J6 (`docs/SCOPE.md`). J6
|
||||
описывает эволюцию плана (drag/resize/merge/split, multi-client sync,
|
||||
optimistic locking) — этот баг чисто визуальный и в J6 не попадает
|
||||
содержательно; корректна только ссылка на J1 («живой обзор... сейчас»). Не
|
||||
блокирует и не меняет объём работы ни на строку — оставляю без правки
|
||||
документа с этой записью.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Формат и полнота разделов** (§7.1 PROCESS.md): сценарий/персона, «что
|
||||
человек увидит до/после», проблема, scope/не-scope, контракт поведения,
|
||||
UX/touch/i18n/данные и миграция (все явно «не меняются»), AC1–AC8, риски,
|
||||
откат, release-артефакты — все присутствуют и совпадают по структуре с
|
||||
принятым прежде ТЗ #261 той же подсистемы.
|
||||
- **Технические утверждения о коде не являются догадкой.** Раздел 3 (зона
|
||||
причины: `multiWallBevelTrianglesAt`/`bevelMultiWallBody`/`bevelMultiWallPaper`)
|
||||
и раздел WALL-THICKNESS.md (`R = 1.25 × H`, `MITRE_LIMIT = 4`) построчно
|
||||
сверены с `src/wall-thickness.ts:46,49,1969-2130` — совпадают.
|
||||
- **История связанных issue не искажена.** #249/#258/#261 закрыты и не
|
||||
переоткрываются (#272 — новый issue, верно); #270 корректно описана как не
|
||||
слитая инфраструктурная основа без продуктового решения; #271 — как
|
||||
рекомендованный, но не обязательный порядок мержа, зафиксированный в §13 как
|
||||
явное техническое предположение с указанной причиной.
|
||||
- **Продуктовых вопросов владельцу нет и не должно быть.** Владелец сам
|
||||
подтвердил дефект в комментарии-аналитике («видимые белые треугольники —
|
||||
дефект») и сам написал ТЗ; открытых вопросов, требующих решения «что видит
|
||||
пользователь», в тексте не осталось — §13 корректно фиксирует только
|
||||
технические предположения, подлежащие оспариванию ревьюером, а не владельцем.
|
||||
- **Не-scope согрёт риски**: отказ от `R`/`MITRE_LIMIT`, округление bevel,
|
||||
Optimize-очистка (#273) явно исключены, что не даёт задаче расползтись на
|
||||
соседние подсистемы.
|
||||
- **AC6 (mutation)** ссылается на реальный существующий механизм
|
||||
`scripts/mutation-gate.mjs` — не изобретённый специально для этого ТЗ
|
||||
инструмент.
|
||||
- **Privacy/determinism (AC7)** и запрет на коммит приватных экспортов (§2)
|
||||
соответствуют `docs/SCOPE.md` («never delete/never leak user data» — здесь
|
||||
зеркально «never commit private exports») и практике репозитория.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `npm run typecheck`/`npm test`/`npm run build` — на этапе ревью
|
||||
ТЗ продуктовый код не менялся (диапазон `origin/dev..HEAD` содержит только
|
||||
документ спеки), гонять их бессмысленно.
|
||||
- Не пытался воспроизвести приватные `1.json`/`2.json` — они не коммитятся по
|
||||
условию задачи; принял описанные в issue числа узлов (12/14) и координаты
|
||||
probe на веру автора аналитики, как и разрешает процесс для зафиксированных
|
||||
владельцем измерений.
|
||||
- Не оценивал сложность самого geometric boolean алгоritma, который будет
|
||||
писать разработчик — ТЗ прямо и правильно оставляет конкретный алгоритм
|
||||
реализации техническим решением (§13.2), и это его законное место, а не
|
||||
моя работа на этапе ревью ТЗ.
|
||||
- Не проверял `docs/ARCHITECTURE.md`/`docs/TESTING.md` целиком, только что они
|
||||
существуют — они процитированы ТЗ как канонические документы, но #272 не
|
||||
меняет их контракт, только тестовые артефакты, которые эти файлы описывают
|
||||
на верхнем уровне.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый: AC выполнимы и в основном доказуемы, но раздел 8 нарушает собственную
|
||||
установленную конвенцию доказательности AC (§2.5 PROCESS.md) на 6 из 8
|
||||
критериев, а ключевое различие «внешний фон vs дыра» (§6.3.3) содержит один
|
||||
неопределённый термин ровно в той точке контракта, ради которой заведена вся
|
||||
задача. Оба High-блокеров нет; обе находки — Medium, в скоупе, чинятся
|
||||
правкой текста ТЗ без пересмотра архитектуры решения.
|
||||
Reference in New Issue
Block a user