diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index b0a4f2ed..0def59ef 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,6 +1,6 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1011, issue: 353. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1012, issue: 354. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| @@ -16,6 +16,7 @@ | #635 | [CODE-REVIEW-635-r1.md](CODE-REVIEW-635-r1.md) | code · r1 | 🟡 жёлтый | 1 | 0 | индекс молчаливо теряет находки и врёт числами по текущему | `docs/reviews/CODE-REVIEW-639-r1.md` `CODE-REVIEW-637-r1.md` `docs/reviews/CODE-REVIEW-594-r1.md` `docs/LESSONS.md` | | #635 | [CODE-REVIEW-635-r2.md](CODE-REVIEW-635-r2.md) | code · r2 | 🟡 жёлтый | 1 | 1 | docs/reviews/INDEX.md, зафиксированный в материале ревью, устарел на собственном SHA — …; parseFindings/parseFiles: фолбэк «первая строка тела блока» вырезает начало буллета и п… | `docs/reviews/INDEX.md` `SPEC-REVIEW-625-r1.md` `SPEC-REVIEW-625-r2.md` `CODE-REVIEW-625-r1.md` `CODE-REVIEW-625-r2.md` `process.yml` `test/reviews-index.test.mjs` `INDEX.md` | | #635 | [CODE-REVIEW-635-r3.md](CODE-REVIEW-635-r3.md) | code · r3 | 🟢 зелёный | 0 | 0 | firstParagraph: ветка нет\b в фильтре мёртвая из-за ASCII-only \b в JS-регэкспах, расхо… | `scripts/reviews-index.mjs` | +| #627 | [SPEC-REVIEW-627-r1.md](SPEC-REVIEW-627-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | избыточное (не противоречивое) условие в AC2; влияние на touch не названо явным пунктом | `docs/TOUCH-SUPPORT.md` | | #625 | [SPEC-REVIEW-625-r1.md](SPEC-REVIEW-625-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | новый инвариант markers[].id не описывает исход для уже испорченной хранимой конфигурации; продуктовые формулировки §7.1 неполны; не проговорены явные «нет» по i18n/touch | `validation.py` `__init__.py` | | #625 | [SPEC-REVIEW-625-r2.md](SPEC-REVIEW-625-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — | | #625 | [CODE-REVIEW-625-r1.md](CODE-REVIEW-625-r1.md) | code · r1 | 🟡 жёлтый | 0 | 0 | Три из четырёх точек вызова validate_active_marker_ids не имеют ни одного теста, exerci…; AC5 текстуально обещает «отдельные тесты сохраняют поведение при отсутствующем length» …; store.py:async_save_config_state — controller.async_flush() и последующий controller.re… | `custom_components/houseplan/websocket_api.py` `test_validation.py` | diff --git a/docs/reviews/SPEC-REVIEW-627-r1.md b/docs/reviews/SPEC-REVIEW-627-r1.md new file mode 100644 index 00000000..c3900d9c --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-627-r1.md @@ -0,0 +1,222 @@ +# SPEC-REVIEW-627-r1 + +Issue: #627 — «Бюджет бандла: граф онбординга вырос 13,9 → 33,8 КБ gzip без потолка; +редакторский в 531 Б от потолка; словари settings/support/topology грузятся для +всех языков» +Этап: ревью ТЗ (PROCESS.md §2.4). Трек: полный (метка `small` не выставлена; +владелец назвал 3 нарушенных критерия §5 в комментарии-оценке — влияние на +производительность, не одна поверхность, сложность > 3). +Заход: r1 · блокирующих циклов израсходовано 0 из 4 (лимит для полного трека — 4). + +## Вердикт + +**Зелёный.** High: 0, Medium: 0 (в скоупе задачи — не заведено, т.к. находок нет), +Low: 2, обе сняты этим ревью с записью ниже. + +## Скоуп ревью + +ТЗ живёт в теле issue (правило #517) — рецензируется тело issue #627, раздел +`## ТЗ`, плюс единственный комментарий-оценка (Matysh, 2026-09-24T01:06:10Z), +который фиксирует отказ от лёгкого трека и корректировку AC2 «≤20 КБ» → измеримое +значение. Комментариев-возражений или уточнений владельца после этого нет — +раунд первый, второго материала не существует, поэтому разделы «Закрытие +раунда r0» и «Унаследовано» не применяются (правило появляется только с r2, +§2.10). + +## Как проверялось + +1. `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §1, §2.4, §2.5, §2.9, §2.10, §4, + §5/§5.1, §7.1, §7.2, §12 — прочитаны целиком для этой сессии. +2. Тело issue #627 и его единственный комментарий получены `gh issue view 627 + --json body,labels,comments` (репозиторий уже локально доступен, `gh` + аутентифицирован). +3. **Фактическая сверка числовых и технических утверждений ТЗ с текущим кодом** + (обязательный шаг для «не додумано/не выполнимо», раз ТЗ полагается на точные + измерения): + - `src/i18n/settings.ts`, `support.ts`, `topology.ts` — подтверждено, что все + три модуля сегодня статически импортируют `en/ru/de/fr` (совпадает с + утверждением «словари грузятся для всех языков»). + - `src/i18n/registry.ts`, `language-runtime.ts` — подтверждено существование + `LanguageRuntime` (две попытки, отпечаток сборки, `fallback`-состояние) и + составного гейта `languageRenderGate` (`inert`+`aria-busy`, ветки + `cold`/`warm`/`ready`) — механизм, который ТЗ предлагает переиспользовать + по экземпляру на пространство, действительно существует и умеет то, что + от него требуется. + - `scripts/bundle-budget.mjs` — подтверждены `LAZY_EDITOR_GZIP_CEILING = + 245_400`, `LAZY_GRAPH_CEILING_BAND = 2_000`, `lazyGraphCeilingViolation` + (полоса ±2000, «выше — отказ», «ниже полосы — опустите потолок») и что + правило #593 уже гейтит editor/furniture-art тем же механизмом, который ТЗ + просит распространить на onboarding. Числа ТЗ (244 433 / 245 400, запас + 967 Б) совпадают с текущими константами и не расходятся с методом счёта. + - `scripts/bundle-manifest.mjs` — подтверждено, что `lazyOnboardingFiles` / + `lazyOnboardingGzipBytes` уже считаются в манифесте (но не гейтятся — точно + то, что описывает «Проблема»), и что механизм retry-токенов сейчас держит + ровно 7 замен (`editor/onboarding/isometric/german/french/furnitureArt/pdf` + — «1/1/1/1/1/1/1»), что подтверждает посылку «тем же механизмом, что у + de/fr» как техническую, а не гипотетическую. + - `docs/USER-GUIDE.ru.md:207-215` — контракт #348 (нейтральный индикатор без + вспышки английского, две ограниченные попытки, тост «Не удалось загрузить + языковой пакет…» при отказе) подтверждён дословно; ключ `toast. + locale_load_failed` и `notifyLanguageLoadFailures` найдены в + `src/houseplan-card.ts:2530` и `src/i18n/registry.ts` — ТЗ не придумывает + поведение, а корректно расширяет уже задокументированный контракт. + - `scripts/mutation-registry.mjs` — существующий аналог `french-locale- + wrong-dictionary` (guard `smoke_french_locale.mjs`) подтверждён; ТЗ ссылается + на него как на образец для нового мутанта корректно. + - Все 13 смоков и 4 существующих unit-теста, названных в «Плане автотестов», + проверены на существование файлов (`demo/smoke_*.mjs`, `test/*.test.mjs`) — + присутствуют все, кроме заявленного новым `test/i18n-lazy-namespaces. + test.mjs` (его отсутствие и есть корректное «новый»). + - `test/zigbee-topology.test.mjs:503-511` — подтверждено, что + `hasTopologyTranslation('ru'|'de', …)` сегодня вызывается синхронно; ТЗ + верно называет необходимость перевести эти вызовы на ожидание `ensure`, + иначе после лениизации тест проверял бы откат, а не перевод. +4. Гейты этого этапа не запускались: ревью ТЗ проверяет текст, а не код — + исполняемых гейтов на этой стадии нет, изменений в рабочем дереве не делалось + (документ не кладётся в `docs/reviews/`, см. системную инструкцию). + +## Проверка обязательных разделов (§7.1) + +Все обязательные разделы присутствуют и не пустые: сценарий (персона + +поверхность + момент — для обеих персон отдельно, включая явное «не должна +добавить ни байта… во View» для домочадцев/киоска) · что человек увидит до и +после (одной фразой, без терминов реализации, с явным «английская вспышка … +недопустима ни в одном кадре») · проблема (с измеренной таблицей и +экспериментом, включая честное признание «исходный AC2 недостижим») · скоуп и +не-скоуп (5 пунктов / 4 пункта, включая явный отказ от выноса form-kit из +первого кадра как отдельного issue) · контракт поведения (загрузка / холодное +открытие / смена языка на лету / отказ / гейт хоста) · UX («новых экранов нет», +переиспользуются существующие) · модель данных и миграция («нет», обосновано) · +i18n («ключи не меняются») · AC1–AC7, каждый с «чем доказан» и «чем краснеет» · +план автотестов (unit/смоки/мутанты/прочее) · риски (5 пунктов) · откат · +release-артефакты. + +Два продуктовых раздела («сценарий», «что человек увидит») идут первыми и +отвечают на вопрос, а не подменяются техническим описанием — соответствует +требованию §7.1. + +## Проверка однозначности и доказуемости AC + +AC1–AC7 численно непротиворечивы и проверены на согласованность между собой и +с текущими константами (см. «Как проверялось» выше): + +- AC1/AC2: пороги (`≤ 28 000 Б`, `−15 000 Б от 244 433`) арифметически + совместимы с полосой `±2000` и правилом #593; отдельно указано, что «−6 500 Б + от 34 526» слабее, чем «≤ 28 000» (28 026 против 28 000) — избыточная, но не + противоречивая формулировка (Low L1, снято, см. ниже). +- AC3–AC7: каждый привязан к конкретному тесту/смоку и конкретному мутанту, + кроме AC7, для которого «чем краснеет» — «любой лишний запрос» в существующем + смоке AC4, а не отдельный именованный мутант; это оправдано, так как AC7 + проверяет *отсутствие* запроса, а не логическую ветку — мутанта, ломающего + «ничего не грузить», для такого свойства не бывает содержательного. +- Ни одно AC не описывает поведение, которого нет ни в одном документе: + контракт заимствован из #348/USER-GUIDE дословно, а не придуман. + +## Догадка vs решение + +Раздел «Принято предположительно» отделяет ровно то, что процесс разрешает +решать без владельца (§7.1: «где хранится состояние… именование… стратегия +тестов»): en статически / ru-de-fr лениво, гранулярность чанков «пространство × +язык», место загрузчиков в ленивых, а не в первом графе, переиспользование +`LanguageRuntime`, составной гейт хоста, переиспользование тоста. Все шесть +пунктов действительно технические (не наблюдаемы пользователем) и корректно +помечены как «поменять свободно» — ревьюер не оспаривает ни один: они +согласуются с уже проверенным устройством `language-runtime.ts` и `registry.ts`. + +Продуктовых вопросов владельцу в ТЗ нет; проверка показывает, что их +действительно не осталось — единственная точка, которая могла требовать +решения владельца («AC2 недостижим»), уже решена в комментарии-оценке самим +автором со ссылкой на измерение, а не угадана. + +## Готовность к DoR (§2.5) — проверено заранее, чтобы не возвращать по мелочи + +- ТЗ есть, ссылка issue↔ТЗ на месте (тело issue). +- AC1…AC7 пронумерованы, у каждого способ доказательства указан. +- Затронутые модули названы на уровне класса (A: `src/i18n/**`, + `src/houseplan-card.ts`, рантаймы; B: `scripts/bundle-*.mjs`, тесты, смоки, + реестр мутантов; D: бандл) — детализация соответствует принятой в проекте + практике (не требуется построчный список файлов). +- i18n: новых ключей нет — пункт закрыт явным «не меняются». +- Миграция/compatibility: «нет», обосновано отсутствием изменений конфига. +- Влияние на перф: названо количественно (это и есть предмет задачи). +- Влияние на touch: **не названо явным пунктом** в ТЗ — см. Low L2 ниже. +- Release-артефакты: названы (бандл, манифест), golden — явно «без изменений + эталонов», changelog — явно «не нужен» с обоснованием. +- Откат: есть. +- Открытых продуктовых вопросов нет (проверено выше). + +## Находки + +### Low L1 — избыточное (не противоречивое) условие в AC2 + +`docs/reviews` (issue #627, тело, раздел AC2): условие «не менее чем на 6 500 Б +ниже 34 526» (≤ 28 026) строго слабее соседнего «≤ 28 000» и никогда не станет +самостоятельно связывающим — оно ничего не меняет и не создаёт двусмысленности +для реализующего агента (действующий порог всё равно 28 000). Не блокирует; +снимаю с записью, а не требую правки ТЗ ради стилистики. + +### Low L2 — влияние на touch не названо явным пунктом + +DoR (§2.5) требует «влияние на touch по `docs/TOUCH-SUPPORT.md` (View и +киоск — блокирующие)» отдельным пунктом; в ТЗ такого пункта нет буквально. +Косвенно он закрыт: комментарий-оценка называет ровно три нарушенных критерия +лёгкого трека из пяти (перф, одна поверхность, сложность) и явно не называет +«влияние на touch» — по методу §5 (нарушенный критерий называется явно, +остальные, значит, не нарушены) touch не задет, и по существу задача не трогает +ни одного интерактивного элемента, только сеть/тайминг загрузки словарей за +уже существующим индикатором. Не блокирую: добавление буквальной строки +«Touch: нет влияния — не тронута ни одна интерактивная поверхность» было бы +чисто редакционным дополнением, ценность которого меньше стоимости очередного +цикла ревью ради одной строки (тот самый принцип, ради которого написан §2.10). +Снимаю с записью; при возврате в `S3-spec` по другой причине эту строку стоит +добавить заодно. + +## Что проверено и корректно + +- Классификация трека (полный, не `small`) обоснована и совпадает с + фактическим объёмом изменения (3 поверхности, влияние на перф, сложность + ленивого гейта). +- Числа проблемы и AC воспроизведены и не противоречат текущему коду и + константам гейта бюджета. +- Технический механизм, на который опирается ТЗ (`LanguageRuntime`, гейт + хоста, retry-токены), существует и умеет ровно то, что от него требуется — + задача не полагается на несуществующий код. +- Контракт поведения — не изобретение, а документированное расширение #348. +- Все AC проверяемы и у каждого назван падающий мутант или иной способ + «умеет падать», кроме AC7, где это оправдано природой утверждения + (отсутствие запроса). +- Не-скоуп разумно ограничивает задачу и не откладывает решение продуктового + вопроса под видом технического. + +## Чего не проверял + +- Не запускал `npm run typecheck`/`npm test`/`npm run build` — на этапе ревью + ТЗ кода ещё нет, гонять гейты не по чему. +- Не проверял вручную числа `эксперимента (не коммитился)` из «Проблемы» — + доверяю им как контексту принятия решения по AC2, они не являются частью + AC и не требуют независимого воспроизведения на этом этапе; AC2 их не + использует напрямую (там свои измеримые пороги). +- Не читал полностью историю связанных issues #423/#459/#474/#593/#600/#608/#614 + — использовал их только как источник конкретных фактов (константы, наличие + функций), которые перепроверил в коде напрямую, а не как источник доверия. + +--- + +**Материал раунда:** тело issue #627 (раздел `## ТЗ`) на момент чтения +2026-09-24T01:06Z, репозиторий на `0f97e3e639c4eb06b7866b8c1c757d98e2c0a5f7` +(`dev`, только для сверки технических утверждений ТЗ с текущим кодом — ТЗ не +привязано к коду и ревьюется как текст issue). + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `0f97e3e639c4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `f2603b2998c17fd63d3bc1248b5e2be36e4b598f` + ``` + git log --all --format='%H %T' | grep f2603b2998c1 + ``` +- Тело issue: `11ffae080ad26691a4b8dbd32f7736895c0c25a3f0cabe1b09da0e21666e6562` +- Вердикт конвейера: `green` · High 0