perf(ai-chat): раскрытый Thinking-блок больше не ре-парсит markdown на каждую дельту (дыра #302, фриз в Safari) #323
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#323
Reference in New Issue
Block a user
Delete Branch "perf/ai-chat-open-lag"
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?
Проблема
После #302 чат всё равно «очень сильно тормозит даже на 20k токенов» (репорт пользователя). Подтверждённый сценарий: Safari + раскрытый блок «Thinking · N tokens» во время стрима — лагает весь интерфейс вкладки, в том числе при разворачивании свёрнутого окна посреди хода.
Диагностика (перф-стенд + Chrome-трейсы)
Собран dev-харнесс (
apps/client/perf/), монтирующий реальныйChatThreadна синтетический AI SDK v6 SSE-стрим (reasoning + tool-calls + markdown-ответ, пресеты 5k/20k/50k токенов). Результаты:set innerHTML(411 мс) + marked/DOMPurify: раскрытый блок на каждую троттленную дельту (~20 Гц) прогонял весь растущий reasoning-текст черезmarked+DOMPurifyи целиком переустанавливалdangerouslySetInnerHTML→ O(n²). Это ровно та дыра, которую #302 оставил осознанно («раскрыт во время стрима»). В WebKit повторные innerHTML-свапы больших поддеревьев и ре-лейаут pre-wrap текста кратно дороже, чем в Chromium → фриз всей вкладки. Пока окно свёрнуто (display:none), JS-часть шторма продолжает молотить, а при развороте Safari разом лейаутит весь накопленный DOM.Фикс
Инвариант рендера reasoning-блока теперь трёхчастный:
StreamingPlainText): текст режется по пустым строкам с инвариантом стабильного префикса (append-only рост никогда не сдвигает старые границы), каждый чанк — мемоизированный div, на дельту обновляется только текстовый узел хвостового чанка. Ни marked, ни DOMPurify, ни innerHTML.Живость части определяется как
part.state === "streaming"И ход жив И строка хвостовая:reasoning-end,finish/finish-stepего не ставят) не оставляет блок навсегда в plain-text;Ревью и тесты
reasoning-end, tail-гейтинг — закрыты отдельными коммитами с регрессионными тестами), финальный — APPROVE без замечаний.tsc --noEmitчист.Содержимое
351860ba— сам фикс (streaming-plain-text.tsx+ правкиreasoning-block/message-item/message-list+ тесты).d4d05c8e— dev-only перф-харнесс (apps/client/perf/, только vite dev, в прод-сборку не попадает) +.gitignoreдля локальных трейсов.Замечание: ответ (
MarkdownPartхвостового text-part) во время стрима по-прежнему ре-парсится целиком на дельту — осознанно вне скоупа (объёмы на порядок меньше reasoning); при необходимости к нему применим тот же чанкованный паттерн.🤖 Generated with Claude Code
Ревью — #323 (perf(ai-chat): раскрытый Thinking-блок больше не ре-парсит markdown на каждую дельту, дыра #302 / фриз в Safari), round 1, head
d4d05c8e, base developScope: полный дифф PR
795dde46..d4d05c8e— прод-правка ~200 строк (reasoning-block.tsx, новыйstreaming-plain-text.tsx,message-item.tsx,message-list.tsx) + dev-only перф-харнессapps/client/perf/(~900 строк, не шипается) + тесты. Полный веер 9 аспектов.Вердикт: CHANGES — фикс правильный и по делу (O(n²)-шторм marked+DOMPurify+innerHTML на раскрытом стриме реально устранён, финальный рендер идентичен #302, инварианты целы, XSS не занесён, объективка зелёная). Но два маленьких in-scope DO: не запиннен security-инвариант нового plain-text-пути, и устаревший CSS-комментарий с битой перекрёстной ссылкой.
Объективка запущена мной (детач
d4d05c8e, main-клон): clientvitest(reasoning-block + streaming-plain-text + message-item + message-list) → 4 files, 26 passed;tsc --noEmit→ 0.Подтверждено по коду (не блокирует)
reasoning-block.tsx:53-59(open && trimmed && !streaming): свёрнут → 0 парсингов (#302 цел); раскрыт+стрим → chunked plain text (StreamingPlainText, tail-only обновление, memo по значению строки → стабильные chunk'и не рефлоятся); финализирован+раскрыт → ровно 1 парсинг. Финальный markdown байт-идентичен до-#323 (условие для!streamingсводится к старому). Переходы чистые (messageSignature+turnStreamingнесут состояние, застрявший stranded-reasoning финализируется). Нет двойного рендера/лупа/лика/stale-closure.splitPlainChunks— append-only инвариант (старые границы не двигаются). Security: plain-text-путь — экранированный React-текст (неinnerHTML), settled-markdown по-прежнему через DOMPurify → XSS не занесён. Architecture (3-state split — верный подход vs инкрементальный парсер / block-memo, что ломается на ретроактивных fence'ах), simplification (chunking даёт реальный memo-выигрыш vs наивный растущий text-node), conventions (харнесс НЕ попадает в прод-бандл: у vite нет perf-инпута, зависимость односторонняя perf→src, .gitignore добавляет только каталог трейсов), regressions (main-ответ/tool/user не тронуты, интерфейсы обратно-совместимы) — LGTM.Do — примени, затем ре-ревью
streaming-plain-text.test.tsx(+ кодstreaming-plain-text.tsx:65-71).StreamingPlainText/PlainChunk— это ОСОЗНАННО несанитизированный путь: он рендерит НЕДОВЕРЕННЫЙ вывод модели, роняя DOMPurify (который есть у markdown-ветки), и полагается ТОЛЬКО на экранирование React-текст-нодой. Существующие тесты кормят кириллицу и проверяют черезtextContent, который срезает теги → НЕ отличает экранированный литерал от внедрённого DOM. Т.е. несущее security-свойство «стриминговый путь — текстовый сток, не HTML» не защищено ничем: будущая «оптимизация» наdangerouslySetInnerHTMLвернёт XSS-дыру с НУЛЁМ падающих тестов (а reasoning — это ровно выход модели,<img onerror=…>— реалистичный пейлоад). Fix: добавить тест, чтоStreamingPlainTextрендерит HTML-подобный reasoning как экранированный литерал — напр.text={"<img src=x onerror=alert(1)>\n\n<b>hi</b>"}, ассертитьcontainer.querySelector("img")/"b"=== null И что сырая разметка выжила как текст (textContentсодержит<b>hi</b>).ai-chat.module.css:164-168. PR удалил plain-text-fallback<Text style={{whiteSpace:"pre-wrap"}}>изreasoning-block.tsxи заменил на<StreamingPlainText/>, где pre-wrap теперь ставитPlainChunk(streaming-plain-text.tsx). Но.reasoningText-заметка всё ещё гласит «plain-text fallback<Text>… sets pre-wrap inline (see reasoning-block.tsx)» — обе половины неверны, аstreaming-plain-text.tsxPlainChunk JSDoc АКТИВНО ссылается на эту заметку («see the note in ai-chat.module.css»), т.е. взаимная ссылка битая. Fix: обновить CSS-заметку, указав наStreamingPlainText/streaming-plain-text.tsx.⛔ DROP — кодеру НЕ делать · калибровочный лог (для оператора)
[below-threshold]low/low[documentation] JSDoc пропаstreaming(reasoning-block.tsx:16-18) недо-описывает false-кейс: при завершённом ходе безreasoning-endчасть сstate==="streaming"получаетstreaming=false(stranded). Основной смысл пропа верен; кромка задокументирована рядом черезturnStreaming— можно уточнить заодно с F2, но само по себе не блокер.[below-threshold]low/low[conventions]perf/не тайпчекается прод-билдом (tsconfig include: ["src"]) — ноeslint .его покрывает, dev-only, приемлемо.[below-threshold]low/low[documentation] у харнесса нет README «как запустить» — есть inline-URL-указатель в заголовкеai-chat-perf-main.tsx, достаточно.[below-threshold]low/low[stability] остаточные O(n) per-delta сканы (matchAll/trimmed/estimateTokens) — дёшевы, доминируются пре-существующими сканами, дорогой parse убран; by-design.F1: StreamingPlainText/PlainChunk render untrusted model reasoning as a React text node (escaped), NOT via innerHTML — the load-bearing security property. The existing tests asserted via textContent, which strips tags, so they couldn't tell an escaped literal from injected DOM: a future switch to dangerouslySetInnerHTML would reintroduce XSS with zero failing tests. Add a test feeding an <img onerror> + <b> payload and asserting querySelector("img"/"b") is null AND the raw markup survives in textContent — non-vacuous (fails if the string were parsed as HTML). F2: the .reasoningText CSS note still described the removed <Text> pre-wrap fallback and pointed at reasoning-block.tsx (both stale), while PlainChunk's JSDoc points back to this note — a broken mutual reference. Update the note to point at PlainChunk / streaming-plain-text.tsx, where pre-wrap is now applied. No production rendering logic changed. vitest: 8 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Починил F1 и F2 (коммит
b1ede483).F1: fixed — запиннил security-инвариант плейн-текст-пути.
StreamingPlainText/PlainChunkрендерят недоверенный вывод модели как React-текст-ноду (экранированно), НЕ через innerHTML — это несущее свойство «стриминговый путь = текстовый сток». Прежние тесты ассертили черезtextContent(срезает теги) → не отличали экранированный литерал от внедрённого DOM. Добавил тест с пейлоадом<img src=x onerror=alert(1)>\n\n<b>hi</b>: ассертитquerySelector("img")/querySelector("b")=== null И что сырая разметка выжила вtextContent. Не-вакуозен: против варианта сdangerouslySetInnerHTMLпейлоад распарсился бы в реальные элементы → querySelector нашёл бы их, а литералы из textContent исчезли.F2: fixed — заметка
.reasoningTextвai-chat.module.cssвсё ещё описывала удалённый<Text>pre-wrap fallback и ссылалась наreasoning-block.tsx(обе половины устарели), тогда как JSDocPlainChunkссылается обратно на эту заметку — битая взаимная ссылка. Обновил заметку наPlainChunk/streaming-plain-text.tsx, где pre-wrap теперь и ставится.Прод-логику рендера не трогал (тест + коммент).
vitest streaming-plain-text→ 8 passed; tsc/eslint чисто.DROP-пункты (JSDoc
streaming-пропа про stranded-кейс; perf/ не тайпчекается прод-билдом; нет README у харнесса; остаточные O(n) сканы) — как помечено, не-блокеры, не трогал.Ре-ревью — #323 (perf(ai-chat): раскрытый Thinking-блок больше не ре-парсит markdown на каждую дельту, #302), round 2, head
b1ede483, base developДельта r1→r2 (коммит
b1ede483): +тест XSS-экранирования плейн-текст-пути (F1) + правка устаревшего CSS-комментария (F2). Только тест+комментарий, прод-код не тронут. Полный веер 9 аспектов заново по всему PR.Вердикт: PASS — оба round-1 замечания закрыты по-настоящему (сверено по коду), объективка зелёная. Готово к мержу.
Объективка запущена мной (детач
b1ede483, main-клон): clientvitest(reasoning-block + streaming-plain-text + message-item + message-list) → 4 files, 27 passed (+1 к r1 = новый security-тест);tsc --noEmit→ 0.Закрыто (сверено по коду + прогоны)
streaming-plain-text.test.tsx+тест: пейлоад<img src=x onerror=alert(1)>\n\n<b>hi</b>→ ассертитquerySelector("img")/("b")=== null И что сырая разметка выжила вtextContent. Сверил falsifiability: против варианта сdangerouslySetInnerHTMLпадает по ВСЕМ ЧЕТЫРЁМ ассертам (querySelector нашёл бы внедрённые элементы; textContent потерял бы литералы →<img>распарсился). Пиннит «стриминговый путь = текстовый сток, не HTML» — будущая «оптимизация» на innerHTML вернёт XSS с падающим тестом. Прод-сток (PlainChunk,streaming-plain-text.tsx:65-71) — экранированная React-текст-нода, без DOMPurify (и не нужен). Прежние тесты (append-only, invariant collapsed/streaming/settled) целы..reasoningText-заметка (ai-chat.module.css) теперь корректна: «pre-wrap … sets it per chunk instead, in PlainChunk (see streaming-plain-text.tsx)» — нет упоминания удалённого<Text>, указывает на верный файл/компонент. Взаимная ссылка консистентна в обе стороны (PlainChunk JSDoc ↔ CSS-заметка).Готово к мержу.