feat(#371): редизайн модалки каталога ролей — карточки-наборы + per-role результаты импорта #375

Open
agent_coder wants to merge 2 commits from feat/371-roles-catalog into develop
Collaborator

Summary

Интеграция редизайна модалки «Каталог ролей» (дизайнерский хендофф из комментов #371), подключённого к реальному API. Родительский ai-agent-roles.tsx не тронут, контракт { opened, onClose, roles } сохранён. closes #371

  • Сервер importFromCatalog теперь отдаёт по-ролевые списки (createdRoles / skippedRoles с reason) РЯДОМ со старыми счётчиками (обратная совместимость), чтобы UI мог назвать конфликтные/установленные роли.
  • Чистый view-model (catalog-bundle-model.ts): bundlePhase (empty|allNew|allInstalled|updates|mixed, транзиентный skipped игнорится), installedLangForRole (подсказка «установлено на другом языке»), mapCatalogRoleToView — юнит-тесты без монтирования.
  • Карточки-наборы со сводным статусом в свёрнутой шапке (eager useQueries по всем наборам, общие кэш-ключи), одно главное действие на набор, чекбоксы + select/deselect-all, инлайн-плашка результата (окно не закрывается), пер-набор и глобальный «Update all» серией с прогрессом, подсказка про другой язык.
  • Плашка partial различает причину skip: «Rename & install» ТОЛЬКО для name-conflict; already-installed-гонка — информационная (rename-реимпорт снова бы пропустил и схлопнулся в ложный успех).
  • Все строки i18n (en/ru); мок-код хендоффа (SEED/mockImport/delay) удалён.

How verified

На стенде:

  • pnpm --filter server exec tsc --noEmit0; pnpm --filter client exec tsc --noEmit0.
  • Сервер jest ai-agent-roles.service58 passed (вкл. per-role response-shape + гонка source-uniqueness теперь ассертит skippedRoles с reason:'already-installed', не только счётчики).
  • Клиент vitest catalog-bundle-model / catalog-role-install-state / import-message → зелёные.
  • i18n: новый ключ в обоих локалях, оба JSON валидны.

Внутренний цикл: 1 проход ревью (APPROVE with suggestions, блокеров нет). Поправил: плашка partial теряла reason → показывала «Rename & install» и на already-installed-гонке (ложный успех при клике) → пробросил reason, кнопка/текст теперь по причине; добор теста гонки (ассерт на массив skippedRoles, не счётчики).

Scope / вне scope

Вне scope (по ишье): батч-эндпоинт обновления ([API #3]); модалка конфликта с dry-run ([API #4] — оставлен fallback на текущем API); поиск по каталогу (Р12, только визуально).

⚠️ Ручной браузерный QA за человеком: светлая/тёмная темы, реальные раскрытие/чекбоксы, импорт с плашкой success/partial и рабочим «Rename & install», прогресс пер-набор/глобального «Update all», переключатель языка. UX-нюанс: «Update all of N» шлёт N одноролевых success-тостов (мутация оставлена как есть — батч-эндпоинт вне scope).

Checklist

  • критерии приёмки #371 выполнены
  • старый код модалки + мок хендоффа удалены
  • ручной браузерный QA — за ревьюером/человеком
## Summary Интеграция редизайна модалки «Каталог ролей» (дизайнерский хендофф из комментов #371), подключённого к реальному API. Родительский `ai-agent-roles.tsx` не тронут, контракт `{ opened, onClose, roles }` сохранён. closes #371 - **Сервер** `importFromCatalog` теперь отдаёт по-ролевые списки (`createdRoles` / `skippedRoles` с `reason`) РЯДОМ со старыми счётчиками (обратная совместимость), чтобы UI мог назвать конфликтные/установленные роли. - **Чистый view-model** (`catalog-bundle-model.ts`): `bundlePhase` (empty|allNew|allInstalled|updates|mixed, транзиентный `skipped` игнорится), `installedLangForRole` (подсказка «установлено на другом языке»), `mapCatalogRoleToView` — юнит-тесты без монтирования. - Карточки-наборы со сводным статусом в свёрнутой шапке (eager `useQueries` по всем наборам, общие кэш-ключи), одно главное действие на набор, чекбоксы + select/deselect-all, инлайн-плашка результата (окно не закрывается), пер-набор и глобальный «Update all» серией с прогрессом, подсказка про другой язык. - Плашка partial различает причину skip: «Rename & install» ТОЛЬКО для `name-conflict`; `already-installed`-гонка — информационная (rename-реимпорт снова бы пропустил и схлопнулся в ложный успех). - Все строки i18n (en/ru); мок-код хендоффа (SEED/mockImport/delay) удалён. ## How verified На стенде: - `pnpm --filter server exec tsc --noEmit` → **0**; `pnpm --filter client exec tsc --noEmit` → **0**. - Сервер `jest ai-agent-roles.service` → **58 passed** (вкл. per-role response-shape + гонка source-uniqueness теперь ассертит `skippedRoles` с `reason:'already-installed'`, не только счётчики). - Клиент `vitest` catalog-bundle-model / catalog-role-install-state / import-message → зелёные. - i18n: новый ключ в обоих локалях, оба JSON валидны. Внутренний цикл: 1 проход ревью (APPROVE with suggestions, блокеров нет). Поправил: плашка partial теряла `reason` → показывала «Rename & install» и на `already-installed`-гонке (ложный успех при клике) → пробросил `reason`, кнопка/текст теперь по причине; добор теста гонки (ассерт на массив `skippedRoles`, не счётчики). ## Scope / вне scope Вне scope (по ишье): батч-эндпоинт обновления ([API #3]); модалка конфликта с dry-run ([API #4] — оставлен fallback на текущем API); поиск по каталогу (Р12, только визуально). ⚠️ **Ручной браузерный QA за человеком**: светлая/тёмная темы, реальные раскрытие/чекбоксы, импорт с плашкой success/partial и рабочим «Rename & install», прогресс пер-набор/глобального «Update all», переключатель языка. UX-нюанс: «Update all of N» шлёт N одноролевых success-тостов (мутация оставлена как есть — батч-эндпоинт вне scope). ## Checklist - [x] критерии приёмки #371 выполнены - [x] старый код модалки + мок хендоффа удалены - [ ] ручной браузерный QA — за ревьюером/человеком
agent_coder added 1 commit 2026-07-05 05:36:56 +03:00
Integrates the designer-handoff Roles Catalog modal, wired to the real API; the
parent ai-agent-roles.tsx and the { opened, onClose, roles } contract are
unchanged.

- Server importFromCatalog now returns per-role lists (createdRoles /
  skippedRoles with a reason) alongside the existing counters (compat-preserving),
  so the UI can name the conflicting/installed roles.
- New pure view-model (catalog-bundle-model.ts): bundlePhase (empty | allNew |
  allInstalled | updates | mixed, ignoring the transient 'skipped'),
  installedLangForRole (same-slug-different-language hint), mapCatalogRoleToView —
  all unit-tested without mounting.
- Bundle cards with a summary status in the collapsed header (eager useQueries
  fan-out over all bundles, sharing the existing per-bundle cache keys), a single
  primary action per bundle, checkboxes + select/deselect-all, an inline result
  plaque that keeps the modal open, per-bundle and global 'Update all' request
  series with progress, and the other-language hint.
- The partial-result plaque distinguishes the skip reason: only a name-conflict
  offers 'Rename & install'; an already-installed race is informational (a rename
  re-import would just skip again and self-heal into a false success).
- All strings i18n'd (en/ru); mock handoff code (SEED/mockImport/delay) removed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
Collaborator

Открыл PR по #371 (каталог ролей). Внутренний цикл: 1 проход, APPROVE with suggestions. Поправил: плашка partial теряла reason skip'а и предлагала «Rename & install» даже на already-installed-гонке (клик → ложный успех) → пробросил reason, кнопка только для name-conflict; добор теста гонки на массив skippedRoles. tsc 0/0, сервер 58 тестов, i18n в обоих локалях. Браузерный QA — за тобой (в теле).

Открыл PR по #371 (каталог ролей). Внутренний цикл: 1 проход, APPROVE with suggestions. Поправил: плашка partial теряла reason skip'а и предлагала «Rename & install» даже на already-installed-гонке (клик → ложный успех) → пробросил reason, кнопка только для name-conflict; добор теста гонки на массив skippedRoles. tsc 0/0, сервер 58 тестов, i18n в обоих локалях. Браузерный QA — за тобой (в теле).
agent_coder added the review/needs label 2026-07-05 05:36:57 +03:00
Collaborator

Ревью — #375 (feat(#371): редизайн модалки каталога ролей — карточки-наборы + per-role результаты импорта), round 1. Вердикт: CHANGES

Крепкая интеграция, веер 9 аспектов сошёлся, 5 LGTM. Ядро сверено: серверный ответ обратно-совместим (regressions: старые счётчики created/skipped/renamed/errors бит-в-бит те же, import-логика байт-идентична — только originalName-рефактор; per-role createdRoles/skippedRoles чисто аддитивны, считаются в lockstep со счётчиками; родительский ai-agent-roles.tsx не тронут, контракт {opened,onClose,roles} цел); reason-таксономия name-conflict|already-installed совпадает сервер↔клиент (coherence: «Rename & install» ТОЛЬКО для name-conflict, already-installed — информационно, без ложного self-heal); security чист (per-role-список scoped на запрошенный bundle, нет over-exposure/SSRF/XSS — React+i18next авто-эскейп); useQueries переиспользует тот же cache-key-factory (нет коллизий/двойных фетчей); «update all» серией без unhandled-rejection; view-model вынесен чисто и покрыт (26 тестов); i18n — 33 ключа в ОБА локаля, паритет полный (сверил: symdiff новых ключей = 0). Открыто 4 (2 warning + 2 low). Эскалации нет.

Открыто: F1 (bundlePhase рапортует allInstalled даже когда все не-installed роли SKIPPED и установлено 0 → шапка «All installed · up to date» противоречит открытой панели «Installed 0 · 1 skipped»); F2 (клиентская ветка reason→action — несущая логика name-conflict-vs-already-installed — без тестов); F3/F4 (unused useRef; дублированный status-tally view-model↔компонент).

Объективка зелёная (мой прогон, голова a325ddba): server tsc 0; client tsc 0; server jest 126 passed / 5 suites (вкл. per-role response-shape + source-uniqueness-гонка ассертит skippedRoles reason); client vitest 26 passed (catalog-bundle-model / install-state / import-message); i18n оба локаля валидны, +33 ключа с нулевым паритет-гэпом (165-key symdiff — ПРЕ-существующий en/ru-разрыв, не этот PR).

📋 Do (F1–F4) + DROP + что сверено

Do — почини, потом ставь review/needs

  1. F1 [coherence · warning] bundlePhase даёт allInstalled при 0-установлено-но-skipped → шапка противоречит панелиapps/client/src/features/ai-chat/utils/catalog-bundle-model.ts:68-77.
    bundlePhase считает только import/update/installed и намеренно игнорит транзиентный skipped, так что imp===0 && ups===0 короткозамыкает в allInstalled даже когда КАЖДАЯ не-installed роль была name-conflict-скипнута. Сверил (код + тест catalog-bundle-model.test.ts:85-87 bundlePhase([viewRole("skipped")]) → "allInstalled" кодирует это). Достижимо, single-role: bundle с единственной importable-ролью (slug writer) в воркспейсе, где уже есть неродственная роль «Writer» (создана вручную, без source). Install → сервер вернёт skippedRoles:[{writer, name-conflict}], created:0. Клиент (modal:299-304) оверлеит статус skipped → bundle = [{status:'skipped'}]bundlePhaseallInstalled. Свёрнутая шапка рисует зелёный StatusDot «All installed · up to date» (modal:629-634) + зелёный «Installed» primary-экшен (:667-675), А открытая панель ОДНОВРЕМЕННО — жёлтый «Skipped»-бейдж + partial-баннер «Installed 0 · 1 skipped» с «Rename & install» (:961-995). Шапка утверждает «полностью установлен и актуален», когда установлено НОЛЬ. Тот же конфликт в partial-success (часть installed + часть name-conflict-skipped): installed>0, imp/ups 0 → allInstalled «up to date», пока роль skipped и баннер предлагает rename. Fix: не классифицируй как allInstalled при наличии skipped-ролей — const skip = roles.filter(r=>r.status==="skipped").length; и возвращай allInstalled только при imp===0 && ups===0 && skip===0; при skipped рядом с нулём import/update → mixed (шапка не соврёт). Обнови тест :85-87 (all-skipped bundle → mixed).

  2. F2 [test-coverage · warning] Клиентская ветка reason→action (name-conflict vs already-installed) без тестовai-agent-roles-catalog-modal.tsx:293-323, 955-992.
    Сервер КОРРЕКТНО эмитит skippedRoles[].reason (покрыто в ai-agent-roles.service.spec.ts), но ветка, превращающая reason в UI-поведение, живёт в компоненте БЕЗ тестов — а она несущая (собственный коммент :951-954: опустить её = «self-heal в ложное 'installed' при нуле установленного»). Две непокрытых reason-ветки: (1) installBundle (:295-304) фильтрует res.skippedRoles на reason==='name-conflict' и оверлеит skipped ТОЛЬКО их; already-installed-гонку — НЕ должно; регрессни .filter → already-installed-роли пропадут из importable. (2) partial-плашка (:955-992) показывает «Rename & install» + copy ТОЛЬКО при name-conflict, иначе информационное «… already installed» БЕЗ кнопки; renameInstall (:331-367, реимпорт с conflict:'rename', схлопывает partial→success) тоже не гоняется. Pure-model-тесты доказывают, что bundlePhase игнорит skipped, а import-from-catalog-message.test.tsx покрывает лишь summary-нотификацию — ни один не трогает reason-ветку; ни один client-тест не импортит модалку / не ссылается на «Rename & install». Проект штатно рендер-тестит компоненты (role-cards.test.tsx, chat-thread.test.tsx) — это в рамках его планки, не coverage-пуризм. Fix: тест reason-ветки — (a) отрендери плашку с моканым import-результатом: name-conflict-skip рендерит «Rename & install» и по клику реимпортит с conflict:'rename'; already-installed-only skip — информационное copy и НЕТ кнопки; ЛИБО (b) вынеси два решения («какие skipped-slug'и становятся transient-skipped» и «предлагает ли partial "Rename & install"») в чистые хелперы catalog-bundle-model.ts и юнит-тесть ОБА reason-значения (не только name-conflict).

  3. F3 [conventions · low] Unused useRef-импортai-agent-roles-catalog-modal.tsx:1.
    Редизайн тянет useRef из "react", но хук нигде в файле не используется. Мёртвый код, введён этим PR. Гейт не ловит: client eslint no-unused-vars: "off" + noUnusedLocals off → тихо шипится. Fix: import { useEffect, useMemo, useState } from "react";.

  4. F4 [simplification · low] Дублированный status-tally view-model↔компонентcatalog-bundle-model.ts:69-77 + modal (upCount/installedCount/повторный bundlePhase).
    bundlePhase уже считает imp/ups/installed фильтрами, но отбрасывает их и возвращает только фазу; BundlePanel пересчитывает те же тэлли (upCount:614, installedCount:615-617, третий bundlePhase(b.roles):619), а родитель отдельно шлёт importableCount (:238) — массив сканится по статусу ~5-6 раз/рендер, tally-логика в двух местах (тот самый model↔component-разрыв, что PR консолидирует). Не баг (мелкие массивы). Fix: чистый bundleCounts(roles): {importable, installed, update} в модели, bundlePhase выводит из него; в BundlePanelconst counts = bundleCounts(b.roles) один раз. (Совпадает с F1 — общий рефактор tally в модель разом лечит оба.)


DROP — кодеру НЕ делать · калибровочный лог (оператору)

  • [below-threshold] low [stability] Запуск per-bundle «Update all» оставляет глобальный «Update all» активным → гоночные клики могут наложить две серии, клоббер progress/busyBundle. UI-only (сервер идемпотентен, нет краша/unhandled-rejection), требует гоночных кликов. DROP.
  • [below-threshold] low [stability] useQueries на открытии = ~1+N index + N bundle GET одновременно; но каталог маленький (curated), статик-хост с 10с/1МБ-cap, амплифицирующий redundant fetchIndex — пре-существующий. DROP.

Сверено (9 аспектов + мои проверки, голова a325ddba): серверный ответ аддитивен (счётчики бит-в-бит, import-логика байт-идентична, lockstep push, родитель не тронут); reason-таксономия сервер↔клиент совпадает (Rename&install только name-conflict); security (scoped bundle, нет over-exposure/SSRF/XSS); useQueries без key-коллизий; update-all-серия без unhandled-rejection, close-mid-import безопасен (родитель монтирует безусловно, React 18.3 no-op); server per-role без краша на пустом/malformed; view-model чист+покрыт (26); i18n 33 ключа паритет 0; F1 bundlePhase-allInstalled сверил кодом+тестом (достижимо, шапка врёт); F2 reason-ветка не покрыта (coherence+test-coverage независимо). F1/F2 — warning; F3/F4 — low. Эскалации нет — все фиксы однозначны.

## Ревью — #375 (feat(#371): редизайн модалки каталога ролей — карточки-наборы + per-role результаты импорта), round 1. Вердикт: **CHANGES** Крепкая интеграция, веер 9 аспектов сошёлся, 5 LGTM. Ядро сверено: серверный ответ **обратно-совместим** (regressions: старые счётчики `created/skipped/renamed/errors` бит-в-бит те же, import-логика байт-идентична — только `originalName`-рефактор; per-role `createdRoles/skippedRoles` чисто аддитивны, считаются в lockstep со счётчиками; родительский `ai-agent-roles.tsx` не тронут, контракт `{opened,onClose,roles}` цел); reason-таксономия `name-conflict|already-installed` совпадает сервер↔клиент (coherence: «Rename & install» ТОЛЬКО для name-conflict, already-installed — информационно, без ложного self-heal); security чист (per-role-список scoped на запрошенный bundle, нет over-exposure/SSRF/XSS — React+i18next авто-эскейп); useQueries переиспользует тот же cache-key-factory (нет коллизий/двойных фетчей); «update all» серией без unhandled-rejection; view-model вынесен чисто и покрыт (26 тестов); i18n — 33 ключа в ОБА локаля, паритет полный (сверил: symdiff новых ключей = 0). Открыто **4** (2 warning + 2 low). Эскалации нет. Открыто: **F1** (`bundlePhase` рапортует `allInstalled` даже когда все не-installed роли SKIPPED и установлено 0 → шапка «All installed · up to date» противоречит открытой панели «Installed 0 · 1 skipped»); **F2** (клиентская ветка reason→action — несущая логика name-conflict-vs-already-installed — без тестов); **F3/F4** (unused `useRef`; дублированный status-tally view-model↔компонент). **Объективка зелёная (мой прогон, голова `a325ddba`):** server tsc **0**; client tsc **0**; server jest **126 passed / 5 suites** (вкл. per-role response-shape + source-uniqueness-гонка ассертит `skippedRoles reason`); client vitest **26 passed** (catalog-bundle-model / install-state / import-message); i18n оба локаля валидны, +33 ключа с нулевым паритет-гэпом (165-key symdiff — ПРЕ-существующий en/ru-разрыв, не этот PR). <details> <summary>📋 Do (F1–F4) + DROP + что сверено</summary> ### Do — почини, потом ставь `review/needs` 1. **F1 [coherence · warning] `bundlePhase` даёт `allInstalled` при 0-установлено-но-skipped → шапка противоречит панели** — `apps/client/src/features/ai-chat/utils/catalog-bundle-model.ts:68-77`. `bundlePhase` считает только import/update/installed и намеренно игнорит транзиентный `skipped`, так что `imp===0 && ups===0` короткозамыкает в `allInstalled` даже когда КАЖДАЯ не-installed роль была name-conflict-скипнута. Сверил (код + тест `catalog-bundle-model.test.ts:85-87` `bundlePhase([viewRole("skipped")]) → "allInstalled"` кодирует это). Достижимо, single-role: bundle с единственной importable-ролью (slug `writer`) в воркспейсе, где уже есть неродственная роль «Writer» (создана вручную, без `source`). Install → сервер вернёт `skippedRoles:[{writer, name-conflict}]`, `created:0`. Клиент (modal:299-304) оверлеит статус `skipped` → bundle = `[{status:'skipped'}]` → `bundlePhase` → `allInstalled`. Свёрнутая шапка рисует зелёный StatusDot «All installed · up to date» (modal:629-634) + зелёный «Installed» primary-экшен (:667-675), А открытая панель ОДНОВРЕМЕННО — жёлтый «Skipped»-бейдж + partial-баннер «Installed 0 · 1 skipped» с «Rename & install» (:961-995). Шапка утверждает «полностью установлен и актуален», когда установлено НОЛЬ. Тот же конфликт в partial-success (часть installed + часть name-conflict-skipped): installed>0, imp/ups 0 → `allInstalled` «up to date», пока роль skipped и баннер предлагает rename. Fix: не классифицируй как `allInstalled` при наличии skipped-ролей — `const skip = roles.filter(r=>r.status==="skipped").length;` и возвращай `allInstalled` только при `imp===0 && ups===0 && skip===0`; при skipped рядом с нулём import/update → `mixed` (шапка не соврёт). Обнови тест `:85-87` (all-skipped bundle → `mixed`). 2. **F2 [test-coverage · warning] Клиентская ветка reason→action (name-conflict vs already-installed) без тестов** — `ai-agent-roles-catalog-modal.tsx:293-323, 955-992`. Сервер КОРРЕКТНО эмитит `skippedRoles[].reason` (покрыто в `ai-agent-roles.service.spec.ts`), но ветка, превращающая reason в UI-поведение, живёт в компоненте БЕЗ тестов — а она несущая (собственный коммент :951-954: опустить её = «self-heal в ложное 'installed' при нуле установленного»). Две непокрытых reason-ветки: (1) `installBundle` (:295-304) фильтрует `res.skippedRoles` на `reason==='name-conflict'` и оверлеит `skipped` ТОЛЬКО их; already-installed-гонку — НЕ должно; регрессни `.filter` → already-installed-роли пропадут из importable. (2) partial-плашка (:955-992) показывает «Rename & install» + copy ТОЛЬКО при name-conflict, иначе информационное «… already installed» БЕЗ кнопки; `renameInstall` (:331-367, реимпорт с `conflict:'rename'`, схлопывает partial→success) тоже не гоняется. Pure-model-тесты доказывают, что `bundlePhase` игнорит `skipped`, а `import-from-catalog-message.test.tsx` покрывает лишь summary-нотификацию — ни один не трогает reason-ветку; ни один client-тест не импортит модалку / не ссылается на «Rename & install». Проект штатно рендер-тестит компоненты (`role-cards.test.tsx`, `chat-thread.test.tsx`) — это в рамках его планки, не coverage-пуризм. Fix: тест reason-ветки — (a) отрендери плашку с моканым import-результатом: `name-conflict`-skip рендерит «Rename & install» и по клику реимпортит с `conflict:'rename'`; `already-installed`-only skip — информационное copy и НЕТ кнопки; ЛИБО (b) вынеси два решения («какие skipped-slug'и становятся transient-skipped» и «предлагает ли partial "Rename & install"») в чистые хелперы `catalog-bundle-model.ts` и юнит-тесть ОБА reason-значения (не только name-conflict). 3. **F3 [conventions · low] Unused `useRef`-импорт** — `ai-agent-roles-catalog-modal.tsx:1`. Редизайн тянет `useRef` из "react", но хук нигде в файле не используется. Мёртвый код, введён этим PR. Гейт не ловит: client eslint `no-unused-vars: "off"` + `noUnusedLocals` off → тихо шипится. Fix: `import { useEffect, useMemo, useState } from "react";`. 4. **F4 [simplification · low] Дублированный status-tally view-model↔компонент** — `catalog-bundle-model.ts:69-77` + modal (`upCount`/`installedCount`/повторный `bundlePhase`). `bundlePhase` уже считает `imp/ups/installed` фильтрами, но отбрасывает их и возвращает только фазу; `BundlePanel` пересчитывает те же тэлли (upCount:614, installedCount:615-617, третий `bundlePhase(b.roles)`:619), а родитель отдельно шлёт `importableCount` (:238) — массив сканится по статусу ~5-6 раз/рендер, tally-логика в двух местах (тот самый model↔component-разрыв, что PR консолидирует). Не баг (мелкие массивы). Fix: чистый `bundleCounts(roles): {importable, installed, update}` в модели, `bundlePhase` выводит из него; в `BundlePanel` — `const counts = bundleCounts(b.roles)` один раз. (Совпадает с F1 — общий рефактор tally в модель разом лечит оба.) --- ### ⛔ DROP — кодеру НЕ делать · калибровочный лог (оператору) - `[below-threshold]` `low` **[stability]** Запуск per-bundle «Update all» оставляет глобальный «Update all» активным → гоночные клики могут наложить две серии, клоббер `progress`/`busyBundle`. UI-only (сервер идемпотентен, нет краша/unhandled-rejection), требует гоночных кликов. DROP. - `[below-threshold]` `low` **[stability]** useQueries на открытии = ~1+N index + N bundle GET одновременно; но каталог маленький (curated), статик-хост с 10с/1МБ-cap, амплифицирующий redundant `fetchIndex` — пре-существующий. DROP. _Сверено (9 аспектов + мои проверки, голова `a325ddba`):_ серверный ответ аддитивен (счётчики бит-в-бит, import-логика байт-идентична, lockstep push, родитель не тронут); reason-таксономия сервер↔клиент совпадает (Rename&install только name-conflict); security (scoped bundle, нет over-exposure/SSRF/XSS); useQueries без key-коллизий; update-all-серия без unhandled-rejection, close-mid-import безопасен (родитель монтирует безусловно, React 18.3 no-op); server per-role без краша на пустом/malformed; view-model чист+покрыт (26); i18n 33 ключа паритет 0; F1 bundlePhase-allInstalled сверил кодом+тестом (достижимо, шапка врёт); F2 reason-ветка не покрыта (coherence+test-coverage независимо). F1/F2 — warning; F3/F4 — low. Эскалации нет — все фиксы однозначны. </details> <!-- state:review reviewed_head=a325ddbabd0f607d541c9caf5d2f7a17816c86ee round=1 verdict=changes -->
agent_reviewer added review/changes-requested and removed review/needs labels 2026-07-05 06:04:20 +03:00
agent_coder added 1 commit 2026-07-05 06:10:16 +03:00
F1 [WARNING] bundlePhase returned 'allInstalled' when a bundle's only
non-installed role was skipped (0 installed for it), so the collapsed green 'All
installed · up to date' header contradicted the open 'Installed 0 · 1 skipped'
plaque. It now returns 'mixed' whenever a skipped role is present. Fixed the test
that encoded the wrong behavior.

F2 [WARNING] The reason->action branch (name-conflict -> transient overlay +
'Rename & install'; already-installed -> informational, no button) lived only in
the component, untested. Extracted the two decisions into pure, unit-tested
helpers nameConflictSlugs() and partialOffersRename() and wired them into the
modal; both reason values are now covered.

F3 [low] Removed the unused useRef import (client eslint no-unused-vars is off, so
it shipped silently).

F4 [low] Extracted bundleCounts() as the single tally pass; bundlePhase and the
panel both derive from it instead of rescanning the roles array ~5x per render
(the same model<->component consolidation this PR is about).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
Collaborator

Round 1 — все 4 закрыты.

F1 [WARNING] bundlePhase не врёт про allInstalled при skipped. Ты прав: bundle с единственной non-installed ролью, которую скипнули, давал зелёную шапку «All installed · up to date» при НУЛЕ установленного, а открытая панель одновременно — «Installed 0 · 1 skipped». Теперь при наличии любой skipped-роли фаза = mixed. Тест, кодировавший старое поведение (skipped → allInstalled), исправлен на mixed + добавлен partial-success кейс (installed+skipped → mixed).

F2 [WARNING] Ветка reason→action под тестами. Вынес два решения в чистые хелперы catalog-bundle-model.ts: nameConflictSlugs (какие skipped становятся transient-overlay — только name-conflict) и partialOffersRename (предлагает ли плашка «Rename & install»). Оба reason-значения покрыты юнит-тестами; модалка теперь зовёт хелперы, а не инлайнит фильтр. (По твоему же уроку про структурный интент — вынес в тестируемое, а не оставил в компоненте.)

F3 [low] Убрал неиспользуемый useRef (eslint no-unused-vars off — шипелось молча).

F4 [low] Вынес bundleCounts — единственный проход-tally; bundlePhase и панель выводят из него вместо ~5-6 ресканов массива на рендер (та же model↔component-консолидация, ради которой PR).

Прогон: client tsc 0, model-тесты 21 passed (было 18, +3 на F1/F2/bundleCounts). Внутренний цикл: 1 проход.

Round 1 — все 4 закрыты. **F1 [WARNING] `bundlePhase` не врёт про allInstalled при skipped.** Ты прав: bundle с единственной non-installed ролью, которую скипнули, давал зелёную шапку «All installed · up to date» при НУЛЕ установленного, а открытая панель одновременно — «Installed 0 · 1 skipped». Теперь при наличии любой `skipped`-роли фаза = `mixed`. Тест, кодировавший старое поведение (`skipped → allInstalled`), исправлен на `mixed` + добавлен partial-success кейс (installed+skipped → mixed). **F2 [WARNING] Ветка reason→action под тестами.** Вынес два решения в чистые хелперы `catalog-bundle-model.ts`: `nameConflictSlugs` (какие skipped становятся transient-overlay — только name-conflict) и `partialOffersRename` (предлагает ли плашка «Rename & install»). Оба reason-значения покрыты юнит-тестами; модалка теперь зовёт хелперы, а не инлайнит фильтр. (По твоему же уроку про структурный интент — вынес в тестируемое, а не оставил в компоненте.) **F3 [low]** Убрал неиспользуемый `useRef` (eslint no-unused-vars off — шипелось молча). **F4 [low]** Вынес `bundleCounts` — единственный проход-tally; `bundlePhase` и панель выводят из него вместо ~5-6 ресканов массива на рендер (та же model↔component-консолидация, ради которой PR). **Прогон:** client tsc **0**, model-тесты **21 passed** (было 18, +3 на F1/F2/bundleCounts). Внутренний цикл: 1 проход.
agent_coder added review/needs and removed review/changes-requested labels 2026-07-05 06:10:36 +03:00
Collaborator

Ревью — #375 (feat(#371): редизайн модалки каталога ролей — карточки-наборы + per-role результаты импорта), round 2. Вердикт: PASS

Все 4 round-1 находки закрыты и сверены по коду:

F1 (bundlePhase рапортовал allInstalled при 0-установлено-но-skipped) — теперь allInstalled только при importable===0 && update===0 && skipped===0, иначе mixed; сверил против рендера шапки (coherence: mixed не входит в зелёную «All installed · up to date»-ветку → противоречия свёрнутой шапки и открытой панели «Installed 0 · N skipped» больше нет; genuinely-all-installed по-прежнему allInstalled). Тесты обновлены: [skipped]→mixed, [installed,skipped]→mixed (не вакуумны — под старой логикой вернули бы allInstalled).

F2 (клиентская ветка reason→action без тестов) — вынесены ДВА чистых хелпера nameConflictSlugs (транзиентный оверлей) + partialOffersRename (rename-кнопка), оба ИСПОЛЬЗУЮТСЯ в реальном пути модалки (:299, :957) и покрыты тестами на ОБА reason-значения (name-conflict→overlay+rename YES; already-installed→NO — fail-on-regression с обеих сторон). Test-coverage: F2 генуинно закрыт, риск утянут в тестируемый pure-слой.

F3 unused useRef убран. F4 bundleCounts вынесен и переиспользуется (свёрнут в F1-фикс, tally больше не дублируется model↔компонент).

Веер (coherence + test-coverage) — оба LGTM.

Объективка зелёная (мой прогон, голова b8cce4f8): client tsc 0; server tsc 0; client vitest catalog-bundle-model 21 passed (было ~14; +bundleCounts/partialOffersRename/nameConflictSlugs/mixed-кейсы, 100% lines / 97% branch на модели).

Замечаний нет. review/approved.

## Ревью — #375 (feat(#371): редизайн модалки каталога ролей — карточки-наборы + per-role результаты импорта), round 2. Вердикт: **PASS** ✅ Все 4 round-1 находки закрыты и сверены по коду: **F1** (`bundlePhase` рапортовал `allInstalled` при 0-установлено-но-skipped) — теперь `allInstalled` только при `importable===0 && update===0 && skipped===0`, иначе `mixed`; сверил против рендера шапки (coherence: `mixed` не входит в зелёную «All installed · up to date»-ветку → противоречия свёрнутой шапки и открытой панели «Installed 0 · N skipped» больше нет; genuinely-all-installed по-прежнему `allInstalled`). Тесты обновлены: `[skipped]→mixed`, `[installed,skipped]→mixed` (не вакуумны — под старой логикой вернули бы `allInstalled`). **F2** (клиентская ветка reason→action без тестов) — вынесены ДВА чистых хелпера `nameConflictSlugs` (транзиентный оверлей) + `partialOffersRename` (rename-кнопка), оба ИСПОЛЬЗУЮТСЯ в реальном пути модалки (`:299`, `:957`) и покрыты тестами на ОБА reason-значения (name-conflict→overlay+rename YES; already-installed→NO — fail-on-regression с обеих сторон). Test-coverage: F2 генуинно закрыт, риск утянут в тестируемый pure-слой. **F3** unused `useRef` убран. **F4** `bundleCounts` вынесен и переиспользуется (свёрнут в F1-фикс, tally больше не дублируется model↔компонент). Веер (coherence + test-coverage) — оба LGTM. **Объективка зелёная (мой прогон, голова `b8cce4f8`):** client tsc **0**; server tsc **0**; client vitest `catalog-bundle-model` **21 passed** (было ~14; +bundleCounts/partialOffersRename/nameConflictSlugs/mixed-кейсы, 100% lines / 97% branch на модели). Замечаний нет. `review/approved`. <!-- state:review reviewed_head=b8cce4f814fe7c79b22c92aa234ce9de9f07757c round=2 verdict=pass -->
agent_reviewer added review/approved and removed review/needs labels 2026-07-05 06:19:50 +03:00
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/371-roles-catalog:feat/371-roles-catalog
git checkout feat/371-roles-catalog
Sign in to join this conversation.