mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-28 19:01:34 +00:00
@@ -0,0 +1,202 @@
|
||||
# Код-ревью #592 · заход r2
|
||||
|
||||
SHA материала: `1403fb440b572cec04d2a82a23e29f72bfbeb73f`. Ветка:
|
||||
`issue/592-extract-dialogs`. Заход r2, блокирующих циклов израсходовано 1 из 4.
|
||||
|
||||
Предыдущий раунд: **r1**, вердикт жёлтый, документ
|
||||
`docs/reviews/CODE-REVIEW-592-r1.md`, материал `61c74a70128a29871547519750bae32695f6e56a`
|
||||
(коммиты `b76f3e57` + `61c74a70`). SHA в вердикте r1 назван прямо (в отличие
|
||||
от типичного пробела #227) — дельта считается от него без дополнительного
|
||||
розыска.
|
||||
|
||||
## Дельта r1 → r2
|
||||
|
||||
```
|
||||
git diff --stat 61c74a70..1403fb44 -- . ':!dist' ':!custom_components/houseplan/frontend' ':!docs/reviews'
|
||||
```
|
||||
|
||||
```
|
||||
scripts/mutation-registry.mjs | 13 ++---
|
||||
scripts/no-new-any.mjs | 121 +++++++++++++++++++++++++++++++++---------
|
||||
test/no-new-any.test.mjs | 102 +++++++++++++++++++++++++----------
|
||||
3 files changed, 176 insertions(+), 60 deletions(-)
|
||||
```
|
||||
|
||||
Плюс некодовый коммит `d2443298` (`docs: review document for #592`, добавляет
|
||||
сам документ r1 в дерево). `src/**`, `test/core-file-budget.test.mjs`,
|
||||
`test/editor-dialog-modules.test.mjs`, `test/houseplan-source.mjs`,
|
||||
`test/space-dialog.test.mjs`, `dist/**`, `custom_components/houseplan/frontend/**`
|
||||
между r1 и r2 не менялись (проверено `git diff --stat` по каждому пути —
|
||||
пусто). Дельта локальна: один коммит `1403fb44`, три файла, ровно правка
|
||||
Medium-находки r1 (M1). Оснований для полного разбора (ребейз на ушедший
|
||||
вперёд `dev`, смена контракта, новая подсистема, объём дельты сравним с
|
||||
исходной задачей) нет — разбор по дельте.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M1** (Medium, в скоупе): `no-new-any.mjs` признавал перенесённой любую добавленную строку, если где-то **в любом файле того же диффа** нашлась дословно совпадающая удалённая строка — без привязки к паре файлов реального переноса. Демонстрировался рабочий обход: несвязанная уборка удаляет строку с `any`, новый файл добавляет текстуально совпадающую — гейт молчит. | Сопоставление переписано на уровне непрерывных **блоков** ≥ `MOVED_BLOCK_MIN` (5) строк подряд, привязанных к конкретному файлу-источнику (`sourcePath` из `--- a/...`), а не к произвольному месту диффа. Одиночное совпадение больше не засчитывается. Добавлен тест, воспроизводящий ровно сценарий обхода из r1, и мутант заменён так, чтобы явно проверять эту дыру. | `scripts/no-new-any.mjs:152-250` (новая `movedLinesByFile`, `MOVED_BLOCK_MIN`); тест `test/no-new-any.test.mjs:172-197` — «#592 одиночное совпадение переносом не считается (M1)» — использует буквально тот же пример (`<span>${this.host._t(k as any)}</span>`), что и в тексте находки r1; мутант `scripts/mutation-registry.mjs` переименован в `no-new-any-forgives-a-single-matching-line`, патч опускает `MOVED_BLOCK_MIN` 5→1, что и есть найденная дыра. |
|
||||
|
||||
Проверено исполнением, не на слово автора:
|
||||
- `node --test test/no-new-any.test.mjs` на `1403fb44` → 15/15 pass, включая
|
||||
новый тест M1.
|
||||
- `node scripts/mutation-gate.mjs --check` → весь реестр `ok`, в том числе
|
||||
`no-new-any-forgives-a-single-matching-line` (мутант применён, `guard`
|
||||
покраснел, патч возвращён — дисциплина «мутант умеет убивать» соблюдена).
|
||||
- `node scripts/no-new-any.mjs --base origin/dev --head HEAD` на реальном
|
||||
диффе задачи → «Новых any нет», 1296 из 1395 добавленных строк признаны
|
||||
перенесёнными (было 1297 в r1 при построчном сопоставлении — минус одна
|
||||
строка, и это ровно то случайное совпадение, из-за которого M1 и была
|
||||
найдена; коммит-сообщение `1403fb44` называет то же число).
|
||||
|
||||
M1 закрыта фактически, не декларативно: старый обход (пример из r1,
|
||||
`room-settings-dialog.ts:66` / `space-settings-dialog.ts:240`) воспроизведён
|
||||
как регрессионный тест и красит гейт при откате блокового порога до 1 строки.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Всё, что дельта не задевает, принимается без повторной проверки — документ
|
||||
`docs/reviews/CODE-REVIEW-592-r1.md`, SHA `61c74a70128a29871547519750bae32695f6e56a`:
|
||||
|
||||
- Побайтовая идентичность переноса всех четырёх диалогов (AC1/AC2, К1) —
|
||||
дельта r2 не трогает ни `src/editors/*.ts`, ни `houseplan-editor-runtime.ts`.
|
||||
- AC2 (golden, 15 названных эталонов — 0 расхождений; два незаявленных кадра
|
||||
`device-icon-state-table-{light,dark}` — предсуществующий дрейф Chromium
|
||||
152/151, отделён экспериментом на чистом `origin/dev`) — рендер не задет.
|
||||
- AC3 (потолок `houseplan-editor-runtime.ts`, `test/core-file-budget.test.mjs`)
|
||||
— файл не менялся.
|
||||
- AC4 (состояние не переехало, `test/editor-dialog-modules.test.mjs`, тест
|
||||
проверен мутацией) — файл не менялся.
|
||||
- AC5 (ленивый граф, `bundle:budget`) — `dist/**` и `custom_components/houseplan/frontend/**`
|
||||
между r1 и r2 не менялись (проверено `git diff --stat`, пусто).
|
||||
- AC6 (якоря мутантов, кроме самого переименованного в этом раунде) —
|
||||
реестр не менялся, кроме записи M1 (see выше).
|
||||
- AC7 (полный набор `npm test` / `gate-small`) — состав тестов вне
|
||||
`test/no-new-any.test.mjs` не менялся.
|
||||
- Смоки AC1 (10 поимённо + `smoke_value_face_source`) и выборка 18 смоков по
|
||||
диалогам — `demo/**` в дельте r2 не тронут (`git diff --stat` пусто).
|
||||
- Три отклонения от ТЗ, разобранные автором в хендоффе (сборщик
|
||||
`test/houseplan-source.mjs`, `test/space-dialog.test.mjs`, имена файлов) —
|
||||
файлы не менялись.
|
||||
- Трейлеры коммитов `b76f3e57`/`61c74a70` и отсутствие правок в CHANGELOG —
|
||||
не переоценивались повторно.
|
||||
|
||||
## Как проверялось (дельта r2)
|
||||
|
||||
Прочитан diff трёх файлов целиком (`git diff 61c74a70..1403fb44 -- scripts/mutation-registry.mjs scripts/no-new-any.mjs test/no-new-any.test.mjs`),
|
||||
включая новую реализацию `movedLinesByFile` построчно:
|
||||
|
||||
- парсинг `--- `/`+++ ` теперь требует пробел после трёх дефисов (точнее
|
||||
прежнего `raw.startsWith('---')`, который ловил любые три дефиса);
|
||||
- удалённые строки группируются по `sourcePath` (файл, из которого строка
|
||||
реально удалена), а не по произвольному ключу — это и есть привязка к паре
|
||||
файлов, которой не хватало в r1;
|
||||
- добавленные строки группируются в непрерывные по номеру строки куски
|
||||
(`runs`), затем для каждого куска ищется наибольший по длине непрерывный
|
||||
блок совпадения среди ещё не потраченных (`spent`) удалённых строк любого
|
||||
файла-источника; засчитывается только если длина ≥ `MOVED_BLOCK_MIN`;
|
||||
- бюджет ведётся позиционно (`spend`/`isSpent`), поэтому повторная вставка
|
||||
того же блока (тест «один удалённый кусок оплачивает ровно одну вставку»)
|
||||
корректно не покрывается дважды.
|
||||
|
||||
Три ручных прогона (перечислены выше в разделе «Закрытие раунда r1») плюс
|
||||
чтение реализации — основной объём проверки этого раунда.
|
||||
|
||||
### Что не является находкой
|
||||
|
||||
Разбирал один пограничный случай парсинга: требование `raw.startsWith('--- ')`
|
||||
(с пробелом) технически может ошибочно принять за заголовок диффа удалённую
|
||||
строку кода, буквально начинающуюся с `"-- "` (например, `-- comment` →
|
||||
в диффе `--- comment`). В TypeScript-файлах, к которым единственно применяется
|
||||
гейт (`isProductTypeScript`, `src/**/*.ts`), такая строка кода практически
|
||||
невозможна (`--` не встречается как первый токен строки вне декремента, а тот
|
||||
не пишется с пробелом после). Прежняя реализация имела ту же двусмысленность
|
||||
(`raw.startsWith('---')`, ещё шире) и просто молча роняла такую строку из
|
||||
подсчёта — по факту новая версия строже прежней, а не слабее. Не поднимаю это
|
||||
до находки: нет реалистичного сценария воспроизведения на продуктовом коде,
|
||||
и это не тот класс дыры, которую M1 описывала (единичное текстовое совпадение
|
||||
между несвязанными файлами) — блоковый порог её по-прежнему закрывает.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет новых находок этого раунда. M1 (Medium, r1) закрыта — см. таблицу выше.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- M1 закрыта по существу: регрессионный тест воспроизводит ровно
|
||||
задокументированный в r1 обход и падает без фикса (проверено мутантом,
|
||||
не декларативно).
|
||||
- Блоковый порог (`MOVED_BLOCK_MIN = 5`) обоснован в комментарии и
|
||||
коммит-сообщении: случайное совпадение пяти строк подряд между
|
||||
несвязанными файлами практически невозможно даже при 887 явных `any` в
|
||||
базе; настоящий перенос подсистемы по определению состоит из таких кусков.
|
||||
- Привязка удалённых строк к файлу-источнику (`sourcePath`) убирает и вторую
|
||||
часть проблемы r1 — сопоставление больше не глобальное по всему диффу без
|
||||
привязки к файлу, хотя поиск блока всё ещё ищет по всем файлам-источникам
|
||||
диффа (это осознанный выбор: перенос может тянуть из нескольких старых мест
|
||||
— задокументировано в комментарии), что при пороге в 5 строк не открывает
|
||||
находку M1 заново.
|
||||
- Бюджет позиционный (`spend`/`isSpent`), не мультимножественный как в r1:
|
||||
тест на повторную вставку блока подтверждает, что вторая копия остаётся
|
||||
новым кодом.
|
||||
- Реальный дифф задачи по-прежнему проходит гейт («Новых any нет»),
|
||||
дельта в количестве признанных перенесёнными строк (1297→1296) —
|
||||
ожидаемая и объяснённая автором (одна случайная строка перестала
|
||||
засчитываться).
|
||||
- Мутант переименован и переориентирован на найденную дыру, а не на
|
||||
соседнюю; `mutation-gate.mjs --check` подтверждает, что весь реестр,
|
||||
включая его, жив.
|
||||
- Трейлеры `1403fb44`/`d2443298`: `Issue: #592`, `User-Visible: no` —
|
||||
корректно, правка гейт-инструментария без изменения продукта, правок
|
||||
CHANGELOG не требуется.
|
||||
- Изменения — класс B (`scripts/**`, `test/**`), переиспользуют номер #592 по
|
||||
правилу AGENTS.md, отдельного issue не требуют.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `npx tsc --noEmit`, `npm run build` (со сверкой бандла), `npm run
|
||||
bundle:budget`, `golden:verify`, `python -m pytest tests_backend`,
|
||||
`npm run invariants`, полный набор `demo/smoke_*.mjs`, `check-docs.mjs`
|
||||
— дельта r2 не трогает `src/**`, `dist/**`, `custom_components/**/*.py`,
|
||||
геометрию/`layout`/`marker.space`/`open_spans`, рендер или `demo/**`;
|
||||
эти гейты и не запускались в r1 сверх необходимого, и здесь дельта их не
|
||||
задевает. Validate на этом SHA (`1403fb44`) зелёный (прогон 35385526275,
|
||||
назван в контексте раунда) — `tsc`/`npm test`/`npm run build` подтверждены
|
||||
им, повторно не гонял.
|
||||
- `npm test` целиком (2770+ тестов) не перегонял ради факта — целевой файл
|
||||
`test/no-new-any.test.mjs` прогнан отдельно (`node --test`, 15/15), он же
|
||||
часть полного набора, Validate на этом SHA подтверждает, что остальной
|
||||
набор зелёный.
|
||||
- Смоки — не запускал ни одного: дельта не трогает `demo/**` и не меняет
|
||||
видимое поведение; `smoke-select.mjs` не запускал по той же причине (нет
|
||||
изменённых символов продукта в дельте, только dev-гейт).
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. Дельта r1→r2 — точечная правка единственной Medium-находки r1,
|
||||
проверена исполнением (тест воспроизводит ровно заявленный обход и умеет
|
||||
падать; мутант подтверждён; реальный дифф задачи по-прежнему проходит гейт).
|
||||
Новых находок нет. Всё, что дельта не касается, унаследовано из r1 по
|
||||
перечню выше.
|
||||
|
||||
---
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/592-extract-dialogs`, коммит `1403fb440b572cec04d2a82a23e29f72bfbeb73f`.
|
||||
- Предыдущий раунд: r1, документ `docs/reviews/CODE-REVIEW-592-r1.md`, SHA `61c74a70128a29871547519750bae32695f6e56a`.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/592-extract-dialogs`, коммит `1403fb440b57` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `fde11c38b97a36045e7148f01f2f459c8a2b88a3`
|
||||
```
|
||||
git log --all --format='%H %T' | grep fde11c38b97a
|
||||
```
|
||||
- Тело issue: `b105f7b3eb3f70575ee5751d1a917940da949981400e342e9cb133153057f673`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user