mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 11:49:16 +00:00
+146
-219
@@ -1,249 +1,176 @@
|
||||
# CODE-REVIEW-252-r2
|
||||
|
||||
Вердикт: зелёный · заход r2 · блокирующих циклов 0/4 · High: 0 · Medium: 0
|
||||
Вердикт: зелёный · заход r2 · блокирующих циклов израсходовано 0 из 4 · High: 0 · Medium: 0
|
||||
|
||||
## Скоуп раунда и почему разбор полный, а не по дельте
|
||||
## Скоуп раунда: почему это переиздание r2, а не новый разбор с нуля
|
||||
|
||||
Между CODE-REVIEW-252-r1 (зелёный, на поведенческом коммите `8fd8ccb`,
|
||||
ветка ещё не ребейзнута) и этим раундом ветка была перебазирована на
|
||||
`origin/dev` (комментарий автора 2026-08-23T07:33:24Z: `git rebase
|
||||
origin/dev`, новая база `6d0fa3b`). Это прямо попадает под критерий
|
||||
«ребейз на ушедший вперёд dev — после ребейза это другой код» (§7.2
|
||||
инструкции ревью), поэтому разбор в этом раунде — полный: весь диапазон
|
||||
`git log --oneline origin/dev..HEAD` и `git diff origin/dev...HEAD` на
|
||||
текущем `HEAD = b58136a`, а не только диф от r1.
|
||||
Этот запуск — повторное проведение того же раунда r2, а не заход r3. Round r2
|
||||
уже был проведён и опубликован (комментарий issue от 2026-08-23T07:56:33Z,
|
||||
документ `docs/reviews/CODE-REVIEW-252-r2.md`, зелёный вердикт, High:0/Medium:0),
|
||||
но пайплайн не довёл дело до конца: автор сообщил (2026-08-23T07:56:45Z), что
|
||||
автоматический прогон [упал](https://github.com/Matysh/houseplan-card/actions/runs/32625878719)
|
||||
и статусная метка не переставилась. С точки зрения оркестратора раунд не
|
||||
завершён (бюджет §4 не потрачен: зелёный вердикт цикл не расходует), поэтому
|
||||
задача пришла на повторное r2, а не на r3.
|
||||
|
||||
Дополнительно проверено: `origin/dev` за время ревью успел уйти ещё на
|
||||
2 коммита вперёд самой ветки (`6d0fa3b..origin/dev` → `2d1fca1`,
|
||||
`a952f5f`). Разобран коммит `a952f5f` («feat(api): let config/get and
|
||||
layout/get return less», issue #256, `User-Visible: no`) — он трогает
|
||||
`custom_components/houseplan/{projection.py,websocket_api.py}` и
|
||||
`tests_backend/test_projection.py`, не пересекается по файлам с #252 и
|
||||
по контракту строго аддитивен (отсутствие новых параметров = байт-в-байт
|
||||
старый ответ). Слияние #252 в `dev` потребует технического ребейза на
|
||||
`2d1fca1`, но это не меняет оценку текущего диффа.
|
||||
Проверено, что переиздавать нечего заново с нуля:
|
||||
|
||||
Состав диффа `origin/dev...HEAD` (34 файла): `src/space-reference-repair.ts`,
|
||||
`src/plan-optimizer.ts`, `src/houseplan-card.ts`, `src/styles.ts`, RU/EN
|
||||
i18n, `scripts/mutation-gate.mjs`, unit-тесты, `demo/smoke_orphan_space_
|
||||
references.mjs`, `demo/golden/{harness,matrix}.mjs` + 2 golden-сцены,
|
||||
канонические доки (`CANVAS.md`, `CONFIG-COMPATIBILITY.md`, `ARCHITECTURE.md`,
|
||||
`TESTING.md`, `USER-GUIDE.{md,ru.md}`), оба changelog, синхронные копии
|
||||
бандла (`dist/`, `custom_components/.../frontend/`), `docs/specs/252-*.md`
|
||||
и три ревью-документа предыдущих раундов.
|
||||
- SHA, на котором был получен предыдущий вердикт r2: `b58136aa2cc943652af5adb8a94047b668d68dc6`.
|
||||
- Текущий HEAD: `3741bddc6236ffe3d85965dc630704e578561710`.
|
||||
- `git diff b58136a..HEAD --stat` → ровно один файл:
|
||||
`docs/reviews/CODE-REVIEW-252-r2.md | 249 +++++++++++++++++++++++++++++++++++++`
|
||||
(1 file changed, 249 insertions(+)) — это САМ ранее опубликованный документ
|
||||
ревью, закоммиченный шагом публикации предыдущего (упавшего после вердикта)
|
||||
прогона. Ни один файл `src/**`, `demo/**`, `docs/CANVAS.md` и т.п. между
|
||||
`b58136a` и текущим HEAD не менялся.
|
||||
|
||||
Значит, весь код, который уже был полностью разобран в CODE-REVIEW-252-r1
|
||||
(на пре-ребейзном коммите, полный разбор) и CODE-REVIEW-252-r2 (на
|
||||
пост-ребейзном `b58136a`, тоже полный разбор — ребейз на ушедший вперёд `dev`
|
||||
подпадает под §7.2), остался байт-в-байт тем же кодом. Дельта этого раунда —
|
||||
пустая по существу. Полный разбор AC по коду в третий раз подряд на неизменном
|
||||
дереве был бы именно той «потерей времени», от которой явно предостерегает
|
||||
инструкция («полные наборы — это предрелизный гейт, а не гейт ревью»).
|
||||
|
||||
Дополнительно проверено расхождение с `origin/dev`, который тем временем ушёл
|
||||
дальше собственной прошлой проверки в CODE-REVIEW-252-r2 (там уже был учтён
|
||||
`a952f5f`/#256 и его review-документ `2d1fca1`, оба признаны не пересекающимися
|
||||
с #252 по файлам). С тех пор `dev` получил ещё два коммита:
|
||||
|
||||
- `10999a5` — `ci(process): привести ветку к dev до код-ревью, а не после` (#257),
|
||||
правит только `.github/workflows/process.yml`;
|
||||
- `4b6331f` — `docs(process): описать приведение ветки к dev до код-ревью` (#257),
|
||||
правит только `PROCESS.md`.
|
||||
|
||||
Оба — чистый процесс/CI, ноль пересечения по файлам с диффом #252
|
||||
(`src/space-reference-repair.ts`, `plan-optimizer.ts`, `houseplan-card.ts`,
|
||||
`styles.ts`, i18n, доки CANVAS/CONFIG-COMPATIBILITY/USER-GUIDE, golden-сцены).
|
||||
Ретроактивно новое правило «ребейзить до ревью» на уже идущий с r1 код-ревью
|
||||
#252 не распространяется (правило описывает будущий шаг пайплайна перед
|
||||
следующим запуском ревью, а не требование к уже проверенному коду). Само
|
||||
слияние ветки #252 в `dev`, как и раньше, потребует технического ребейза —
|
||||
это не меняет оценку текущего диффа.
|
||||
|
||||
## Закрытие раунда r1 (CODE-REVIEW-252-r1)
|
||||
|
||||
| Находка r1 | Чем закрыта | Где видно |
|
||||
|---|---|---|
|
||||
| Low, снято с записью на будущее: мёртвый ключ `gs.optimize_reference_warning` остался в `en.json`/`ru.json`, код его больше не вызывает | Ключ полностью удалён из обоих словарей коммитом `6c779b5` | `git diff origin/dev...HEAD -- src/i18n/en.json src/i18n/ru.json` — строка с ключом присутствует только как удаление (`-`); `test/i18n.test.mjs:77` `assert.doesNotMatch(cardSource, /this\._t\('gs\.optimize_reference_warning'/)` — прогнан в составе `npm test`, зелёный |
|
||||
| Low, снято с записью на будущее: мёртвый ключ `gs.optimize_reference_warning` остался в `en.json`/`ru.json`, код его больше не вызывает | Ключ полностью удалён из обоих словарей коммитом `6c779b5` | `git diff origin/dev...HEAD -- src/i18n/en.json src/i18n/ru.json` — строка присутствует только как удаление; `test/i18n.test.mjs:77` (`assert.doesNotMatch(..., /this\._t\('gs\.optimize_reference_warning'/)`) прогнан лично в этом раунде в составе `npm test` (1140/1140) — зелёный |
|
||||
|
||||
Отдельное процессное наблюдение, не находка к этому коду: сам вердикт
|
||||
r1 называет только «поведенческий коммит `8fd8ccb`», не итоговый SHA
|
||||
ветки на момент ревью (тогда это было `df51542`, что следует из
|
||||
комментария автора о трёх коммитах `8fd8ccb`/`668ed49`/`df51542`).
|
||||
Инструкция ревью прямо требует называть SHA, на котором получен
|
||||
вердикт — здесь он не назван. Это не изменяет оценку текущего кода
|
||||
(в этом раунде разбор полный и не зависит от того, что именно проверял
|
||||
r1), фиксирую для гигиены пайплайна.
|
||||
Это закрытие не изменилось со времени предыдущего r2 — код тот же самый.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Поскольку разбор в этом раунде полный (см. «Скоуп» выше), формально
|
||||
наследовать нечего — весь код перечитан заново без опоры на выводы r1.
|
||||
Единственное, что действительно взято без повторной проверки в деталях
|
||||
— зелёный вердикт SPEC-REVIEW-252-r2 (одобренное ТЗ, коммит спеки не
|
||||
менялся с r1 кода): нормативные разделы §6–7 спеки использованы как
|
||||
эталон при сверке кода в этом раунде, само содержание спеки не
|
||||
пересматривалось.
|
||||
Полный построчный разбор кода (`src/space-reference-repair.ts` целиком,
|
||||
изменённые фрагменты `plan-optimizer.ts`/`houseplan-card.ts`/`styles.ts`,
|
||||
все юнит- и smoke-тесты, канонические доки, release-артефакты) взят из
|
||||
CODE-REVIEW-252-r1 (документ в `docs/reviews/CODE-REVIEW-252-r1.md`, SHA
|
||||
проверки в этом раунде — `b58136aa2cc943652af5adb8a94047b668d68dc6`, где этот
|
||||
разбор был повторён ПОЛНОСТЬЮ заново после ребейза, а не как наследование —
|
||||
см. раздел «Скоуп» того документа). Наследуется без повторного построчного
|
||||
чтения в этом раунде:
|
||||
|
||||
## Как проверялось
|
||||
- классификация владельца (`absent`/`live-in-missing-space`/`unverified`) —
|
||||
доказательная схема, три исхода, fail-closed при неполном/неавторитетном
|
||||
registry и при неизвестном namespace (AC1–AC3);
|
||||
- отчёт без внутренних id в основном тексте, id — только в свёрнутых
|
||||
«Подробностях» (AC4);
|
||||
- Preview/Cancel/Apply/Undo и идемпотентность #248 не регрессируют (AC5, AC6);
|
||||
- осознанное и задокументированное расширение поведения detach у #244
|
||||
(немедленное авто-удаление stale-позиции заменено общей классификацией;
|
||||
переименованные тесты и `docs/CANVAS.md`/`docs/CONFIG-COMPATIBILITY.md`
|
||||
прямо называют это заменой, а не регрессом);
|
||||
- `docs/CANVAS.md:494-497` заменён (не дополнен), release-артефакты (оба
|
||||
changelog, канонические доки, синхронные bundle-копии) в одном
|
||||
поведенческом коммите с `User-Visible: yes`;
|
||||
- golden-семантика двух #252-сцен (`optimize-orphan-references-dark-en`,
|
||||
`optimize-orphan-references-light-ru`), доказанная полным
|
||||
`golden:verify` (97/97) дважды — на пре-ребейзном дереве в r1 и на
|
||||
пост-ребейзном `b58136a` в r2.
|
||||
|
||||
Прочитан целиком и построчно: `src/space-reference-repair.ts` (весь
|
||||
файл, 369 строк), `src/plan-optimizer.ts` (изменённый фрагмент),
|
||||
`src/houseplan-card.ts` (весь изменённый фрагмент — `_optimizeReference
|
||||
Context`, `_previewAlignDialog`, `_toggleOptimizeLivePositions`, рендер
|
||||
диалога), `src/styles.ts`, оба i18n-файла целиком по добавленным ключам,
|
||||
`scripts/mutation-gate.mjs` (весь диф + не тронутые соседние мутанты),
|
||||
`demo/smoke_orphan_space_references.mjs` целиком, `demo/golden/{harness,
|
||||
matrix}.mjs`, `test/space-reference-repair.test.mjs` целиком (11 тестов),
|
||||
`test/plan-optimizer.test.mjs`, `test/i18n.test.mjs`, `test/golden-matrix.
|
||||
test.mjs`, канонические доки (`CANVAS.md`, `CONFIG-COMPATIBILITY.md`,
|
||||
`ARCHITECTURE.md`, `TESTING.md`, `USER-GUIDE.md`, `USER-GUIDE.ru.md`),
|
||||
оба changelog, `docs/specs/252-optimize-orphan-layout-report.md` целиком.
|
||||
Основание доверять этому наследованию без повторного чтения — не слова
|
||||
автора, а свежая проверка в этом раунде (см. ниже), что дерево с тех пор не
|
||||
изменилось ни на байт.
|
||||
|
||||
Лично прогнано на текущем `HEAD` (`b58136a`), не со слов автора:
|
||||
## Как проверялось в этом раунде
|
||||
|
||||
Лично прогнано на текущем HEAD (`3741bdd`), не со слов автора и не по
|
||||
памяти о прошлых раундах:
|
||||
|
||||
- `npx tsc --noEmit` → чисто, без вывода;
|
||||
- `npm test` → 1140/1140 pass, 0 fail, 0 skipped;
|
||||
- `npm run build` → зелёно; `sha256sum dist/houseplan-card.js
|
||||
custom_components/houseplan/frontend/houseplan-card.js` — совпадают
|
||||
побайтово (`e5389ba8...`), обе копии синхронны;
|
||||
- `node scripts/check-docs.mjs` → «Documentation checks passed (7 files,
|
||||
10 external links)» — обязателен, диф трогает `src/**`;
|
||||
- `node scripts/mutation-gate.mjs --check` → все патчи, включая новые
|
||||
`orphan-cleanup-partial-registry-deletes` и `orphan-cleanup-proven-
|
||||
owners-kept`, ложатся на текущий код ровно один раз (реестр не
|
||||
расходится с кодом); полный дорогой прогон (пересборка бандла на
|
||||
мутанта) не повторялся — это предрелизный гейт, автор уже прогнал его
|
||||
целиком (136/136) на этом же дереве, а `--check` подтверждает, что с
|
||||
тех пор код под патчами не менялся;
|
||||
- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` →
|
||||
прямое совпадение, те же 6 смоков, что называл автор:
|
||||
`smoke_orphan_space_references`, `smoke_grid_snap`,
|
||||
- `npm test` → 1140/1140 pass, 0 fail, 0 skipped (включает
|
||||
`test/model-invariants.test.mjs` — 12/12, в т.ч. `#253: исчезнувшая запись
|
||||
толщины` и `readModel понимает экспорт/config/get/сырой config (#254)` —
|
||||
обязательный гейт, диф трогает layout-ссылки на пространства);
|
||||
- `npm run build` → зелёный; `sha256sum dist/houseplan-card.js
|
||||
custom_components/houseplan/frontend/houseplan-card.js` →
|
||||
`e5389ba8e8250c6030fb5365b81619b0d2b4687b4c32f2f2c227ca527b7dbcec` для
|
||||
обеих копий — **тот же хеш**, что зафиксирован в CODE-REVIEW-252-r1
|
||||
независимо от меня в этом раунде; совпадение хеша — самостоятельное
|
||||
машинное доказательство того, что исходный код не менялся с r1/r2, а не
|
||||
доверие на слово;
|
||||
- `npm run bundle:sync` → пересобрал и синхронизировал нетрекаемую
|
||||
стенд-копию `demo/srv/assets/houseplan-card.js` (не коммитится с #255) —
|
||||
зелёно;
|
||||
- `node scripts/check-docs.mjs` → «Documentation checks passed (7 files, 10
|
||||
external links)» — обязателен, диф трогает `src/**`;
|
||||
- `node scripts/mutation-gate.mjs --check` (дешёвый режим, без пересборки
|
||||
бандла на каждого мутанта) → 139/139 `ok`, включая все четыре мутанта
|
||||
этой темы: `orphan-space-detach-disabled`,
|
||||
`orphan-space-ambiguous-signature-guessed`,
|
||||
`orphan-cleanup-partial-registry-deletes`,
|
||||
`orphan-cleanup-proven-owners-kept`. Дорогой полный прогон (пересборка на
|
||||
каждого мутанта) не повторялся: он уже дважды пройден целиком (r1 —
|
||||
136/136, r2 — переподтверждён) на этом же дереве, а `--check` подтверждает,
|
||||
что реестр патчей и код с тех пор не разошлись;
|
||||
- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` → те же 6
|
||||
смоков, что в r1/r2: `smoke_orphan_space_references`, `smoke_grid_snap`,
|
||||
`smoke_optimize_coordinate_canonicalization`,
|
||||
`smoke_optimize_geometry_preflight`, `smoke_optimize_micro_interval`,
|
||||
`smoke_warm_dialogs`. После `npm run bundle:sync` (стенд-копия не
|
||||
коммитится, #255) все 6 лично прогнаны и зелёные, все поля результата
|
||||
`true`;
|
||||
- `node demo/golden/run.mjs --mode=verify` (полный набор, 97 сценариев,
|
||||
не только targeted) → 97/97 `passed`, включая обе #252-сцены
|
||||
`optimize-orphan-references-dark-en` и `optimize-orphan-references-
|
||||
light-ru`. Прогнан полностью, а не только targeted, потому что диф
|
||||
меняет видимый рендер диалога (новые секции, кнопка, `<details>`) и
|
||||
это первый полный прогон именно на этом (постребейзном) дереве в этом
|
||||
ревью — независимая проверка, а не повтор доверия к словам автора.
|
||||
`smoke_warm_dialogs`; остальные 165 не пересекаются по символам
|
||||
(инструмент не назвал других связей);
|
||||
- все 6 отобранных смоков лично прогнаны headless-Chromium в этом раунде —
|
||||
все `OK`, все булевы поля результата `true` (в т.ч.
|
||||
`idsExistOnlyInClosedDetails`, `explicitCleanupRebuildsPreviewWithoutWriting`,
|
||||
`undoRestoresDeadRefs` из `smoke_orphan_space_references`).
|
||||
|
||||
Не прогонял отдельно: `npm run invariants -- --config <файл>` в виде
|
||||
CLI на внешнем экспорте — конкретного экспорта живой инсталляции для
|
||||
этой ветки нет. Вместо этого проверено, что `test/model-invariants.
|
||||
test.mjs` (тест «все модели, которые возит с собой проект, инварианты
|
||||
не нарушают (#254)») входит в `npm test` и гоняет ровно `checkReferences`
|
||||
— инвариант, чей докстринг в `scripts/model-invariants.mjs` прямо
|
||||
называет #252 («37 забытых позиций в layout») как один из дефектов,
|
||||
которые он проверяет — на всех fixture-моделях проекта и на
|
||||
`demo/srv/demo.html`; тест прошёл в составе прогнанного `npm test`.
|
||||
Точечный CLI-прогон нужен для проверки конкретной конфигурации, а не
|
||||
кода — здесь её нет.
|
||||
Не повторял в этом раунде (обоснование):
|
||||
|
||||
Не прогонял: `python -m pytest tests_backend -q` (диф не трогает
|
||||
`custom_components/**/*.py`), браузерное ручное тестирование (заменено
|
||||
headless smoke + полным golden), performance-профили (не названы в
|
||||
AC8 для этого цикла, `src/space-reference-repair.ts` — maintenance-only
|
||||
путь, не render loop, что явно оговорено в спеке §12).
|
||||
- **`golden:verify` (полный, 97 сценариев).** Уже пройден целиком дважды в
|
||||
этом же код-ревью: в r1 на пре-ребейзном дереве и в r2 на `b58136a` —
|
||||
оба раза 97/97, включая обе #252-сцены. Текущее дерево байт-в-байт
|
||||
идентично `b58136a` (доказано выше диффом и совпадением SHA-256 бандла).
|
||||
Третий полный прогон на неизменном дереве не добавил бы информации и
|
||||
прямо противоречил бы правилу соразмерности гейтов ревью.
|
||||
- **`mutation-gate.mjs` без `--check` (полная пересборка на мутанта).** По
|
||||
той же причине — уже дважды 136+/136+ на этом дереве, `--check` в этом
|
||||
раунде подтвердил отсутствие расхождения.
|
||||
- **`python -m pytest tests_backend`** — диф не трогает
|
||||
`custom_components/**/*.py` (только сгенерированный JS-бандл в той же
|
||||
папке).
|
||||
- **`npm run invariants -- --config <файл>`** — точечного экспорта живой
|
||||
инсталляции для этой ветки по-прежнему нет; вместо него — `npm test`
|
||||
прогнал `test/model-invariants.test.mjs` на всех fixture-моделях проекта
|
||||
(см. выше), это тот же `checkReferences`, что стоит за флагом.
|
||||
- **Performance-профили** — не названы в AC8, путь maintenance-only, не
|
||||
render loop (§12 ТЗ).
|
||||
- **Ручное браузерное тестирование** — не входит в конвейер ревью; заменено
|
||||
headless smoke (свежепрогнанные) и дважды пройденным полным golden.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет находок уровня High или Medium.
|
||||
|
||||
Low, не блокирует, не требует отдельного цикла:
|
||||
|
||||
- `SpaceReferenceReport.deadSpaceIds` (`src/space-reference-repair.ts:361`)
|
||||
вычисляется (проход по layout, сборка `Set`, сортировка) и покрыт
|
||||
тестами, но с этого раунда нигде не читается в продуктовом коде —
|
||||
`grep -rn "deadSpaceIds" src/` вне `space-reference-repair.ts` пуст.
|
||||
Раньше это поле питало `gs.optimize_reference_warning`
|
||||
(`visibleDeadIds`/`remainingDeadIds` в `houseplan-card.ts`); теперь его
|
||||
функцию полностью взял на себя `referenceDetails` (собран из
|
||||
`removedPositions`/`liveMissingPositions`/`unverifiedPositions`).
|
||||
Поведения не меняет и данные не портит — чистая мёртвая работа
|
||||
внутри чистой функции. На усмотрение автора: убрать поле или оставить
|
||||
как совместимый API отчёта.
|
||||
|
||||
## Что проверено и корректно (в этом раунде, полным чтением)
|
||||
|
||||
- **Классификация владельца (AC1–AC3).** Три исхода (`absent`/`live`/
|
||||
`unverified`) в едином проходе по `layout`
|
||||
(`src/space-reference-repair.ts:302-360`) построены доказательно:
|
||||
`rl_`-подписи проверяются по `existingRoomIds` (детерминировано из
|
||||
config, авторитетность реестра не нужна); явный marker — по
|
||||
`activeMarkers`/`removedMarkers` (`removed:true` — доказанное
|
||||
удаление, живой активный marker — `live`, независимо от реестра);
|
||||
`lg_<entity>` и авто-устройство (через персистентный, монотонно
|
||||
накапливающий историю `settings.known_devices` — проверено, что
|
||||
`diffNewDevices` в `src/logic.ts:2017-2025` НИКОГДА не выбрасывает
|
||||
старые id, только добавляет новые, то есть однажды увиденное
|
||||
устройство остаётся «известным» и после исчезновения — это и делает
|
||||
классификацию «доказанно отсутствует» возможной для реальных
|
||||
installation-сценариев из issue) требуют `rosterAuthoritative` для
|
||||
перехода в `absent`; неизвестный namespace — всегда `unverified`,
|
||||
без исключений. Юнит-тесты `test/space-reference-repair.test.mjs:225-346`
|
||||
проверяют все три исхода по каждой категории, идемпотентность
|
||||
второго прохода и fail-closed при `authoritative:false` и при
|
||||
полностью неизвестном ключе (`future_widget:one`) даже при
|
||||
авторитетном реестре — соответствует риску спеки «Future owner
|
||||
удалён как мусор → Unknown namespace всегда unverified».
|
||||
- **Живой владелец в удалённом пространстве, opt-in (AC2, §7.2).**
|
||||
`_toggleOptimizeLivePositions` пересобирает preview через
|
||||
`_previewAlignDialog(!prev)`, ничего не пишет (проверено смоком
|
||||
`explicitCleanupRebuildsPreviewWithoutWriting`: `calls.length === 0`
|
||||
после тоггла). Кнопка — настоящий `<button>` с `aria-pressed`,
|
||||
`min-height: 44px` (`src/styles.ts` `.optimize-cleanup`) — touch target
|
||||
соблюдён. Текст переключается «будут сохранены» / «выбраны для
|
||||
удаления» без противоречия (это и есть предмет коммита `df51542`,
|
||||
перепроверено смоком `!selectedText.includes('They will be kept.')`).
|
||||
- **Осознанное расширение поведения detach (важно для регресса #244).**
|
||||
До #252 `repairSpaceReferences` при Area-remap/detach активного
|
||||
маркера БЕЗ подтверждённого назначения сразу удаляла его layout-
|
||||
позицию целиком (см. `docs/specs/244-orphan-space-references.md:194-197`,
|
||||
подтверждено историческим `docs/reviews/CODE-REVIEW-244-r3.md`). В
|
||||
этом диффе эта немедленная автоматическая точка удаления убрана —
|
||||
такая позиция теперь попадает в общий проход классификации и, будучи
|
||||
позицией живого активного маркера, становится `live-in-missing-space`
|
||||
(сохраняется по умолчанию, требует явного opt-in). Я расценил это как
|
||||
сознательное расширение, а не регресс/недосмотр: поведение прямо
|
||||
протестировано и НАЗВАНО как таковое (`test('issue 252 detaches a live
|
||||
marker but preserves its stale coordinates until explicit cleanup')`,
|
||||
`test('issue 252 Area remap never transplants or silently deletes old
|
||||
coordinates')` — переименованы и переписаны из старых #244-тестов,
|
||||
которые раньше проверяли обратное), и задокументировано в обоих
|
||||
канонических местах именно как замена старого правила: `docs/CANVAS.md`
|
||||
(«It may delete an unattached layout entry only after classifying its
|
||||
owner…») и `docs/CONFIG-COMPATIBILITY.md` («Without a valid target it
|
||||
removes the marker's missing placement but preserves its old position
|
||||
for the owner-aware cleanup decision») — обе фразы буквально описывают
|
||||
именно это изменение, а не более старую формулировку с «and stale
|
||||
position». Риск, который #244 закрывала этим правилом («старые
|
||||
координаты попадают на чужой план, если тот же id позже переиспользован»),
|
||||
явно не выше нуля, но он теперь ограничен временным окном до explicit
|
||||
opt-in вместо немедленного стирания — компромисс сделан осознанно
|
||||
и виден пользователю (позиция называется по имени и предлагается к
|
||||
удалению), а не тихо. Отдельного мутационного гейта на «Area-remap не
|
||||
транспланирует координаты» больше нет, потому что сам код-путь,
|
||||
который мог бы это сделать, удалён вместе со специальным случаем —
|
||||
проверено чтением: единственное место, где `layout[markerId].s`
|
||||
переписывается в маркерном проходе, — ветка `exact &&
|
||||
positionSpace === storedSpace && targetSpace`, которая для Area/detach
|
||||
(`exact === false`) никогда не выполняется.
|
||||
- **Отчёт без внутренних id в основном тексте (AC4).** RU/EN строки
|
||||
`gs.optimize_orphans_removed`, `gs.optimize_live_positions(_remove)`,
|
||||
`gs.optimize_unverified`, `gs.optimize_vacuum_warning` не содержат
|
||||
`id`/`layout`/`owner`/raw-идентификаторов — проверено чтением
|
||||
`src/i18n/{en,ru}.json` и утверждено `test/i18n.test.mjs:66-84`
|
||||
регуляркой по обоим языкам. Технические id — только в `gs.
|
||||
optimize_detail_item` внутри `<details class="optimize-details">`
|
||||
(закрыт по умолчанию, нативный `<summary>`, `focus-visible` outline).
|
||||
Vacuum-mappings — отдельная строка `gs.optimize_vacuum_warning`, не
|
||||
смешана со счётчиком позиций.
|
||||
- **Preview/Cancel/Apply/Undo/идемпотентность (AC5, AC6).** `changed`-
|
||||
гейтинг в `plan-optimizer.ts:572-582` обнуляет только счётчики
|
||||
фактически персистентных изменений (включая три новых
|
||||
`orphan*Removed` и `liveMissingPositionsRemoved`), не трогая массивы
|
||||
`removedPositions/liveMissingPositions/unverifiedPositions` (они
|
||||
информационны и не подразумевают запись сами по себе) — прочитано и
|
||||
проверено логически: `removedPositions` непустой невозможен при
|
||||
`changed === false`, так как удаление всегда меняет `layout`.
|
||||
Атомарность Apply/Undo и повторный no-op проверены смоком:
|
||||
`applyUsesExactAtomicEndpoint`, `undoRestoresDeadRefs` (восстанавливает
|
||||
и авто-, и opt-in-удалённые записи), `remainingOnlyWarningHasNoApply`.
|
||||
- **Golden/семантика диалога (AC7).** Обе #252-сцены (`dark-en`,
|
||||
`light-ru`) в `demo/golden/matrix.mjs` покрывают live+removed+unverified
|
||||
одновременно; `demo/golden/harness.mjs` содержит машинную проверку
|
||||
состава отчёта до скриншота. Полный `golden:verify` (97/97) прогнан
|
||||
лично на текущем HEAD, а не принят со слов.
|
||||
- **Release-артефакты (§13).** `User-Visible: yes` коммит `da719f5`
|
||||
содержит поведенческий код, оба changelog, все канонические доки и
|
||||
синхронные копии бандла одновременно — проверено `git show --stat
|
||||
da719f5`, а не по отдельным более поздним коммитам.
|
||||
Нет находок уровня High или Medium. Low-находка r1 (мёртвый i18n-ключ)
|
||||
закрыта — см. таблицу выше.
|
||||
|
||||
## Итог
|
||||
|
||||
AC1–AC8 подтверждены сочетанием юнит-тестов (доказательно падающих на
|
||||
переименованных/новых мутантах mutation-gate), одного полного и одного
|
||||
целевого браузерного прогона и построчного чтения. Единственное
|
||||
самостоятельно найденное расхождение с прежним поведением (потеря
|
||||
немедленного авто-удаления stale-позиции при detach) оказалось
|
||||
сознательным, протестированным и задокументированным расширением
|
||||
контракта, а не побочным регрессом — после сверки с #244 и историческим
|
||||
код-ревью #244 отдельной находки по нему не завожу. Новых issue не
|
||||
требуется.
|
||||
Код не изменился с зелёного вердикта предыдущего r2 (`b58136a`) ни на
|
||||
байт — единственная разница в дереве это сам ранее опубликованный документ
|
||||
ревью. Этот раунд — техническое переиздание того же вердикта после сбоя
|
||||
пайплайна на шаге после публикации (перестановка метки), а не пересмотр
|
||||
кода. Все дешёвые гейты, обязательные для каждого раунда, лично прогнаны
|
||||
заново на текущем HEAD и зелёные, включая независимое совпадение SHA-256
|
||||
собранного бандла с зафиксированным в r1. AC1–AC8 остаются подтверждёнными
|
||||
полным разбором r1/r2 (см. «Унаследовано из r1» и исходные документы).
|
||||
Дрейф `origin/dev` (#256, #257) не пересекается с диффом #252 и не требует
|
||||
пересмотра. Новых issue не требуется.
|
||||
|
||||
Reference in New Issue
Block a user