mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 12:49:56 +00:00
@@ -1,10 +1,11 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 232, issue: 112. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 233, issue: 113. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). 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 | — | — |
|
||||
| #740 | [SPEC-REVIEW-740-r1.md](SPEC-REVIEW-740-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | устаревший номер строки в «Проблема», п.2 | `src/stairs-view.ts` `src/stairs-editor.ts` `stairs-view.ts` `stairs-editor.ts` `stairs.ts` `large-house.mjs` `matrix.mjs` |
|
||||
| #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 | — | — |
|
||||
|
||||
@@ -0,0 +1,212 @@
|
||||
# SPEC-REVIEW-740-r1
|
||||
|
||||
**Issue:** #740 «Производительность: слой лестниц — ≈50 мс на переключение на
|
||||
этаж 1»
|
||||
**Этап:** spec (PROCESS.md §2.4) · **Заход:** r1 · блокирующих циклов 0/4
|
||||
**Трек:** `ask` (критерий §5 «перф»: меняется цена переключения этажа, файлы
|
||||
класса A; дополнительно «геометрия» — меняется разметка символа лестницы)
|
||||
**Материал:** тело issue #740, раздел `## ТЗ` (снимок на момент ревью, issue
|
||||
открыт, лейблы `S4-spec-review`, `tech-debt`, `track:ask`; один комментарий —
|
||||
«Оценка», трек подтверждён аналитиком)
|
||||
**Проверка кода велась против рабочей копии на** `b27ae06ef81538db6f4fcd183036918088cbca85`
|
||||
(`HEAD`, совпадает с `origin/dev` на момент ревью; ветки `issue/740-*` нет —
|
||||
кода по задаче ещё не существует) — не как материал ревью (ТЗ ещё не код), а
|
||||
чтобы проверить, что номера строк, имена функций и константы в ТЗ отражают
|
||||
текущий код, а не домысел.
|
||||
|
||||
## Скоуп
|
||||
|
||||
К1 — ступени одной лестницы (`src/stairs-view.ts`, `src/stairs-editor.ts`)
|
||||
переходят с N отдельных `<line class="hp-stair-tread">` на один
|
||||
`<path class="hp-stair-tread">` со строкой `d`, построенной один раз на объект
|
||||
геометрии из `cachedStairRenderGeometry`, а не на каждом рендере. Контур,
|
||||
трапеция, стрелка, `g.hp-stair` и его атрибуты/обработчики не меняются.
|
||||
Эффект доказывается Full Performance (`switchCycleMs` фикстуры `large-house`,
|
||||
`isometric`). Обслуживает J1 `docs/SCOPE.md` («переключение этажей —
|
||||
частое действие», сценарий явно называет домочадцев на киоске/телефоне и
|
||||
администратора). `User-Visible: no` — заявлена пиксельная идентичность кадра,
|
||||
что проверяется тем же AC3, которое доказывает отсутствие видимого изменения.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью ТЗ на этапе spec не гоняет тестовые гейты — кода ещё нет (ветки `issue/
|
||||
740-*` не существует, подтверждено `git branch -a` и `git log --all --oneline`
|
||||
по номеру задачи). Вместо этого каждое фактическое утверждение ТЗ (номер
|
||||
строки, имя константы/функции, число элементов, существующая golden-сцена)
|
||||
сверено с текущим деревом на `b27ae06e`, поскольку «догадка, записанная как
|
||||
факт» — находка (§7.1).
|
||||
|
||||
Прочитано и сверено построчно:
|
||||
- `src/stairs-view.ts` — `renderLayer()` (:63, ТЗ указывает то же), строка
|
||||
контура `outline.map(...).join(' ')` (:71, совпадает), блок ступеней
|
||||
`geometry.treads.map(...)` (:115, совпадает — это и есть точка контракта К1),
|
||||
геттер `stairs` → `stairList(...)` (фактически :35, ТЗ указывает :34 — см.
|
||||
находку ниже).
|
||||
- `src/stairs-editor.ts` — `renderStair()` → `geometry.treads.map(...)` (:535,
|
||||
совпадает — вторая точка контракта К1).
|
||||
- `src/stairs.ts` — `cachedStairRenderGeometry` (:320, совпадает),
|
||||
`STAIR_STROKE_CM = 3.6` (:8, совпадает с «одна физическая толщина 3.6 см»),
|
||||
`MAX_STAIRS_PER_SPACE = 250` (:9, совпадает с «предел на этаж» и фикстурой),
|
||||
`stairIntervalCount` (:170) и внутренний радиус ступеней винтовой лестницы
|
||||
`stair.radius * scale * 0.18` (:298, совпадает с «начинаются на 0.18
|
||||
радиуса»), `isStair` (:180) — подтверждает, что валидация принимает любой
|
||||
`radius > 0`, без нижней границы 30 см: претензия риска №2 («импорт с
|
||||
радиусом меньше 30 см» как уже существующее, не вносимое этой задачей
|
||||
состояние) подтверждена чтением, а не аннотацией.
|
||||
- `demo/fixtures/large-house.mjs` — `STAIR_COUNT = 250` (:15), применение на
|
||||
этаже 1 (:250) — совпадает с фикстурой бенчмарка, описанной в «Сценарии».
|
||||
- `demo/golden/matrix.mjs` — все четыре названные в AC3 сцены существуют
|
||||
(`stairs-flat-normal-light`, `stairs-flat-hover-dark`,
|
||||
`stairs-flat-selected-light`, `stairs-isometric-dark`, :273–283) и
|
||||
используют `stairLayerFixture`, в котором у `golden-stair-straight-small`
|
||||
уже задано `opacity: 0.85` — то самое сочетание «частичная прозрачность +
|
||||
прямая лестница», от которого риск №1 (расхождение сглаживания `path` и
|
||||
`line`) зависит сильнее всего. Заявление «новых сцен не нужно» подтверждено.
|
||||
- `demo/smoke_stairs.mjs`, `test/stairs.test.mjs`, `scripts/mutation-registry.mjs`
|
||||
— все три файла существуют; в реестре мутантов уже есть прецеденты вида
|
||||
`stairs-view-pan-opens-target-floor` (гард `node demo/smoke_stairs.mjs`,
|
||||
файл `src/stairs-view.ts`), то есть предложенный в АС5 формат нового мутанта
|
||||
`stairs-view-tread-lines` с тем же гардом — не изобретение, а повторение
|
||||
устоявшегося в этом же реестре паттерна «один файл — один мутант», даже
|
||||
когда смок проверяет разметку сразу двух рантаймов (View и редактор).
|
||||
- PROCESS.md §5 — «перф» и «геометрия» в списке критериев `ask`
|
||||
(«геометрия, миграции конфига, публичные контракты, перф и touch, новый
|
||||
UX-контракт») — трек соответствует заявленному обоснованию в шапке ТЗ.
|
||||
|
||||
## Находки
|
||||
|
||||
### Low — устаревший номер строки в «Проблема», п.2
|
||||
|
||||
ТЗ ссылается на «повторную проверку сырого конфига `stairList(raw)` (:34)» в
|
||||
`src/stairs-view.ts`. Строка :34 сейчас — это объявление
|
||||
`private get stairs(): Stair[] {`, сам вызов `stairList(...)` — строкой ниже,
|
||||
:35. Разница в одну строку, не найдено её источника (рабочая копия и issue
|
||||
сверены на одном и том же `b27ae06e`, `dev` за время ревью не двигался) —
|
||||
похоже на опечатку при подсчёте, а не на дрейф от более нового коммита.
|
||||
|
||||
**Почему Low, а не Medium:** ссылка — часть описательной прозы профилирования
|
||||
(«что происходит при заходе на этаж 1»), не часть контракта К1 или оракула
|
||||
AC1/AC2. Оба контрактных указателя — `src/stairs-view.ts:115` и
|
||||
`src/stairs-editor.ts:535` — точны и проверены отдельно. Реализатора эта
|
||||
строка не введёт в заблуждение: соседний код однозначно идентифицируется по
|
||||
имени функции (`stairList`) и его единственному вызову в файле. Снимаю находку
|
||||
сам, без возврата автору.
|
||||
|
||||
Других расхождений факт/код не найдено: полная выборка из ~12 процитированных
|
||||
номеров строк, имён функций и констант по пяти файлам (`stairs-view.ts`,
|
||||
`stairs-editor.ts`, `stairs.ts`, `large-house.mjs`, `matrix.mjs`) совпала,
|
||||
включая менее очевидные ссылки (`:320` на `cachedStairRenderGeometry`, `:9` на
|
||||
`MAX_STAIRS_PER_SPACE`, значение 0.18 для внутреннего радиуса ступеней
|
||||
винтовой лестницы).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Обязательные разделы §7.1 — все на месте:** сценарий, что человек увидит
|
||||
до/после, проблема (с цифрами и профилем), скоуп/не-скоуп, контракт
|
||||
поведения (К1), UX·данные·i18n·touch, граничные случаи (§2.6: данные, async,
|
||||
редактор, объём, визуал — пять из шести явно пройдены, host/input неприменим
|
||||
и это корректно для чисто рендер-перф задачи без ввода), AC1–AC5 с
|
||||
доказательством и oracle, план автотестов (включая «чем краснеет» по
|
||||
AC1–AC3), затронутые файлы, производительность и бюджеты, риски, откат,
|
||||
release-артефакты.
|
||||
- **Каждый AC однозначен и имеет названный способ доказательства** (unit /
|
||||
smoke / golden / Full Performance / gate), с конкретными проверяемыми
|
||||
условиями, а не «примерно»: AC1 — точная формула сборки `d` и инвариант «не
|
||||
пересобирается повторно» с явным механизмом проверки (счётчик или `===`);
|
||||
AC2 — точные числа элементов по кind и по поверхности (View/редактор); AC3 —
|
||||
четыре названные golden-сцены без обновления baseline; AC4 — сравнение
|
||||
медиан по конкретным профилям с прописанным планом на случай
|
||||
недостижения эффекта («разбор `longTasks.switchCycle` по индексам 0,3,6,9,
|
||||
без него AC не выполнен» — явный «красный» случай для количественного AC);
|
||||
AC5 — конкретный мутант с именем и гардом.
|
||||
- **Защитные AC доказаны**: АС1/АС2 имеют заполненную строку «чем краснеет»
|
||||
(«помощника нет на текущем коде» / «3–7 `line.hp-stair-tread` на текущем
|
||||
коде»), АС5 называет конкретный мутант в реестре с гардом, который реально
|
||||
красит АС2. Пустых столбцов нет.
|
||||
- **Пиксельная идентичность (AC3) разобрана предметно**, а не декларативно:
|
||||
спецификация выводит числовую границу непересечения ступеней (винтовая —
|
||||
минимум 6.77 см при r=32 см против толщины линии 3.6 см) и называет
|
||||
единственный класс визуального риска (сглаживание `path` vs `line` при
|
||||
сильном отдалении), покрытый существующими сценами, включая комбинацию
|
||||
«прямая лестница + `opacity: 0.85`», которая больнее всего для этого риска.
|
||||
- **Не-скоуп обоснован фактурой, а не декларацией**: вариант s1 (трапеция
|
||||
одним `path`) явно отклонён с конкретной причиной (видимое потемнение в
|
||||
углах при `opacity < 1`, золотая сцена, которая это поймает), а не просто
|
||||
«не делаем»; JS-гигиена из s2 отклонена с числом («не сдвинула заход на
|
||||
этаж 1 дальше шума»), а не на глаз.
|
||||
- **DoR (§2.5) закрыт полностью**: файлы и модули перечислены; i18n — «нет»
|
||||
(обоснованно, разметка без текста); миграция/compatibility — «нет»
|
||||
(персистентная модель не меняется); touch — адресован явной ссылкой на
|
||||
`docs/TOUCH-SUPPORT.md` и смоком `smoke_stairs`; перф-бюджеты названы явно
|
||||
(«не меняются», бандл — полоса ±2000 Б, #699); release-артефакты по каждому
|
||||
пункту явно «да» или «нет»; откат описан и тривиален (revert коммита, ни
|
||||
данных, ни миграций, ни флагов); открытых продуктовых вопросов нет.
|
||||
- **Единственный продуктовый вопрос решён автором по умолчанию, а не
|
||||
вынесен владельцу голословно**: «делать ли К1 при маргинальной пользе для
|
||||
реальных домов» закрыт ссылкой на уже данное владельцем поручение
|
||||
(«довести открытые задачи до S8») и явной инженерной оценкой цены/выгоды —
|
||||
это не техническая деталь, а продуктовый вопрос приоритизации, и формат
|
||||
(«что неясно · предлагаемый вариант по умолчанию, владелец может
|
||||
поменять») соответствует требуемому §7.1.
|
||||
- **«Принято предположительно» — действительно технические пункты**:
|
||||
расположение помощника строк разметки, не-изменение карточки
|
||||
пространства/PDF, судьба трапеции/стрелки, временный харнесс для локальных
|
||||
метрик АС4 — ни один не наблюдаем пользователем, продуктовых вопросов,
|
||||
ошибочно не заданных владельцу, не нашёл.
|
||||
- **Риск пересечения с #725/#739 признан, а не замолчан**: в момент ревью
|
||||
#725 — `S4-spec-review`/`S7` по словам автора («на код-ревью»), не `S8`;
|
||||
ТЗ явно требует снятия AC4 на ветке, приведённой к актуальному `dev`, а не
|
||||
предполагает, что файлы разошлись раз и навсегда.
|
||||
- **Трек `ask` обоснован корректно**: критерии «перф» и «геометрия» из §5
|
||||
применимы (цена переключения этажа меняется, меняется разметка символа),
|
||||
файлы класса A (`src/stairs-view.ts`, `src/stairs-editor.ts`) есть, задача
|
||||
не инфраструктурная.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не гонял `typecheck`/`npm test`/`npm run build` — на этапе spec кода для
|
||||
задачи не существует (ветки `issue/740-*` нет), это обязанность
|
||||
реализатора и следующего код-ревью; зависимости/Chromium на этапе spec не
|
||||
ставились (#696).
|
||||
- Не запускал Full Performance и не оценивал правдоподобность абсолютных
|
||||
цифр 37–46/171–231 мс и прогнозов «вдвое меньше на CI» числовым
|
||||
моделированием — ТЗ само помечает их как «оценка, а не обещание», и
|
||||
единственный судья — AC4 на реальном прогоне после реализации.
|
||||
- Не проверял, что `s3`/`s1` эксперименты автора (ветки на его локальной
|
||||
машине) существуют где-либо кроме описания в issue — принял методологию
|
||||
(«3 сэмпла × 3/2 раунда, конфигурации чередуются, медианы») как разумную
|
||||
инженерную практику, без доступа к сырым данным.
|
||||
- Не оценивал числовую границу «6.77 см при r=32 см» в АС полным перебором
|
||||
радиусов 30–10000 см самостоятельно — проверил формулу (`inner = 0.18 ×
|
||||
radius`, `STAIR_STROKE_CM = 3.6`) на согласованность с текстом, не
|
||||
пересчитал весь диапазон.
|
||||
- Не проверял состояние #725/#739 глубже слов автора о статусе («на
|
||||
код-ревью») — не моя обязанность на этом этапе; риск пересечения
|
||||
зафиксирован в ТЗ как риск №4, а не как блокер DoR.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. ТЗ полное по §7.1, каждый AC однозначен, проверяем и имеет названный
|
||||
способ доказательства; защитные AC (АС1, АС2) доказаны без пустых столбцов;
|
||||
продуктовый вопрос решён автором по умолчанию в верном формате, без скрытых
|
||||
допущений, выданных за факт. Единственная находка — Low, устаревший номер
|
||||
строки в описательной прозе профилирования, не в контракте — снята
|
||||
ревьюером без возврата автору. Открытых продуктовых вопросов нет.
|
||||
|
||||
---
|
||||
|
||||
**Вердикт:** зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `b27ae06ef815` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `791d1e396d66c509f253ed021e03b0811e40055b`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 791d1e396d66
|
||||
```
|
||||
- Тело issue: `9cfcfc9e8157d5a25961e793ed530de257848fb5d870dee885fbd71103ffb3bd`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
Reference in New Issue
Block a user