mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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` |
|
||||
|
||||
@@ -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).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `0f97e3e639c4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `f2603b2998c17fd63d3bc1248b5e2be36e4b598f`
|
||||
```
|
||||
git log --all --format='%H %T' | grep f2603b2998c1
|
||||
```
|
||||
- Тело issue: `11ffae080ad26691a4b8dbd32f7736895c0c25a3f0cabe1b09da0e21666e6562`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user