mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 21:01:21 +00:00
@@ -0,0 +1,223 @@
|
||||
# SPEC-REVIEW-492-r1
|
||||
|
||||
Issue: #492 «CI: проверять точный кандидат интеграции и полный набор зависимостей selection/reuse»
|
||||
Этап: ТЗ на ревью (PROCESS.md §2.4). Трек: полный (не `small`) — ТЗ живёт в
|
||||
`docs/specs/492-exact-candidate-and-input-manifest.md`.
|
||||
Заход: r1 · блокирующих циклов израсходовано 0 из 4.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Предмет ревью — файл `docs/specs/492-exact-candidate-and-input-manifest.md`
|
||||
(174 строки) и обновление `docs/specs/README.md`, оба из коммита `941c7cff`
|
||||
на ветке `issue/492-exact-candidate-and-input-manifest`. Материал не менялся
|
||||
между заходами (r1, предыдущих раундов не было — единственный комментарий в
|
||||
issue — аналитика S2, не вердикт ревью).
|
||||
|
||||
Класс задачи по AGENTS.md: issue заявлен `infra`/`P1`/`tech-debt`, ни одного
|
||||
файла класса A ни в ТЗ, ни в перечне «Затронутых файлов» (§12 ТЗ). По
|
||||
механическому признаку AGENTS.md такая задача могла бы идти вне S-флоу вовсе;
|
||||
автор явно называет это в комментарии S2 и ведёт задачу по меткам ради
|
||||
ревью конвейером, ссылаясь на прецедент того же рода — #472, #475, #481
|
||||
(все — gate-инфраструктура, все проверены: #472 и #481 закрыты через полный
|
||||
S1…S8, #475 — `small`, полный S1…S8 тоже пройден, класс файлов везде B/C/D).
|
||||
Прецедент подтверждён напрямую (`gh issue view`), это корректная и уже
|
||||
устоявшаяся практика репозитория, а не самовольное расширение флоу. Первый
|
||||
вопрос «какую строку Core user jobs закрывает задача» здесь неприменим по
|
||||
предмету: `docs/SCOPE.md` описывает продуктовые обязательства перед тремя
|
||||
персонами, а #492 правит собственный процесс проверки (`PROCESS.md`,
|
||||
`.github/workflows/**`), у которого пользователя-персоны в смысле SCOPE.md
|
||||
нет — сценарий и адресаты корректно названы в §1.1 ТЗ как «автор задачи,
|
||||
ревьюер, обслуживающий чат, владелец, читающий статусы».
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Прочитаны целиком `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (весь файл,
|
||||
включая §1–§10.4: классы файлов, лёгкий/полный трек, лимит циклов §4,
|
||||
обязательные разделы ТЗ §7.1, шаблоны вердиктов §7.2, гейты §8).
|
||||
- Прочитано тело issue #492 и единственный комментарий (аналитика владельца,
|
||||
S2), где подтверждены источники аудита по SHA.
|
||||
- **Каждое фактическое техническое утверждение §1 ТЗ проверено чтением кода на
|
||||
цитируемом SHA `ea6061e9`, а не принято на слово:**
|
||||
- п.1 (слияние): `.github/workflows/process.yml` на `ea6061e9` — шаг «Слить
|
||||
ветку в dev» действительно делает `git rebase origin/dev`, затем сразу
|
||||
`git push … HEAD:dev` без какой-либо проверки нового дерева между ребейзом
|
||||
и push; шаг «dev ушёл вперёд» только комментирует, не блокирует (строки
|
||||
~917–969 файла на этом SHA). Подтверждено.
|
||||
- п.2 (реюз бэкенда): `scripts/gate-reuse.mjs` на `ea6061e9` —
|
||||
`HARNESS.backend.roots` = `['tests_backend', 'custom_components',
|
||||
'pytest.ini', 'scripts/backend-coverage-baseline.txt', 'pyproject.toml']`
|
||||
— не включает `scripts/support-relay/**`, `scripts/sh3d-convert/**`,
|
||||
`scripts/config-schema.json`, `scripts/dump-config-schema.py`; при этом
|
||||
`reuseKey` подмешивает `sourceFingerprint(root)` (полный `src/**`) во
|
||||
**все** job без исключения, включая `backend`. `scripts/classify-changes.mjs`
|
||||
на том же SHA: регэксп `backend` содержит `scripts/support-relay/`, но не
|
||||
`pyproject.toml` и не `scripts/sh3d-convert/`. Оба утверждения — и «relay
|
||||
классифицируется как backend, но не входит в ключ реюза», и «pyproject/
|
||||
sh3d-convert не классифицируются вовсе» — подтверждены буквально.
|
||||
- п.4 (мутанты): `scripts/backend-test-guard.mjs` на `ea6061e9` — третий
|
||||
аргумент (`testFile`) действительно опционален с дефолтом
|
||||
`tests_backend/test_ha_import_export.py`; `grep` по `mutation-gate.mjs`
|
||||
даёт 35 использований этого гарда, что согласуется с «10 из 35 без
|
||||
третьего аргумента» (точное число 10 не пересчитывалось построчно —
|
||||
порядок величины и факт существования дефолта подтверждены, этого
|
||||
достаточно для проверки формулировки проблемы на этапе ТЗ).
|
||||
- Ни одно техническое утверждение раздела «Проблема» не оказалось догадкой,
|
||||
выданной за факт — все проверяемые пункты подтвердились чтением
|
||||
процитированного SHA.
|
||||
- Сверены оба направления связи issue ↔ ТЗ: issue #492 не содержит прямой
|
||||
ссылки на файл ТЗ в теле (обычно это делает владелец/автор отдельной правкой
|
||||
либо ссылка остаётся в `docs/specs/README.md` — она на месте), сам файл ТЗ
|
||||
ссылается на issue первой строкой; `docs/specs/README.md` содержит строку
|
||||
`#492` → `492-exact-candidate-and-input-manifest.md`. Связь двусторонняя по
|
||||
факту (README + текст ТЗ), проверено чтением диффа коммита `941c7cff`.
|
||||
- Проверено соответствие обязательных разделов §7.1 PROCESS.md: сценарий
|
||||
(§1.1), что человек увидит до/после (§1.2), проблема (§1), скоуп/не-скоуп
|
||||
(§2/§3), контракт поведения (§4–§8), UX/модель данных/i18n (§10.1, явное
|
||||
«не затрагиваются» — корректно для infra), критерии приёмки (§10, AC1–AC10),
|
||||
риски (§10.2), откат (§9), release-артефакты (§11) — все присутствуют.
|
||||
Отдельного раздела «план автотестов» под таким заголовком нет, но по
|
||||
содержанию план распределён по §8 (пять групп отрицательных тестов с
|
||||
конкретными представителями и файлами) и §12 (перечень новых/правимых
|
||||
`test/*.test.mjs`) — это не пропуск, а другая раскладка того же
|
||||
содержания; не самостоятельная находка.
|
||||
- Проверена внутренняя согласуемость: таблица категорий манифеста (§5.1)
|
||||
сверена построчно со списком представителей §8.1 — каждая категория
|
||||
(`source`/`tests`/`fixtures`/`config`/`toolchain`/`protocol`) имеет хотя бы
|
||||
один представитель в негативных тестах, кроме `tests` (уже покрыта
|
||||
существующим корнем `tests_backend` в текущем `HARNESS`, поэтому не входит
|
||||
в список **новых** гарантий). Список из «шести мутантов на протокол» в §13
|
||||
и §8.5 совпадает поимённо (6 пунктов).
|
||||
- По каждому AC1–AC10 отдельно проверено, к какому пункту §8 (или §7) он
|
||||
привязан как доказательство — см. находку ниже, единственный разрыв на
|
||||
AC6.
|
||||
- Дешёвые гейты не прогонялись отдельно: Validate на `941c7cff` зелёный
|
||||
(ссылка дана в постановке задачи), а класс изменений — только `docs/**`
|
||||
(класса C), код не менялся. `typecheck`/`test`/`build`/`check-docs`
|
||||
нерелевантны этапу ТЗ и этому диффу.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — AC6 не называет способ доказательства
|
||||
|
||||
**Файл:** `docs/specs/492-exact-candidate-and-input-manifest.md`, AC6 (раздел
|
||||
10) и §5.3.
|
||||
|
||||
AC6 формулирует защитное утверждение: «`backend` не зависит от `src/**` —
|
||||
включено последним коммитом после AC4–AC5». Это ровно тот тип критерия,
|
||||
для которого PROCESS.md §2.5 требует явно назвать способ доказательства
|
||||
(`unit`/`backend`/`smoke`/`golden`/«ревью кода»), а последующее код-ревью
|
||||
(§2.7) потребует таблицу «AC · чем доказан · чем краснеет» — но в
|
||||
доступном виде оно уже должно быть заложено на этапе ТЗ, как это сделано
|
||||
для AC1–AC5, AC7–AC9 через §8.1–§8.5.
|
||||
|
||||
Для AC1–AC5 и AC7–AC10 каждое утверждение имеет прямую привязку к
|
||||
конкретному пункту §8 (представитель, отрицательный тест или таблица
|
||||
случаев). Для AC6 такой привязки нет: §8.1 перечисляет представителей всех
|
||||
шести проверок (`frontend` через `src/**` там не фигурирует ни разу как
|
||||
объект отрицательного теста), а §5.3 лишь описывает механизм («ключ = хеш
|
||||
`inputsOf(job)`, `sourceFingerprint` уходит») без утверждения, что где-то
|
||||
проверяется обратное: что правка файла из `src/**` **не** меняет ключ
|
||||
`backend` после финального коммита.
|
||||
|
||||
**Сценарий отказа:** реализация выполняет AC1–AC5, AC7–AC10 с тестами, но
|
||||
финальный коммит AC6 сводится к удалению строки `source:
|
||||
sourceFingerprint(root)` из вычисления ключа `backend` без теста. Регресс
|
||||
(кто-то по невнимательности вернёт эту строку в будущей правке
|
||||
`check-inputs.mjs`, или `inputsOf('backend')` по ошибке продолжит включать
|
||||
`src/**` через общий `source`-манифест) не поймает ни один автотест —
|
||||
ровно то поведение, которое всё ТЗ ставит целью исключить («Зелёный вердикт
|
||||
обязан означать, что именно этот код проверен»), окажется непроверенным
|
||||
для собственного шестого критерия.
|
||||
|
||||
**Почему это Medium, а не High:** AC6 не блокирует остальные девять
|
||||
критериев и не делает ТЗ невыполнимым — пробел локален и дёшево чинится:
|
||||
одна строка в §8.1 (представитель — файл `src/**`, например
|
||||
`src/houseplan-card.ts`; ожидание — ключ `backend` **не** меняется, ключ
|
||||
`frontend`/`smoke` меняется) плюс одноимённый пункт в перечне тестов §12
|
||||
(`test/gate-reuse.test.mjs`). Находка в скоупе задачи (сам AC6 уже в §10) —
|
||||
чинится в этом же ТЗ, отдельный issue не заводится.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все пять пунктов раздела «Проблема» (§1) фактически подтверждены чтением
|
||||
кода на процитированном SHA `ea6061e9` — ни одно не оказалось домыслом
|
||||
(детали — выше, «Как проверялось»).
|
||||
- Скоуп/не-скоуп (§2/§3) чётко разграничивают протокол проверки от
|
||||
содержания самих проверок и от продуктового кода; при обнаружении
|
||||
продуктового дефекта явно предписан отдельный issue — соответствует
|
||||
правилу «Medium вне скоупа = новый issue» на будущее код-ревью.
|
||||
- §4 (точный кандидат) корректно устраняет реальный, только что
|
||||
подтверждённый чтением дефект — push без проверки между ребейзом и push;
|
||||
алгоритм п.1–7 покрывает все ветвления (dev не двигался / двигался с
|
||||
равным patch-id / двигался с другим patch-id / Validate красный / lease
|
||||
отклонён / попытки исчерпаны), и каждое ветвление имеет счётчик выхода в
|
||||
статус (`S6`/`S7`/`S8`) без зависания.
|
||||
- §6 (замыкание входов гарда) адресует ровно найденный технический дефект
|
||||
(10 из 35 гардов без третьего аргумента, `trail-resume-test-guard.mjs`
|
||||
скрывает пути тестов) явно объявляемым `GUARD_INPUTS` и статическим
|
||||
замыканием импортов — реализуемо без исполнения кода на этапе отбора.
|
||||
Точечность выбора по стороне патча внутри `src/**` явно вынесена в
|
||||
не-скоуп (§3, §6.4) — не оставлена недосказанной.
|
||||
- §7 (ночной прогон) — минимальная точечная правка, устраняющая
|
||||
единственный названный дефект (успех в момент постановки в очередь).
|
||||
- §9 (откат) корректен для infra-задачи без продуктового кода: revert
|
||||
коммитов, отдельно назван риск устаревания старых маркеров реюза (один
|
||||
лишний полный прогон, не сбой).
|
||||
- Раздел «Принятые предположения» (§13) содержит только технические решения
|
||||
(эквивалентность Validate на ветке проверке дерева; PAT остаётся PAT;
|
||||
порядок реализации) и явно помечен как «принято предположительно,
|
||||
поменять свободно» — соответствует PROCESS.md §7.1: продуктовых вопросов
|
||||
к владельцу нет и не должно быть, поскольку задача не имеет
|
||||
пользовательской поверхности в смысле `docs/SCOPE.md`.
|
||||
- AC1–AC5, AC7–AC10 однозначны и проверяемы, каждый имеет прямую привязку к
|
||||
конкретному отрицательному тесту или таблице случаев в §8.
|
||||
- Формат ТЗ и связь issue ↔ ТЗ ↔ README соблюдены; трек (полный, не
|
||||
`small`) выбран верно — задача касается четырёх поверхностей протокола
|
||||
CI одновременно, что явно нарушает критерий «одна поверхность» лёгкого
|
||||
трека (сам ТЗ называет это прямо: «Трек: полный … без файлов класса A»).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Реализацию по существу — на этом этапе её нет, в diff'е только
|
||||
`docs/specs/**` и `docs/specs/README.md` (оба класса C). Код-ревью
|
||||
впереди.
|
||||
- Точный подсчёт «10 из 35» гардов без третьего аргумента (§1 п.4) — вручную
|
||||
не пересчитывал каждое из 35 вхождений `backend-test-guard.mjs` в
|
||||
`mutation-gate.mjs`; подтверждён факт существования дефолтного значения и
|
||||
порядок величины через `grep -c`, этого достаточно для проверки
|
||||
формулировки проблемы, но не является независимым пересчётом точного
|
||||
числа.
|
||||
- Достижимость таймингов §10.2 (5–8 мин ожидания Validate внутри ревью-job,
|
||||
лимит 45 мин, «десятки минут в 3 шардах» для расширенного отбора
|
||||
мутантов) — это оценки автора на будущее, эксплуатационная проверка
|
||||
относится к пост-мержу/код-ревью, не к ТЗ.
|
||||
- Тяжёлые гейты (`golden`, `smoke`, `performance_smoke`, backend pytest) —
|
||||
не прогонялись: diff класса C, изменений в `src/**`/`custom_components/**`
|
||||
нет, гейты нерелевантны этапу и типу изменений.
|
||||
- Прецеденты #472/#475/#481 проверены только по метаданным issue
|
||||
(`gh issue view`: заголовок, метки, состояние) и списку файлов
|
||||
`docs/reviews/` — содержимое их спек-документов построчно не сверялось с
|
||||
#492, кроме одного (`SPEC-REVIEW-481-r1.md`), прочитанного как образец
|
||||
формата и калибровки серьёзности находок для infra-специфики.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый. High: 0, Medium: 1 (в скоупе задачи — чинится в этом же ТЗ, отдельный
|
||||
issue не заводится). Пробел дешёвый: одна строка в §8.1 плюс один тестовый
|
||||
файл в §12.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/492-exact-candidate-and-input-manifest`, коммит `941c7cff7a1b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `b9b9b1a2171c2cafd5f24cf83b1d6c5baa2fbb6b`
|
||||
```
|
||||
git log --all --format='%H %T' | grep b9b9b1a2171c
|
||||
```
|
||||
- ТЗ `docs/specs/492-exact-candidate-and-input-manifest.md`, блоб `2fcaeb946e984a30e0188a15ebed904eb3d31b7d`
|
||||
```
|
||||
git log --all --find-object=2fcaeb946e984a30e0188a15ebed904eb3d31b7d -- docs/specs/492-exact-candidate-and-input-manifest.md
|
||||
```
|
||||
Reference in New Issue
Block a user