docs: review document for #739

Issue: #739
User-Visible: no
This commit is contained in:
claude[bot]
2026-10-01 05:08:55 +00:00
parent f3cfb93e94
commit bb25110dc4
2 changed files with 161 additions and 1 deletions
+2 -1
View File
@@ -1,10 +1,11 @@
# Индекс ревью
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 231, issue: 111. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 232, issue: 112. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|---|---|---|---|---:|---:|---|---|
| бета v1.79.0-beta.1 | [SHIP-REVIEW-v1.79.0-beta.1.md](SHIP-REVIEW-v1.79.0-beta.1.md) | пакетное ревью ship · — | ⚪ — | 0 | 0 | — | — |
| #739 | [SPEC-REVIEW-739-r1.md](SPEC-REVIEW-739-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #737 | [SPEC-REVIEW-737-r1.md](SPEC-REVIEW-737-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #732 | [CODE-REVIEW-732-r1.md](CODE-REVIEW-732-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #730 | [CODE-REVIEW-730-r1.md](CODE-REVIEW-730-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | текст сводки «повтор и ребейз не помогут» вводит в заблуждение именно в сценарии, котор… | `scripts/merge-candidate.mjs` |
+159
View File
@@ -0,0 +1,159 @@
# SPEC-REVIEW-739-r1
Issue: #739 · Этап: spec (PROCESS.md §2.4) · Трек: ask · Заход: r1 · блокирующих циклов 0/4
## Материал раунда
- ТЗ: тело issue #739 (раздел `## ТЗ`), снято `gh issue view 739` в момент
ревью; issue открыт, метки `S4-spec-review`, `tech-debt`, `track:ask`.
Комментарий автора: оценка ценность 4/10 · сложность 3/10 · P3.
- Рабочая копия репозитория — `HEAD` на `3b9f25ea` (материал параллельного
code-review этапа, не связан с этой задачей по содержанию); для spec-этапа
код читался как текущее состояние `dev`, в которое #739 будет впадать после
merge `#725` (подтверждено: `235f60e6` — коммит #725 — предок `HEAD`).
## Скоуп
К1: в 2.5D с подложкой (`imagePlan: true`) переключение этажа не ищет заново
цвет бумаги карточки, если идентичность темы (`darkMode`, `default_theme`,
`default_dark_theme`, `theme`) и `_mode` не изменились, и рендерит карточку
один раз вместо двух. `revealActiveTab()` (`header-menu.ts`) и
`SummaryPanelPresentation.updated()` сознательно выведены из скоупа решением
по умолчанию («Решено по умолчанию», п.1) — это техническое решение
(§7.1), не продуктовый вопрос, и ревьюер с ним согласен (см. «Как
проверялось»).
Обслуживает J1 (`docs/SCOPE.md`) — «show the whole home and what's
happening now» в режиме живого обзора по этажам; задача найдена при
реализации #725 (тот же перф-трек), не самостоятельная фича.
## Как проверялось
Ревьюер ≠ автор: issue и ТЗ прочитаны без устных пояснений. Вместо
доверия числам и строкам кода в тексте ТЗ каждое фактическое утверждение
сверено с реальным деревом:
1. **Обязательные разделы §7.1** — все на месте: сценарий, что человек
увидит до/после, проблема, скоуп и не-скоуп, контракт поведения,
UX·данные·i18n·touch, граничные случаи (§2.6), критерии приёмки AC1–AC5
с доказательством, план автотестов, риски, откат, release-артефакты,
плюс оба блока решений (решено по умолчанию / принято предположительно).
2. **Технический корень дефекта** прочитан в коде, не принят на слово:
- `src/iso-first-frame.ts` целиком — `isoPaperContext` (строки 18–28),
`prepare`/`sync`/`commitPaper` (45–86) — ключ `key` действительно
включает `space` первым элементом массива (строка 24), поэтому любое
переключение этажа меняет `key` и форсирует `commitPaper` заново
(строка 56, вызов `resolveThemePaper()`), что ровно описывает
найденный дефект.
- `src/houseplan-card.ts:4004` (`willUpdate` → `prepare(...)`) и `:4046`
(`updated` → `sync(...)` → `requestUpdate()` при `true`) — номера
строк и цепочка вызовов подтверждены построчно.
- `src/header-menu.ts:127,130` (`revealActiveTab`, `nav.clientWidth`) и
`src/summary-panel-presentation.ts:36–37,53` (`updated`, ранний выход
без `pending`, `getComputedStyle`) — номера строк и условия выхода
совпадают с текстом «Не-скоуп» дословно.
- `scripts/render-layout-read.mjs` прочитан целиком: гейт уже проверяет
весь `src/iso-*.ts` на принудительные чтения раскладки и не трогает
`updated()` карточки отдельно — вызов `_cssColor` из `updated()`
(разрешённое место) и предлагаемое кеширование цвета не меняют состав
гейта; фраза ТЗ «расширять на файл гейт нет смысла» для
summary-panel подтверждена чтением условия `pending` в коде.
3. **Существование артефактов, на которые ссылается план автотестов**:
`test/iso-stage6.test.mjs` (включая строку `a theme change hides the
stale classification`, :94 — тест #654, который ТЗ обещает оставить
зелёным), `demo/smoke_iso_first_frame.mjs`,
`demo/smoke_isometric_contract.mjs`, `demo/smoke_iso_flat_parity.mjs`,
`demo/smoke_iso_theme_walls.mjs`, `test/core-file-budget.test.mjs`
(строка бюджета `src/houseplan-card.ts` на месте) — все существуют,
план автотестов не ссылается на несуществующие файлы или функции.
Демо-дом: `demo/srv/demo.html:54,62` действительно задаёт `plan_url` для
`f1` и `garden` — подложки у обоих пространств, как заявлено.
4. **Конвенция `// private-ok: <причина>`**, которую AC2 использует для
счётчиков вызовов в smoke, существует и работает так, как описано
(`scripts/no-new-private-writes.mjs`, `docs/TESTING.md:249`) — формат
`// private-ok: счётчик проходов и поиска бумаги (#739)` пройдёт гейт.
5. **База измерений** — ветка `issue/725-layout-reads-and-fingerprint`
(коммит `071c74a5`, уже не резолвится — ожидаемо: #725 слит
squash-коммитом `235f60e6` в `dev`). Это не находка (§2.10 про
нерезолвящийся SHA относится к материалу предыдущего раунда ревью, но
тот же принцип применИм: дерево и блоб важнее хеша ветки, которая была
удалена после слияния); все утверждаемые номера строк и поведение
сверены с текущим `dev`-деревом и совпадают.
6. Трек подтверждён: `track:ask`, перф-класс (§5), файлы класса A
(`src/houseplan-card.ts`, `src/iso-first-frame.ts`) — ревью ТЗ для
`ask` обязательно (§2.4), что и происходит.
Гейты (typecheck/test/build) не прогонялись — это ожидаемо на этапе spec:
зависимости и Chromium не ставились (#696), а артефакт этапа — ТЗ, не код.
Прогон гейтов на будущей ветке — обязанность code-review.
## Находки
Нет. High — 0, Medium — 0, Low — 0.
Технических вопросов, ошибочно вынесенных владельцу, не найдено: оба блока
(«Решено по умолчанию», «Принято предположительно») корректно помечены как
не-продуктовые решения реализатора/ревьюера и явно допускают пересмотр без
эскалации — ровно то, что требует §7.1.
## Что проверено и корректно
- Все обязательные разделы §7.1 присутствуют и содержательны, не
шаблонные заглушки.
- Каждый AC (AC1–AC5) однозначен, имеет названный способ доказательства
(`unit`/`smoke`/`gate`/performance-workflow) и описывает проверяемое
состояние или счётчик, а не «работает корректно» общими словами.
- Защитный AC5 (гейт/мутант) содержит строку «чем краснеет» — мутант
`iso-paper-resolved-per-floor` с guard-тестом `test/iso-stage6.test.mjs`
(AC1), раздел «План автотестов» дополнительно называет, что именно
краснеет на текущем коде для AC1 и AC2 до фикса.
- Контракт поведения (К1) и граничные случаи (§2.6: async, данные, хост,
режимы, объём) покрывают переходы между темами, режимами, наличием/
отсутствием подложки и сменой режима редактор↔View.
- Оценка производительности честно помечена как «оценка, а не обещание»
там, где измерение бенчмарком невозможно (CI-профили без подложки) —
это не нарушение §7.1 («догадка как факт»), так как оговорка на месте.
- Откат, release-артефакты (`User-Visible: no`, абзац `docs/ISOMETRIC.md`),
i18n/миграции («нет») и touch (киоск — блокирующая поверхность, вход не
меняется) закрыты явно.
- Фактические ссылки на код (номера строк, имена функций, поведение
гейта `render-layout-read.mjs`) проверены чтением и совпадают дословно —
редкий случай, когда ревью не нашло ни одного расхождения между ТЗ и
деревом.
## Чего не проверял
- Гейты `npx tsc --noEmit`, `npm test`, `npm run build` — не прогонялись:
не требуется на этапе spec (#696), код задачи ещё не написан.
- Сами измерения из раздела «Проблема»/«Не-скоуп» (CPU-профили, числа
`switchCycleMs`, `longTasks`) не переснимались — они сделаны временным
флагом харнесса вне репозитория и не воспроизводимы на этом материале;
проверена только непротиворечивость чисел с кодом и пометка «оценка, не
обещание» там, где это уместно.
- Мутация `iso-paper-resolved-per-floor` не существует в
`scripts/mutation-registry.mjs` на момент ревью — это ожидаемо: ТЗ
описывает её как часть будущей реализации (AC5), а не как уже
существующий гейт.
- Браузерные смоки/Full Performance не запускались — на этом треке и
этапе не требуются (нет кода, нет ветки).
## Вердикт
Зелёный. ТЗ выполнимо, каждый AC проверяем и привязан к существующим
файлам/инструментам, продуктовых вопросов не осталось открытыми, скоуп и
не-скоуп согласованы и обоснованы профилированием. Переход в «Готово к
разработке».
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `3b9f25ea659c` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `e94ca566360e026e50ecff7594279f9db87cfbbc`
```
git log --all --format='%H %T' | grep e94ca566360e
```
- Тело issue: `b4865b454fae1e35d41752478fda842157505fded49f39f20bbaff6fa2794b37`
- Вердикт конвейера: `green` · High 0 · маршрут `fix`