mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
committed by
Sergey Matyunin
Sergey Matyunin
parent
9f6efd0c11
commit
61b437fd3f
@@ -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` (публикуется шагом конвейера
|
||||||
|
из данного файла).
|
||||||
Reference in New Issue
Block a user