fix(#300 ui): различимые per-agent цвета глифа + аватар пользователя на переднем плане #319
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#319
Reference in New Issue
Block a user
Delete Branch "feat/300-avatar-colors"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Проблема
После мержа #307 у всех агентов кружок глифа выходил одинаково фиолетовым, и аватар пользователя был наполовину спрятан за кружком агента.
Причины
Avatar variant="filled"с override черезstyles, но Mantine перебивал фон своей переменной--avatar-bg→ для каждого агента падал дефолтный primary-цвет темы (фиолет). Плюс «сырой»hue = hash % 360клал многие имена в один «фиолетово-маджентовый» сектор (напр. «Структурный редактор» → 287°, «Фактчекер» → 331° — оба фиолетовые на глаз).zIndex:0, из-за чего в правом верхнем углу была видна лишь обрезанная кромка головы.Решение (только клиент, 1 файл + тест)
Boxс явнымbackground— без борьбы с--avatar-bgMantine. Цвет применяется всегда.IconSparklesостаются читаемыми. Со скрина: «Структурный редактор» → фиолет, «Фактчекер» → оранжевый.zIndexлончера поднят выше глифа — теперь это полноценный бейдж в правом верхнем углу, не спрятанный за агентом.Верификация
tsc --noEmit— 0 ошибок.vitest(agent-avatar-stack + comment-list-item) — зелёные; в тестagentGlyphBackgroundдобавлена проверка, что «Структурный редактор» и «Фактчекер» дают РАЗНЫЕ цвета (та самая пара со скрина).Follow-up к #307 / #304, ref #300.
Ревью — #319 (fix(#300 ui): различимые per-agent цвета глифа + аватар пользователя на переднем плане), round 1, head
344b9723, base developScope: полный дифф PR
33d22ff1..344b9723— 1 компонент + тест (agent-avatar-stack.tsx+52/−34,.test.tsx+11/−0), 153 строки. Клиент-only UI. Полный веер 9 аспектов.Вердикт: CHANGES — фикс по сути правильный и целен (цвет теперь гарантированно применяется, палитра различимая и детерминированная, z-order лаунчера верный, регрессий нет), объективка зелёная. Но 2 маленьких in-scope DO: главный баг (что цвет реально доходит до DOM) не запиннен тестом, и топ-level JSDoc + два заголовка теста описывают СТАРЫЙ z-order.
Объективка запущена мной (детач
344b9723, main-клон): clientvitest agent-avatar-stack→ 8 passed;tsc --noEmit→ 0.Подтверждено по коду (не блокирует)
Boxс inlinebackground: agentGlyphBackground(name)(:97-108) вместоAvatar variant="filled"— inline-стиль не перебивается Mantine--avatar-bg, цвет применяется всегда. Палитра — 14 категориально-разных тёмных цветов, выборhashName(name) % 14; детерминирован и в границах (Math.abs, INT_MIN-угол безопасен — сверено stability). Лаунчер (человек)zIndex:2над глифомzIndex:1(:214/:228) — теперь на переднем плане.Boxсохраняет 38px-круг/скругление/центрирование/белый глиф;avatarUrlпо-прежнему черезCustomAvatar; интерфейсAgentInfo/props не менялся, оба потребителя (history-item.tsx:103,comment-list-item.tsx:143) не затронуты. Security чист (имя — только индекс массива, не интерполируется в стиль; нет HTML-sink/секретов). Architecture/conventions/simplification LGTM (палитра-по-хешу — устоявшийся паттернcustom-avatar/label-colors; мёртвого кода после Avatar→Box нет).Do — примени, затем ре-ревью
agent-avatar-stack.test.tsx(render-блок:49-131), фикс под тестомagent-avatar-stack.tsx:96-118. Весь баг был в том, что цвет НЕ применялся (Mantine--avatar-bgперебивал), — самаagentGlyphBackgroundи до фикса возвращала разные цвета. Юнит-тесты покрывают только чистую функцию (детерминизм + различность на границе функции), а НИ ОДИН render-тест не читает фон глифа (grepпоbackground/style/getComputedStyleв тесте — пусто). Значит откат к багованномуAvatar(или хеш-константа) оставил бы весь сьют зелёным — риск-критичный путь «цвет реально в DOM» не защищён. Дёшево в jsdom (фикс использует INLINEstyle.background, jsdom отдаёт его дословно). Fix: добавить render-ассерт — отрендерить стек для emoji-агента и проверить, что inlinebackgroundконтейнера глифа ===agentGlyphBackground(agent.name)(падает против до-фиксногоAvatar); в идеале ещё два агента с палитро-разными именами → два РАЗНЫХ применённых фона в DOM.agent-avatar-stack.tsx:136-138(JSDocAgentAvatarStack) + заголовки тестовagent-avatar-stack.test.tsx:50и:97. JSDoc всё ещё гласит «the AGENT glyph in front … the HUMAN who launched it … offset behind», но PR перенёс лаунчера (человека) на передний план (zIndex:2> глифzIndex:1); соседние z-order-комментарии (:28-29,:212-213) кодер обновил, а этот топ-level JSDoc — нет. Заголовки тестов:50(«human launcher behind») и:97(«agent avatar in front, NO launcher behind») тоже описывают старый порядок. Fix: привести JSDoc и оба заголовка в соответствие с новым z-order (человек-лаунчер спереди).⛔ DROP — кодеру НЕ делать · калибровочный лог (для оператора)
[below-threshold]low/low[conventions]agentGlyphBackgroundэкспортирован и юнит-тестирован — мог бы жить вfeatures/*/utils/*-color.ts(какrole-card-color.ts/label-colors.ts), но прямой соседcustom-avatar.tsxдержит палитру in-component, так что оставить здесь — легитимный выбор, не нарушение.Починил F1 и F2 (коммит
13a33363).F1: fixed — добавил render-тесты, пиннящие, что цвет реально доходит до DOM (весь баг был именно в этом — Mantine
--avatar-bgперебивал inline-фон; самаagentGlyphBackgroundи до фикса возвращала разные цвета, поэтому зелёный сьют не ловил регресс). Повесилdata-testid="agent-glyph"на глиф-Boxи добавил два теста: (1) применённый inlinebackgroundглифа emoji-агента ===agentGlyphBackground(name); (2) два палитро-различных агента дают в DOM РАЗНЫЕ фоны. Нюанс: React применяет стили через CSSOM (hsl→rgb), поэтому сравниваю обе стороны через один и тот же нормализатор против реального вывода функции (не замороженный литерал). Против до-фиксногоAvatar(нет inline-фона / нет testid) — падает.F2: fixed — привёл в соответствие устаревшее описание z-order: top-level JSDoc
AgentAvatarStackи два заголовка тестов гласили «глиф агента спереди, человек сзади», хотя PR перевернул порядок (лаунчер-человек теперь спереди,zIndex:2 > 1). Обновил JSDoc + оба заголовка.vitest→ 10 passed (+2 новых render-теста); tsc по файлу чисто.DROP-пункт (вынести
agentGlyphBackgroundв отдельный util) — как помечено, не трогал.Ре-ревью — #319 (различимые per-agent цвета глифа + аватар пользователя на переднем плане, #300), round 2, head
13a33363, base developДельта r1→r2 (коммит
13a33363):data-testid="agent-glyph"на глиф-Box, два DOM-фон render-теста + нормализатор, правка устаревших z-order описаний. Полный веер 9 аспектов заново по всему PR (33d22ff1..13a33363, 2 файла, +137).Вердикт: PASS — оба round-1 замечания закрыты по-настоящему (сверено по коду), объективка зелёная. Готово к мержу.
Объективка запущена мной (детач
13a33363, main-клон): clientvitest agent-avatar-stack→ 10 passed;tsc --noEmit→ 0.Закрыто (сверено по коду + прогоны)
data-testid="agent-glyph"на тот самый outer-Box, что несётbackground: agentGlyphBackground(name)(:98/:103), + два render-теста. Сверил вакуозность построчно: (1)emoji glyph applies its per-agent color(test:76-97) — ЛЕВАЯ сторонаglyph.style.backgroundчитается из РЕАЛЬНОГО смонтированного DOM-элемента (querySelector('[data-testid="agent-glyph"]')), ПРАВАЯ —normalizeColor(agentGlyphBackground(name))(вывод функции через тот же CSSOM hsl→rgb нормализатор). Это НЕ тавтологияnormalize(fn)===normalize(fn): нормализуется только функц-сторона, DOM-сторона читается сырой. Гард:93not.toBe("")превращает патологический jsdom-случай в красный, не в ложно-зелёный. Против до-фиксногоAvatar(нет inline-фона, нет testid) —querySelector→null→not.toBeNull()падает. (2) distinctness (test:99-131) читает ОБА фона из DOM и сверяет различие; хеши сверены независимо (Researcher→8 sky,Нарратор→9 blue — разные записи палитры). Риск-критичный путь «цвет реально в DOM» теперь защищён.AgentAvatarStack(:137-141, теперь «launcher … in FRONT, zIndex 2 > glyph zIndex 1») и два заголовка тестов (:60,:164). Соседние z-order-комментарии (:28-29,:215-216) уже были верны.Boxсохраняет 38px-круг/центрирование/белый глиф;hashName%14тотален/детерминирован/в границах (INT_MIN-угол безопасен);data-testidинертен (не меняет рендер/ветку); z-order лаунчер(2)>глиф(1) без окклюзии; потребители (history-item,comment-list-item) не затронуты. Security (имя — только индекс палитры, не в стиль-строку; нет HTML-sink/секретов), architecture (палитра-по-хешу = паттернcustom-avatar), conventions (data-testid= паттернcomment-hover-preview), simplification, stability, coherence(самого фикса) — LGTM.⛔ DROP — кодеру НЕ делать · калибровочный лог (для оператора)
Три пограничных находки веера, отклонены (с обоснованием — чтобы ты мог свериться с моей планкой):
[below-threshold]low/low[coherence/documentation] остаточные упоминания «human behind» — интерфейсный хедер:13-14(«BEHIND identity (the human)»),:124,:184, тест:174. НЕ считаю их незакрытым F2: (а) это НЕ изменённые в PR строки, и на round-1 их смотрели —:13-15был явно оценён как identity-provenance (#300: агент-актор vs фоновый человек-запускатель, «computed server-side, internal-vs-MCP provenance»), а НЕ z-order отрисовки; (б):124/:184/:174«no human behind» = про ОТСУТСТВИЕ второй (человеческой) идентичности, не про пиксельный порядок. Реальные z-order-описания (JSDoc+заголовки) кодер починил. Пере-флажить неизменённые строки, что я пропустил на r1, = двигать ворота (вредит сходимости). Если хочешь единообразия формулировок — скажи, заведу отдельно, но как блокер не держу.[below-threshold]low/low[simplification]test:123-128дублируют exact-match ассерт теста 1 (дляResearcher) перед своей реальной проверкой различия (:130) — избыточно, но НЕ неверно; тест корректен, автор мог оставить для явности.[style/linter]low/low[conventions]container.querySelector('[data-testid]')+glyph!вместоscreen.getByTestId— но файл УЖЕ используетquerySelector(.tabler-icon-sparkles, пре-существующе), локально консистентно.