docs: review document for #669

Issue: #669
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-27 06:41:04 +00:00
parent 944adc65a9
commit e45dcad47a
2 changed files with 145 additions and 1 deletions
+2 -1
View File
@@ -1,12 +1,13 @@
# Индекс ревью
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1095, issue: 390. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1096, issue: 390. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|---|---|---|---|---:|---:|---|---|
| #672 | [CODE-REVIEW-672-r1.md](CODE-REVIEW-672-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #671 | [CODE-REVIEW-671-r1.md](CODE-REVIEW-671-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #669 | [SPEC-REVIEW-669-r1.md](SPEC-REVIEW-669-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | AC3 не гарантирует заявленную цель issue; фактическая неточность в «Проблема» (не блокирует) | — |
| #669 | [SPEC-REVIEW-669-r2.md](SPEC-REVIEW-669-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
| #667 | [CODE-REVIEW-667-r1.md](CODE-REVIEW-667-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #666 | [CODE-REVIEW-666-r1.md](CODE-REVIEW-666-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #665 | [SPEC-REVIEW-665-r1.md](SPEC-REVIEW-665-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | заявленная правка docs/ISOMETRIC.md | `docs/ISOMETRIC.md` `docs/adr/160-isometric-stage3-overlays.md` `check-docs.mjs` `ISOMETRIC.md` |
+143
View File
@@ -0,0 +1,143 @@
# SPEC-REVIEW-669-r2
- **Issue:** https://github.com/Matysh/houseplan-card/issues/669
- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
- **Трек:** полный (метки `bug`, `P1`, `S4-spec-review`; ни `small`, ни
`trivial` не выставлены — не изменилось с r1).
- **Материал:** тело issue #669, раздел `## ТЗ`, редакция 2 (комментарий автора
«ТЗ, редакция 2 — по ревью r1»). Полный текст получен `gh issue view 669
--json body,comments`.
- **Заход:** r2 · блокирующих циклов израсходовано 1 из 4
- **Роль:** ревьюер ТЗ (не автор)
## Скоуп ревью (без изменений с r1)
Регрессия производительности в `cleanFloorForRoom` (`src/clean-floor.ts:47`):
каждый промах кэша чистого пола сразу вычитает **все** лестницы пространства
(`geometryAreaMinusStairs` → `geometryMinusStairs`, `src/stairs.ts:234-250`) —
хотя площадь читают только два потребителя (подсказка комнаты, PDF), а путь
пола (нужный четырём местам рендера) вычитания не требует. Задача переносит
вычисление площади на момент первого чтения (лениво, мемоизировано в том же
кэше) и добавляет отбор лестниц по пересечению bbox перед polyclip. SCOPE-связь
с J1 не менялась и не пересматривается в r2.
## Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
| **Medium** — AC3/преамбула «Ожидаемое поведение» обещают полностью зелёный `performance_smoke`, но `spaceSwitchMs` (обязательная `hardMaxMs`-метрика того же job) уже проваливается на точном SHA независимо от этой задачи и вынесена в отдельный #675; разрыв между «AC3 выполнен» и «issue решает свою же преамбулу» не был явно назван. | Автор выбрал вариант (a) из ревью: явно ограничил обещание двумя метриками и назвал #675 сопутствующим условием. | Новый раздел «Связь с „Ожидаемым поведением“» в теле issue: «Эта задача обещает бюджеты `modelReadyMs` и `firstStableRenderMs`… Бюджет `spaceSwitchMs` того же job тоже обязателен, но от лестниц не зависит… он закрывается #675. Полностью зелёный perf-smoke кандидата и выпуск v1.78.0-beta.5… — результат этой задачи **вместе с** #675. Ни одна из двух задач поодиночке этого не обещает.» Дополнительно AC3 дописан: «`spaceSwitchMs` этим AC не утверждается — это условие #675; если job красный только по `spaceSwitchMs`, AC3 считается выполненным, а выпуск ждёт #675.» |
| **Low** — «Проблема» утверждала «пять мест рендера запрашивают только путь пола», чтением кода подтверждено четыре; пятое (`:11873`) читает `.area`. | Число и классификация исправлены в тексте. | «Проблема»: «…хотя **четыре** места рендера запрашивают только путь пола (`.path`: `src/houseplan-card.ts:9059`, `9229`, `10459`, `11011`), а площадь читают лишь подсказка комнаты при наведении (`_roomArea`, `:11864`, чтение `.area` на `:11873`) и PDF (`resolveRoomArea`, `:10182`)». |
Обе находки закрыты правкой текста ТЗ (комментарий автора помечен «ТЗ,
редакция 2»), без привлечения владельца — как и предписывал вердикт r1
(технические, не продуктовые вопросы).
## Унаследовано из r1 (без повторной проверки)
Дельта r1→r2 — точечная правка двух фрагментов текста (один новый раздел,
одна цифра с перечислением строк). Она не меняет ни один AC по существу, ни
контракт поведения, ни границы скоупа/не-скоупа, ни план тестов, ни
затронутые файлы, ни риски, ни откат, ни release-артефакты — весь остальной
текст ТЗ идентичен редакции 1. Поэтому без повторной проверки принимается
(документ `docs/reviews/SPEC-REVIEW-669-r1.md`, материал: тело issue,
дерево `2c7a4c124868cf7499d7b5c725ee028d3810eb62`):
- Обязательные разделы §7.1 присутствуют и в правильном порядке; первые два —
продуктовые, без терминов реализации.
- AC1, AC2, AC4 однозначны, с явной парой «доказательство / чем краснеет»;
тестовый план реализуем без изменения контракта пути/геометрии.
- Контракт поведения («в пределах относительной погрешности 1e-9») и «Не-скоуп»
(`geometryMinusStairsSteps` не заденется, 2.5D-рендер — только замер с явным
условием возврата в ТЗ) корректны.
- Риск «устаревшая площадь при копировании объекта» описан реалистично и не
имеет текущего нарушителя (`grep` на spread/`Object.assign` результата
`_cleanFloor`/`cleanFloorForRoom` пуст на момент r1).
- Release-артефакты («changelog — нет») согласованы с `docs/STATUS.md`:
`v1.78.0` ещё не выходил стабильным релизом.
- Продуктовых открытых вопросов к владельцу нет — видимое поведение не
меняется, оставшийся разрыв в AC3 был техническим и закрыт в этом раунде.
## Как проверялось в r2
1. Получено тело issue #669 текущей редакции и все 5 комментариев (включая
комментарий «ТЗ, редакция 2», которого не было в материале r1) —
`gh issue view 669 --json body,comments`.
2. Сверено дословно, что оба пункта, заявленные автором как правка
(«Medium: добавлен раздел…», «Low: в „Проблеме“ — четыре места…»),
присутствуют в теле issue и закрывают ровно то, что просил r1 (таблица
выше) — не переформулировка в духе находки, а её содержательное закрытие.
3. Перечитан текст AC3 целиком в новой редакции: проверено, что критерий
остаётся однозначным и проверяемым — судья (код-ревьюер/разработчик)
получает явное правило «job красный только по `spaceSwitchMs` → AC3
выполнен», а не молчаливое допущение.
4. Проверена согласованность нового раздела с «Не-скоуп» (bullet
«`spaceSwitchMs` — #675», не менялся с r1) и с «Влиянием» — противоречий
нет.
5. Перечитаны строки кода, на которые ссылается исправленная «Проблема»,
заново (не полагаясь на вывод r1):
- `grep -n "_cleanFloor(" src/houseplan-card.ts` → `:9059, :9229, :10459,
:11011` читают только `.path` (подтверждено чтением каждой строки: во
всех четырёх результат употребляется как `.path` без чтения `.area`);
`:11873` (`_roomArea`) присваивает весь объект в `clean` и ниже читает
`clean.area` — корректно отнесён к читателям площади, как теперь и
написано в ТЗ.
- Существование и формулировка #675 (`gh issue view 675`) подтверждают
контекст нового раздела: заголовок «`spaceSwitchMs` изометрического
профиля стоит на границе бюджета без регрессии кода» — согласуется с
утверждением ТЗ «от лестниц не зависит» и «не устраняется этой задачей».
Существо #675 не проверялось (отдельная задача, вне скоупа этого
ревью).
6. Проверено, что остальной текст ТЗ (AC1/AC2/AC4, Скоуп, Контракт поведения,
План автотестов, Риски, Откат, Release-артефакты, Затронутые файлы,
«Принято предположительно») идентичен цитатам, зафиксированным в
`SPEC-REVIEW-669-r1.md`, — расхождений не найдено, отдельная повторная
критика не требуется (раздел «Унаследовано» выше).
## Находки
Нет. Обе находки r1 закрыты текстом; новых неоднозначностей или новых
незакрытых обещаний правка не вносит.
## Что проверено и корректно
- Оба замечания r1 (Medium и Low) закрыты именно так, как просил вердикт r1:
Medium — вариантом (a) («ограничить обещание, назвать #675 сопутствующим
условием»), без обращения к владельцу (техническая правка, решена автором
самостоятельно, как и предписывало «ты его снимаешь и решаешь по существу»);
Low — точным числом и корректной классификацией `:11873`.
- AC3 в новой редакции остаётся однозначным и проверяемым: явно исключает
`spaceSwitchMs` из своей области и явно называет условие полного релиза.
- Новый раздел «Связь с „Ожидаемым поведением“» не расширяет и не сужает
скоуп задачи (AC1/AC2/Скоуп/Не-скоуп не тронуты) — это уточнение границы
обещания, а не новая работа.
- Тело issue снова содержит все обязательные разделы §7.1 в порядке, продукт
и код в описании проблемы согласованы построчно.
## Чего не проверял
- Не проверял #675 по существу — использовал только факт его существования,
открытого статуса и заголовка для согласованности ссылки (как и в r1).
- Не запускал `npm test`/`typecheck`/`build`/perf-смок — реализация не
начата (`git log --all --oneline | grep -i 669` не показывает ветки
реализации), AC1–AC4 по-прежнему доказываются на этапе код-ревью, не здесь.
- Не проверял заново части ТЗ, не задетые дельтой r1→r2 (см. «Унаследовано»).
## Вердикт
Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0
---
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `8b775247a705` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `c776611697d24a44fcd357c1aa77058ce08520ee`
```
git log --all --format='%H %T' | grep c776611697d2
```
- Тело issue: `1325d6906ba9cf19798f52282b54bf435c4603185b0a323e8a7a5ef01204ae9c`
- Вердикт конвейера: `green` · High 0