mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-07 15:09:30 +00:00
@@ -0,0 +1,203 @@
|
||||
# SPEC-REVIEW-520-r2
|
||||
|
||||
## Якоря
|
||||
|
||||
- Issue: [#520](https://github.com/Matysh/houseplan-card/issues/520)
|
||||
«Первый устойчивый кадр большого дома вырос до 3.36 с при потолке 3.0 с —
|
||||
регрессия после #509/#500», метка `S4-spec-review`, состояние `OPEN`
|
||||
(подтверждено `gh issue view 520 --json labels,state`, 2026-09-10).
|
||||
- ТЗ: тело issue #520, раздел `## ТЗ`, продолжает жить в теле issue (#517:
|
||||
архив `docs/specs/` закрыт для новых ТЗ) — то же место, что и в r1.
|
||||
- sha256 текущего тела issue (`gh issue view 520 --repo Matysh/houseplan-card
|
||||
--json body -q .body | sha256sum`):
|
||||
`7f68e4cd99242f44e13e9929cd0a7aa9dc3cb6ae5f871878670bade18a2436ac`.
|
||||
Отличается от хеша r1 (`5477d052e0e87182f51a9b3b05d37fce0a7ada3d9dc66c10f92
|
||||
7f64564ccc6e4`) — тело менялось, что и ожидается после возврата на правки.
|
||||
- Заход r2. Бюджет циклов §4 до этого ревью: **1/4** (израсходован r1,
|
||||
жёлтый вердикт). Вердикт ниже — снова жёлтый, поэтому расходует цикл:
|
||||
после этого ревью бюджет **2/4**.
|
||||
- Материал кода не менялся: `git log --oneline -5` на момент этого ревью —
|
||||
тот же диапазон, что видел r1 (`ad2f9afd` — публикация документа r1,
|
||||
`27ea23c1`/`3d439805`/`37ef970d` — диагностика бенчмарка, `test(perf):`,
|
||||
без единой строки в `src/**`). Изменилось только тело issue.
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Повторный заход ревью ТЗ (PROCESS.md §2.4, §2.10) после жёлтого вердикта r1.
|
||||
Предмет — **дельта тела issue между r1 и r2**: три правки, заявленные автором
|
||||
в комментарии `2026-09-10T15:23:01Z` («M1 — раздел «Риски»», «M2 — AC3», «M3 —
|
||||
изометрия»), плюс всё, до чего эта дельта дотягивается (AC4, симметрия
|
||||
профилей). Остальной текст ТЗ (продуктовая рамка, AC1/AC2/AC5, откат,
|
||||
release-артефакты, «принято предположительно») дельту не задевает —
|
||||
наследуется из r1 без повторной проверки (см. раздел «Унаследовано»).
|
||||
|
||||
Дельта локальна: правка текста ТЗ, ни ребейза, ни смены контракта, ни новой
|
||||
подсистемы — полный разбор всей задачи заново не требуется.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Получено текущее тело issue #520 (`gh issue view 520 --json body`) и три
|
||||
новых комментария после r1 (фикс-комментарий автора, метка
|
||||
`S4-spec-review` возвращена).
|
||||
2. Прочитан `docs/reviews/SPEC-REVIEW-520-r1.md` целиком — вердикт, три
|
||||
находки Medium, материал (SHA `27ea23c1ec25`, дерево `f80bcdd58b72`,
|
||||
тело `5477d052e0e8…`).
|
||||
3. **Каждая находка r1 сверена построчно с текущим текстом**, а не принята
|
||||
на слово автора:
|
||||
- M1 (риски) — раздел `### Риски` в теле issue теперь существует, 4
|
||||
пункта. Проверил сам утверждение «потребителей `elementProperties` в
|
||||
дереве нет»: `grep -rn "elementProperties" src test scripts demo
|
||||
--exclude-dir=srv` — 0 совпадений, подтверждает пункт 1 раздела.
|
||||
- M2 (AC3) — строка AC3 в таблице теперь помечена `(не гейт, запись)`,
|
||||
колонка «чем краснеет» заменена на «— · критерий поглощён
|
||||
AC1+AC2+AC4»; AC3 больше не выдаёт себя за независимо доказуемый гейт.
|
||||
- M3 (изометрия) — AC4 расширен до «`modelReadyMs` обоих профилей —
|
||||
взаимодействия и изометрии — внутри относительного лимита»; в
|
||||
«Контракте» добавлено «изометрия чинится тем же изменением и
|
||||
проверяется тем же способом (AC4)».
|
||||
4. **Симметрию M3 перепроверил количественно, а не по формулировке.**
|
||||
Пересчитал по формуле `demo/performance/evaluate.mjs:73-118`
|
||||
(`effective = min(hardMaxMs, max(base*(1+ratio), base+allowance))`) не
|
||||
только `modelReadyMs` (как это сделал r1), но и второе число из того же
|
||||
предложения «Симптома» — `firstStableRenderMs` профиля изометрии.
|
||||
Прочитаны бюджеты `demo/performance/budgets-large-house-isometric.json:6-7`:
|
||||
`firstStableRenderMs`: `maxRegressionRatio: 0.2`, `noiseAllowanceMs: 250`,
|
||||
`hardMaxMs: 3500`. См. находку Medium-1 ниже — результат разошёлся с тем,
|
||||
что покрывает амендированный AC4.
|
||||
5. Проверено, что потолок `cache.entries.cleanFloor` в
|
||||
`budgets-large-house-isometric.json` и `budgets-isometric-smoke.json` уже
|
||||
равен 100 (не поднимался в `914e8402`, который трогал только два файла
|
||||
interaction-профиля) — значит для AC5 симметрии с изометрией не требуется,
|
||||
вопрос из хвоста M3 («и его budgets-файлов, если они регрессировали
|
||||
похожим образом по cleanFloor») закрыт отрицательным результатом: там
|
||||
регресса нет.
|
||||
6. `git log --oneline -5` и `git status` — рабочая копия на `ad2f9afd`, той
|
||||
же, что видел r1; продуктовый код (`src/**`) не менялся между раундами,
|
||||
гейты кода этому раунду не относятся (см. «Чего не проверял»).
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium-1 — амендированный AC4 не покрывает второе число из того же предложения «Симптома»: `firstStableRenderMs` изометрии
|
||||
|
||||
**Файл:** тело issue #520, AC4 (строка 59 полученного тела) против «Симптома»
|
||||
(строка 3) и «Контракта» (строка 46).
|
||||
|
||||
«Симптом» приводит для профиля изометрии **два** числа одним предложением:
|
||||
«Профиль изометрии — то же: `modelReadyMs` 2165.6 → 2626.3 при относительном
|
||||
потолке 2598.72, `firstStableRenderMs` 2227 → 2678.2». Правка M3 (закрытие
|
||||
находки r1) добавила в AC4 только первое из них — `modelReadyMs` «обоих
|
||||
профилей». Второе число того же предложения контракт не упоминает вовсе.
|
||||
|
||||
Пересчитано по той же формуле, которую сам автор и r1 применили к
|
||||
`modelReadyMs` (`demo/performance/evaluate.mjs:107-118`,
|
||||
`budgets-large-house-isometric.json:7`: `maxRegressionRatio: 0.2`,
|
||||
`noiseAllowanceMs: 250`, `hardMaxMs: 3500`):
|
||||
|
||||
```
|
||||
relativeLimit = max(2227 * 1.2, 2227 + 250) = max(2672.4, 2477) = 2672.4
|
||||
effective = min(hardMaxMs=3500, relativeLimit=2672.4) = 2672.4
|
||||
```
|
||||
|
||||
Кандидат **2678.2 > 2672.4** — превышение на 5.8 мс (≈0.22 %). Тот же класс
|
||||
«мягкого» (не абсолютного) нарушения, что и `modelReadyMs` изометрии
|
||||
(2626.3 против 2598.72, превышение ≈1.05 %), который M3 признал достаточным
|
||||
основанием для правки AC4. По одинаковому стандарту оба числа одного
|
||||
предложения должны попасть в контракт одинаково — либо оба, либо ни одного с
|
||||
явным объяснением почему.
|
||||
|
||||
Это не формальность: относительные пределы (в отличие от абсолютного
|
||||
`firstStableRenderMs ≤ 3000` профиля interaction) не гейтуют обычный
|
||||
`Validate`, они видны только при ручном чтении «полного сравнения» —
|
||||
единственный документ, который скажет будущему код-ревьюеру, что именно
|
||||
сверять в этом логе, — это текст AC4. Формулировка «Контракта» («изометрия
|
||||
… проверяется тем же способом (AC4)», строка 46) сейчас **утверждает больше**,
|
||||
чем буквально требует таблица AC4: в контракте — «тем же способом» для
|
||||
изометрии в целом, а в AC4 — только одна из двух её метрик.
|
||||
|
||||
**Чем чинится:** одной правкой AC4 — добавить `firstStableRenderMs`
|
||||
изометрии в тот же относительный чек, либо явно исключить его одной фразой
|
||||
(например, «в пределах шума 250 мс, отдельно не гейтуем») там же, где ТЗ уже
|
||||
объясняет статус AC3. Не блокирует контракт — правка одна и дешёвая.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| Medium-1 — отсутствует раздел «Риски» (§7.1) | Добавлен раздел `### Риски`, 4 пункта; названный ревьюером риск (`elementProperties`) снят с указанием способа проверки | Тело issue, раздел `### Риски`; проверено `grep -rn "elementProperties" src test scripts demo --exclude-dir=srv` → 0 совпадений |
|
||||
| Medium-2 — AC3 не имеет дешёвого способа доказательства | AC3 явно понижен до записи: помечен `(не гейт, запись)`, «чем краснеет» заменено на «поглощён AC1+AC2+AC4» — больше не выдаёт себя за независимый гейт | Тело issue, строка AC3 таблицы AC |
|
||||
| Medium-3 — регрессия изометрии не отражена в контракте/AC | AC4 расширен на `modelReadyMs` обоих профилей; в «Контракте» явно сказано, что изометрия чинится тем же изменением | Тело issue, AC4 (строка 59) и «Контракт» (строка 46). **Частично** — см. новую находку Medium-1 этого раунда: второе число того же предложения «Симптома» (`firstStableRenderMs` изометрии) осталось непокрытым |
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Приняты без повторной проверки — дельта их не задевает:
|
||||
|
||||
- **Причина регрессии измерена, не догадка**: три технических утверждения о
|
||||
поведении `wrapped`-свойств Lit (`createProperty`, форсированная запись в
|
||||
`changedProperties` на первом апдейте, работа `requestUpdate` с
|
||||
необъявленным именем) — r1 сверил дословно с исходником
|
||||
`@lit/reactive-element` (`docs/reviews/SPEC-REVIEW-520-r1.md`, раздел «Как
|
||||
проверялось» п.4, материал SHA `27ea23c1ec25`, дерево `f80bcdd58b72`).
|
||||
- **AC1** (source-scan тест по образцу существующего
|
||||
`config-adoption-ownership.test.mjs`) и **AC2** (юнит на вызов
|
||||
`onBodyReplaced`) — доказуемы дёшево и детерминированно, текст этих строк
|
||||
между r1 и r2 не менялся.
|
||||
- **AC5** (`cache.entries.cleanFloor` ≤ 100 на двух interaction-файлах
|
||||
бюджетов) — привязан к точным файлам, числа сверены построчно; не менялся.
|
||||
- **Откат** (revert двух строк `static properties` + потолка 180) и
|
||||
**release-артефакты** (`User-Visible: no`, разблокирует
|
||||
`v1.74.0-beta.1`) — просты, однозначны, не менялись.
|
||||
- **Продуктовая рамка** (администратор дома, поверхность — первый показ
|
||||
большого дома) совпадает с `docs/SCOPE.md` J1 и фикстурой 60 комнат/200
|
||||
устройств — r1 сверил, текст не менялся.
|
||||
- **Выбор полного трека** — верен, не оспаривается.
|
||||
- **Ни одного вопроса владельцу не требуется** — весь материал технический,
|
||||
подтверждено и в r1, и в этом раунде.
|
||||
|
||||
Документ-источник: `docs/reviews/SPEC-REVIEW-520-r1.md`, материал —
|
||||
ветка `issue/520-first-frame` на `27ea23c1ec25`, дерево `f80bcdd58b72`.
|
||||
|
||||
## Что проверено и корректно (этот раунд)
|
||||
|
||||
- Все три пункта фикс-комментария автора (`2026-09-10T15:23:01Z`)
|
||||
подтверждены чтением текущего тела issue, а не приняты по заявлению — M1 и
|
||||
M2 закрыты полностью, M3 закрыт частично (см. Medium-1 выше).
|
||||
- Потолки `cache.entries.cleanFloor` в бюджетах изометрии (`budgets-large-
|
||||
house-isometric.json`, `budgets-isometric-smoke.json`) не поднимались в
|
||||
`914e8402` и уже равны 100 — симметричная правка AC5 для изометрии не
|
||||
требуется, это установлено чтением файлов, а не предположением.
|
||||
- Код между раундами не менялся (`git log`, `git status` на `ad2f9afd`) —
|
||||
дешёвые гейты (`tsc`, `test`, `build`) этому раунду не относятся по той же
|
||||
причине, что и в r1: предмет этапа — текст ТЗ, продуктовой правки в дереве
|
||||
ещё нет.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- **Гейты кода** (`tsc --noEmit`, `npm test`, `npm run build`,
|
||||
`check-docs.mjs`) — не гонял: `src/**` не менялся ни в r1, ни между r1 и
|
||||
r2, весь диапазон коммитов — диагностика бенчмарка (`test(perf):`).
|
||||
- **`npm run invariants`** — не гонял: геометрия и её ссылки не тронуты.
|
||||
- **Реальный прогон `Full Performance`** — не запускал; числа Симптома
|
||||
(включая пересчитанный `firstStableRenderMs` изометрии) взяты из текста
|
||||
issue и перепроверены только арифметически по опубликованной формуле и
|
||||
файлам бюджетов, не переизмерены заново на живом рантайме.
|
||||
- **Регистрацию мутанта `adoption-bodies-declared-reactive`** в
|
||||
`scripts/mutation-gate.mjs` — предмет код-ревью, ещё не существует.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый. High: 0, Medium: 1 (в скоупе задачи — чинится автором в тексте ТЗ,
|
||||
без нового issue). Возврат в «ТЗ в работе» (`S3-spec`).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/520-first-frame`, коммит `ad2f9afd2bfe` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `cc8e3bd73eeb7274db2725b8b308e12dc59a36e4`
|
||||
```
|
||||
git log --all --format='%H %T' | grep cc8e3bd73eeb
|
||||
```
|
||||
- Тело issue: `c5e79b63b5312eb40c5243fba19bd003f9b72e11b83fed51c1b1522547cc349c`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user