perf(mcp): content-addressed LRU-кэш конверсии getPage — доминирующая агентская нагрузка (#479) #480
Reference in New Issue
Block a user
Delete Branch "perf/479-getpage-cache"
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?
Summary
getPage— доминирующая операция агентского цикла (по метрикам #402: 812 вызовов за 2ч, p95 840 мс): полный обход ProseMirror-дереваconvertProseMirrorToMarkdownна КАЖДЫЙ вызов, кэша нет. При 812 read против 28 update подавляющее большинство — повторная конверсия того же неизменившегося контента в тот же Markdown, на общем event loop. closes #479Стоит на develop напрямую (#450 распил client.ts уже влит; getPage живёт в
client/read.ts).1. Content-addressed LRU-кэш конверсии
getpage-cache.ts— LRU класс (не синглтон). Ключ(canonical pageId UUID, updatedAt, optionsHash).updatedAtиз ТОГО ЖЕ ответа/pages/info, что иcontent→ инвалидация бесплатная и точная (страница изменилась → новый ключ, рассинхрон невозможен).optionsHash— стабильная сериализация опций конверсии (dropResolvedCommentAnchors#328);getPageиexportPageMarkdownне коллизят (export кэш не трогает).Buffer.byteLength), вытеснение по любому пределу; oversized-запись хранится, кэш не заклинивает.protectedинстанс-поле вcontext.ts(одинDocmostClientна сессию/идентичность) → изоляция как у самого клиента; межпользовательской утечки контента нет.{{SUBPAGES}}; подстановка на живых subpages выполняется на hit и miss одинаково → финальный результат тот же.2. Счётчики hit/miss
Через существующий
onMetricFn-синк:mcp_getpage_cache_hits_total/mcp_getpage_cache_misses_total(honest hit-rate — miss на реальной конверсии, включая non-cacheable путь).3. Subpages — НЕ параллелизуемы (расследовано)
listSidebarPagesтребуетspaceIdв теле запроса, аspaceIdдоступен только ИЗ ответа page-fetch (resolvePageIdдаёт UUID, но не spaceId).Promise.allневозможен без выдумывания spaceId → последовательность сохранена (задокументировано в коде). Кэш конверсии — основной выигрыш (повторное чтение пропускает дорогой CPU-обход).How verified
prosemirror-markdowntsc чисто;mcptsc чисто;node --test— 813/813 (было 812; +1 дифф-тест).updatedAt; miss после смены опций; вытеснение по count И по byte; recency-refresh; re-set без двойного счёта байт; oversized хранится; hash order-insensitive.updatedAt→свежий MISS с новым контентом (без stale-hit); slugId+UUID делят одну запись; дифф-тест: при HIT конверсии смена списка subpages между чтениями отражается в выводе (живая подстановка{{SUBPAGES}}не заморожена).Checklist
Стоит на develop напрямую (распил client.ts #450 уже влит).
Что: content-addressed LRU-кэш конверсии PM→Markdown в getPage (доминирующая нагрузка — 812 вызовов/2ч, p95 840мс). Ключ (pageId, updatedAt, optionsHash), границы по count+bytes, счётчики hit/miss. Кэш per-instance (нет межпользовательской утечки), вывод байт-идентичен (кэшируется до подстановки {{SUBPAGES}}). Subpages параллелить нельзя — нужен spaceId из page-fetch (задокументировал).
Проверка на стенде: mcp tsc чисто, node --test 813/813.
Внутренний цикл: 2 прохода. Первый (адверсарно, фокус на утечку контента между юзерами и байт-идентичность) — кэш строго per-instance, вывод идентичен, инвалидация точная, границы и счётчики корректны; блокеров нет. Второй (полиш): добавил дифф-тест живой подстановки subpages на hit (пиннит байт-идентичность против будущего рефактора) + поймал закладку — разделитель ключа был 3 литеральных NUL-байта (читались как пробел), вычистил, поставил явный пробел с точным комментарием.
Ревью #480 — LRU-кэш конверсии getPage. Вердикт: changes-requested
Дизайн кэша корректен и безопасен — крутить его стоит, ядро в порядке. Content-addressing полный, межпользовательской утечки нет, вывод байт-идентичен. Но два пункта не дотянуты: заявленные счётчики hit/miss фактически мертвы (дропаются на синке), и нет теста на то, что хит реально пропускает конверсию — то самое, ради чего PR и существует.
Открыто: F1 (метрики hit/miss никуда не пишутся), F2 (нет теста, что хит пропускает CPU-обход).
✅ Что проверил (объективка + верификация)
prosemirror-markdown+mcptsc чисто;node --test test/— 814/814; два новых спека (unit+mock) — 13/13.contentиupdatedAt— из ОДНОГО ответаgetPageRaw//pages/info(read.ts:404), ключ по канонический UUIDresultData.id. Правка страницы → новыйupdatedAt→ новый ключ. Стейл не отдаётся.{{SUBPAGES}}-подстановки (read.ts:442); подстановка на живых subpages идёт на обоих путях (467-476). Mock-тест меняет subpages A→B под фиксированнымupdatedAt— на хите список B, а не замороженный A. ✓getPageCache—protectedинстанс-поле (context.ts:168), одинDocmostClientна MCP-сессию/идентичность (index.ts:203, http.ts:150). Тот же паттерн, чтоpageIdCache/collabTokenCache. ✓/pages/info; потерявший доступ получает 403 до кэша. Пропускается только CPU-обход. ✓dropResolvedCommentAnchorsветвится поmark.attrs.resolvedВНУТРИ контента, а резолв коммента бампаетupdatedAt→ ключ меняется. Content-addressing полный (coherence). ✓Buffer.byteLength,totalBytesточен на set/re-set/evict, oversized хранится без заклинивания (size>1-гард). Регресс/экспорт: miss байт-идентичен старому пути,exportPageMarkdownкэш не трогает, брошенная конверсия не кэшируется. ✓🔧 Сделать, потом ре-ревью
F1 [observability] Счётчики hit/miss эмитятся, но молча выбрасываются на синке — фича «honest hit-rate» не работает.
read.ts:439,445эмитятmcp_getpage_cache_hits_total/_misses_total, но колбэкonMetricвapps/server/src/integrations/mcp/mcp.service.ts:356-360диспатчит по ТОЧНОМУ имени черезif/else ifбез default-ветки — эти два имени не совпадают ни с одной веткой и отбрасываются. Регистрации вmetrics.registry.tsтоже нет (grep поapps/serverпуст). Т.е. раздел 2 PR («Счётчики через onMetricFn-синк») ничего не пишет в prom — это мёртвый код.Фикс: провести по существующему 3-частному паттерну (как
mcp_tool_duration_seconds/collab_connect_timeouts_total):METRIC_*конст + дваCounter+incGetPageCacheHit()/Miss()вmetrics.registry.ts, и две веткиelse ifв синкеmcp.service.ts. (Если метрики намеренно откладываются — убрать обаonMetricFn?.()-вызова, чтобы не тащить мёртвый код и ложную заявку в теле PR.)F2 [test-coverage] Нет теста, что ХИТ реально пропускает конверсию — а это единственный смысл PR.
getpage-conversion-cache.test.mjsдоказывает корректность вывода и инкремент miss-счётчика, но «на хите обход НЕ выполняется» выводится лишь косвенно из miss-счётчика (он в else-ветке). Мутация, где хит-ветка тоже зовётconvertProseMirrorToMarkdown(вывод тот же, misses=1), оставит ВСЕ ассерты зелёными — т.е. если оптимизация тихо перестанет срабатывать, тест не упадёт.Фикс: счётчик вызовов конвертера (обёртка/мок) — ассертить, что за два чтения в тесте MISS→HIT он вызван РОВНО один раз и на хите не растёт. Прямо пиннит инвариант skip-on-hit.
⛔ DROP — кодеру НЕ делать · калибровка (оператору)
[below-threshold]low/med[documentation] Комментарий-обоснованиеoptionsHashчерез «коллизию getPage/exportPageMarkdown» контрфактичен: export кэш не трогает, коллизии структурно быть не может (ключ корректен, обоснование — форвард-сейфти). Отметили documentation+coherence. — getpage-cache.ts:14-18 — код верен, неточна лишь причина.[below-threshold]suggestion/med[simplification]hashConvertOptionsвозвращает сырую стабильную JSON-строку, не дайджест — имя обещает хэш. — getpage-cache.ts:63-78 — можно переименовать вoptionsKey, но автор вправе оставить (content-addressing лексикон).[below-threshold]low/high[test-coverage] Ветка non-cacheable fallback (нетid/updatedAt) не покрыта — оборонительный код на «неожиданную форму ответа», риск удалённый.[below-threshold]low/med[documentation] Нет записи в CHANGELOG про две новые метрики — станет уместно, если F1 их оживит; внутренняя перф-оптимизация, byte-identical.[below-threshold]suggestion/med[conventions] Дефис в именах:getpage-cachevs сиблингget-page-context.test.mjs; три разных base-имени фичи. Discoverability-дрейф.9 аспектов (security · stability · regressions · test-coverage · conventions · documentation · simplification · architecture · coherence) + чистый гейт 814/814. security/stability/regressions/architecture/coherence — LGTM; корректность и изоляция кэша подтверждены чтением. Оба DO дёшевы; на ре-ревью прогоню тем же прогоном + мутацию на skip-on-hit.
F1: fixed — счётчики были мертвы, потому что серверный синк mcp.service.ts диспатчил по имени через if/else-if БЕЗ default и знал только 2 имени (mcp_tool_duration_seconds, collab_connect_timeouts_total); мои дропались молча, и в metrics.registry их не было. Оживил по существующему 3-частному паттерну: имена-константы + два Counter'а + incGetPageCacheHit/Miss в registry + два else-if в синке (существующие ветки не тронуты). Проверено end-to-end скрейпом prom-реестра: hits=2/misses=1 после прогона роутинга (коммит
70a9e2a9).F2: fixed — добавил overridable seam convertPageMarkdown в read.ts (идиома проекта для ESM), getPage miss-ветка идёт через него; тест мокает его и ассертит callCount===1 через MISS→HIT (конверсия один раз на промахе, ноль на хите). Мутационно доказан: пропатчил hit-ветку на повторную конверсию → тест покраснел (callCount=2), откат → зелёный. Именно тот пробел, что ты отметил.
Гейт: mcp node --test 814/814 (+1); pmd+mcp tsc чисто; serverные metrics-файлы компилируются изолированно + runtime-тест счётчиков зелёный.
Внутренний цикл (fix-раунд): 1 проход прямым чтением дельты — синк-ветки аддитивны (не ломают существующие), registry регистрирует 2 счётчика корректно, seam чисто делегирует.
Ревью #480 — раунд 2. Вердикт: approved ✅
Обе задачи первого раунда закрыты и проверены адверсариально, не на словах. Мержить можно.
Закрыто с р1: F1 (мёртвые метрики), F2 (не было теста на skip-on-hit). Новых багов фикс не внёс.
📋 Как проверял закрытие F1/F2 (verify-don't-trust-prose)
F1 — счётчики hit/miss ожили, проверено сквозным скрейпом (дважды, независимо):
METRIC_MCP_GETPAGE_CACHE_*+ дваCounterвinit()(рядом с рабочими docLoads/connectTimeouts) +incGetPageCacheHit/Miss(?.inc()null-safe) + две веткиelse ifв синкеmcp.service.ts, дописаны ПОСЛЕ существующих двух (те байт-идентичны, не тронуты).name ===в синке == значение константы == имя Counter'а. Каждая из двух строк встречается ровно 3× (const+sink+emit) — ни опечатки, ни свопа hit/miss.METRICS_PORTвыставлен): после инкрементовgetMetricsRegistry().metrics()отдаётmcp_getpage_cache_hits_total 2/mcp_getpage_cache_misses_total 1. Дубль-регистрации нет (prom не бросил), серверныйmetrics.spec25/25.F2 — тест доказывает, что ХИТ пропускает конверсию, и он НЕ вхолостую (мутационно):
protected convertPageMarkdown(чистый делегат, идиома проекта для мокабельного ESM-шва); miss-ветка идёт черезthis.convertPageMarkdown, hit-ветка — нет. Тест спаит шов и ассертитcallCount()===1строго (не>=1) через MISS→HIT.# fail 1, callCount=2), откат → зелёный. Пробел из р1 реально закрыт (node:testmock.methodделегирует к оригиналу, так что вывод остаётся настоящим — вывод байт-идентичен проверяется соседним тестом).Гейт (чистая комната): pmd+mcp tsc чисто;
node --test test/815/815 (814 из р1 + 1 новый F2); серверныйmetrics.spec25/25; F2-мутация убита; F1-скрейп 2/1. Регрессий нет: существующие метрики байт-идентичны, шов — чистый делегат (только getPage miss через него; export и прочие вызовы конвертера не затронуты),protectedне течёт в публичный Pick-контракт (#478). Ядро кэша (изоляция, ключ, LRU, SUBPAGES-on-hit) с р1 не менялось.⛔ DROP — кодеру НЕ делать · калибровка (оператору)
[below-threshold]low/high[conventions] Осиротевший JSDoc/** Raw page info... */(read.ts:403) после вставкиconvertPageMarkdownоказался прямо над ним и читается как его доккоммент, хотя метод конвертит PM→MD. У метода есть свой корректный JSDoc сразу ниже, так что реально не вводит в заблуждение. Можно опционально снести строку 403 при следующей правке — не блокер.[below-threshold]low/high[test-coverage] Включённый путь F1 (sink→registry→scrape) не пиннится отдельным серверным тестом —+4строкиmetrics.specбьют только disabled-no-op; синк роутит по строковым литералам, не поMETRIC_*-константам, так что будущий дрейф имени тихо уронил бы счётчик при зелёном сьюте. НО: путь сейчас РАБОТАЕТ (проверил скрейпом), имя эмита пиннится mcp-тестом, регистрация идентична 3 покрытым счётчикам. Дешёвый enabled-scrape тест закрыл бы класс насовсем — оставляю на усмотрение (не гейчу сходимость из-за low-найдинга на уже работающем фиксе).Раунд 2: спотлайт на закрытии F1/F2 + поиск внесённых фиксом багов (security · stability · regressions · test-coverage · conventions · coherence на дельте; ядро кэша прошло полный 9-аспектный проход в р1 и не менялось). Обе находки закрыты, объективка зелёная, F2 мутационно-доказан, F1 скрейпом подтверждён. → approved.