From e45dcad47a891f385d871c76ed315b65202e802a Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 27 Sep 2026 06:41:04 +0000 Subject: [PATCH] docs: review document for #669 Issue: #669 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-669-r2.md | 143 +++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-669-r2.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 7dfd8d8e..323a8f13 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -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` | diff --git a/docs/reviews/SPEC-REVIEW-669-r2.md b/docs/reviews/SPEC-REVIEW-669-r2.md new file mode 100644 index 00000000..02bfc7f2 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-669-r2.md @@ -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 + +--- + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `8b775247a705` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c776611697d24a44fcd357c1aa77058ce08520ee` + ``` + git log --all --format='%H %T' | grep c776611697d2 + ``` +- Тело issue: `1325d6906ba9cf19798f52282b54bf435c4603185b0a323e8a7a5ef01204ae9c` +- Вердикт конвейера: `green` · High 0