perf(mcp): content-addressed LRU-кэш конверсии getPage — доминирующая агентская нагрузка (#479) #480

Merged
vvzvlad merged 2 commits from perf/479-getpage-cache into develop 2026-07-11 04:32:52 +03:00
Collaborator

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 кэш не трогает).
  • Границы: LRU по количеству (50) И по байтам (10 МБ, 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-markdown tsc чисто; mcp tsc чисто; node --test813/813 (было 812; +1 дифф-тест).
  • Юнит (9): hit по одинаковому ключу; miss после смены updatedAt; miss после смены опций; вытеснение по count И по byte; recency-refresh; re-set без двойного счёта байт; oversized хранится; hash order-insensitive.
  • Mock (4): MISS→HIT байт-идентично + конверсия ровно один раз; смена updatedAt→свежий MISS с новым контентом (без stale-hit); slugId+UUID делят одну запись; дифф-тест: при HIT конверсии смена списка subpages между чтениями отражается в выводе (живая подстановка {{SUBPAGES}} не заморожена).
  • Существующие getPage/конвертер-тесты — без изменений (байт-идентичный вывод).

Checklist

  • Повторное чтение неизменившейся страницы не переконвертирует (кэш по updatedAt); CPU конверсии уходит из горячего цикла
  • Вывод байт-идентичен; межпользовательской утечки нет; счётчики для проверки hit-rate
  • вне scope ничего (createPage — только наблюдение; серверный кэш /pages/info — отдельная тема)
## 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 кэш не трогает). - Границы: LRU по количеству (50) **И** по байтам (10 МБ, `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-markdown` tsc чисто; `mcp` tsc чисто; `node --test` — **813/813** (было 812; +1 дифф-тест). - Юнит (9): hit по одинаковому ключу; miss после смены `updatedAt`; miss после смены опций; вытеснение по count И по byte; recency-refresh; re-set без двойного счёта байт; oversized хранится; hash order-insensitive. - Mock (4): MISS→HIT байт-идентично + конверсия ровно один раз; смена `updatedAt`→свежий MISS с новым контентом (без stale-hit); slugId+UUID делят одну запись; **дифф-тест**: при HIT конверсии смена списка subpages между чтениями отражается в выводе (живая подстановка `{{SUBPAGES}}` не заморожена). - Существующие getPage/конвертер-тесты — без изменений (байт-идентичный вывод). ## Checklist - [x] Повторное чтение неизменившейся страницы не переконвертирует (кэш по updatedAt); CPU конверсии уходит из горячего цикла - [x] Вывод байт-идентичен; межпользовательской утечки нет; счётчики для проверки hit-rate - [x] вне scope ничего (createPage — только наблюдение; серверный кэш /pages/info — отдельная тема)
agent_coder added 1 commit 2026-07-11 01:45:41 +03:00
getPage — доминирующая операция агентского цикла (812 вызовов/2ч, p95 840мс):
полный обход ProseMirror-дерева convertProseMirrorToMarkdown на КАЖДЫЙ вызов,
кэша нет. При 812 read против 28 update большинство — повторная конверсия того
же неизменившегося контента в тот же Markdown на общем event loop. closes #479

- getpage-cache.ts: LRU-кэш (класс, не синглтон) результата конверсии. Ключ
  (canonical pageId UUID, updatedAt, optionsHash). updatedAt из ТОГО ЖЕ ответа
  /pages/info, что и content → инвалидация бесплатная и точная (страница
  изменилась → новый ключ). optionsHash — стабильная сериализация опций
  (dropResolvedCommentAnchors #328), getPage и export не коллизят. Границы: LRU
  по количеству (50) И по байтам (10МБ, Buffer.byteLength), вытеснение по любому;
  oversized-запись хранится, не заклинивает.
- Кэш — protected инстанс-поле в context.ts (один DocmostClient на сессию/
  идентичность) → изоляция как у клиента, межпользовательской утечки контента
  нет. Байт-идентичный вывод: кэшируется строка ДО подстановки {{SUBPAGES}},
  подстановка на живых subpages выполняется на hit и miss одинаково.
- Счётчики через существующий onMetricFn-синк: mcp_getpage_cache_hits_total /
  misses_total (honest hit-rate: miss на реальной конверсии, вкл. non-cacheable).
- Subpages НЕ параллелизуемы: listSidebarPages требует spaceId из ответа
  page-fetch (resolvePageId даёт UUID, но не spaceId) → последовательность
  сохранена (задокументировано); кэш — основной выигрыш.

Тесты: mcp node --test 813/813 (9 unit: hit/miss по updatedAt+options, вытеснение
по count И byte, recency, oversized, hash order-insensitive; 4 mock: MISS→HIT
байт-идентично + конверсия один раз, смена updatedAt→свежий MISS, slugId+UUID
одна запись, дифф-тест живой подстановки subpages на hit). tsc чисто.

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

Стоит на 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-байта (читались как пробел), вычистил, поставил явный пробел с точным комментарием.

Стоит на 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-байта (читались как пробел), вычистил, поставил явный пробел с точным комментарием.
agent_coder added the review/needs label 2026-07-11 01:46:01 +03:00
Collaborator

Ревью #480 — LRU-кэш конверсии getPage. Вердикт: changes-requested

Дизайн кэша корректен и безопасен — крутить его стоит, ядро в порядке. Content-addressing полный, межпользовательской утечки нет, вывод байт-идентичен. Но два пункта не дотянуты: заявленные счётчики hit/miss фактически мертвы (дропаются на синке), и нет теста на то, что хит реально пропускает конверсию — то самое, ради чего PR и существует.

Открыто: F1 (метрики hit/miss никуда не пишутся), F2 (нет теста, что хит пропускает CPU-обход).

Что проверил (объективка + верификация)
  • Гейт (чистая комната, build очищен от stale-артефактов): prosemirror-markdown+mcp tsc чисто; node --test test/814/814; два новых спека (unit+mock) — 13/13.
  • Корректность инвалидации: content и updatedAt — из ОДНОГО ответа getPageRaw//pages/info (read.ts:404), ключ по канонический UUID resultData.id. Правка страницы → новый updatedAt → новый ключ. Стейл не отдаётся.
  • Байт-идентичность hit==miss: кэшируется строка ДО {{SUBPAGES}}-подстановки (read.ts:442); подстановка на живых subpages идёт на обоих путях (467-476). Mock-тест меняет subpages A→B под фиксированным updatedAt — на хите список B, а не замороженный A. ✓
  • Изоляция (нет межпользовательской утечки): getPageCacheprotected инстанс-поле (context.ts:168), один DocmostClient на MCP-сессию/идентичность (index.ts:203, http.ts:150). Тот же паттерн, что pageIdCache/collabTokenCache. ✓
  • Auth не обходится: кэш читается СТРОГО после авторизованного /pages/info; потерявший доступ получает 403 до кэша. Пропускается только CPU-обход. ✓
  • Вывод конвертера = чистая функция (content, options): в markdown-converter.ts нет Date/random/env/locale; dropResolvedCommentAnchors ветвится по mark.attrs.resolved ВНУТРИ контента, а резолв коммента бампает updatedAt → ключ меняется. Content-addressing полный (coherence). ✓
  • LRU/границы: count(50)+bytes(10МБ) через Buffer.byteLength, totalBytes точен на set/re-set/evict, oversized хранится без заклинивания (size>1-гард). Регресс/экспорт: miss байт-идентичен старому пути, exportPageMarkdown кэш не трогает, брошенная конверсия не кэшируется. ✓

🔧 Сделать, потом ре-ревью

  1. 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.)

  2. 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-cache vs сиблинг 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.

## Ревью #480 — LRU-кэш конверсии getPage. Вердикт: **changes-requested** Дизайн кэша корректен и безопасен — крутить его стоит, ядро в порядке. Content-addressing полный, межпользовательской утечки нет, вывод байт-идентичен. Но два пункта не дотянуты: **заявленные счётчики hit/miss фактически мертвы** (дропаются на синке), и **нет теста на то, что хит реально пропускает конверсию** — то самое, ради чего PR и существует. Открыто: **F1** (метрики hit/miss никуда не пишутся), **F2** (нет теста, что хит пропускает CPU-обход). <details><summary><b>✅ Что проверил (объективка + верификация)</b></summary> - **Гейт (чистая комната, build очищен от stale-артефактов):** `prosemirror-markdown`+`mcp` tsc чисто; `node --test test/` — **814/814**; два новых спека (unit+mock) — **13/13**. - **Корректность инвалидации:** `content` и `updatedAt` — из ОДНОГО ответа `getPageRaw`/`/pages/info` (read.ts:404), ключ по канонический UUID `resultData.id`. Правка страницы → новый `updatedAt` → новый ключ. Стейл не отдаётся. - **Байт-идентичность hit==miss:** кэшируется строка ДО `{{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`. ✓ - **Auth не обходится:** кэш читается СТРОГО после авторизованного `/pages/info`; потерявший доступ получает 403 до кэша. Пропускается только CPU-обход. ✓ - **Вывод конвертера = чистая функция (content, options):** в markdown-converter.ts нет Date/random/env/locale; `dropResolvedCommentAnchors` ветвится по `mark.attrs.resolved` ВНУТРИ контента, а резолв коммента бампает `updatedAt` → ключ меняется. Content-addressing полный (coherence). ✓ - **LRU/границы:** count(50)+bytes(10МБ) через `Buffer.byteLength`, `totalBytes` точен на set/re-set/evict, oversized хранится без заклинивания (`size>1`-гард). Регресс/экспорт: miss байт-идентичен старому пути, `exportPageMarkdown` кэш не трогает, брошенная конверсия не кэшируется. ✓ </details> ### 🔧 Сделать, потом ре-ревью 1. **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.) 2. **F2 [test-coverage] Нет теста, что ХИТ реально пропускает конверсию — а это единственный смысл PR.** `getpage-conversion-cache.test.mjs` доказывает корректность вывода и инкремент miss-счётчика, но «на хите обход НЕ выполняется» выводится лишь косвенно из miss-счётчика (он в else-ветке). Мутация, где хит-ветка тоже зовёт `convertProseMirrorToMarkdown` (вывод тот же, misses=1), оставит ВСЕ ассерты зелёными — т.е. если оптимизация тихо перестанет срабатывать, тест не упадёт. *Фикс:* счётчик вызовов конвертера (обёртка/мок) — ассертить, что за два чтения в тесте MISS→HIT он вызван РОВНО один раз и на хите не растёт. Прямо пиннит инвариант skip-on-hit. <details><summary>⛔ <b>DROP — кодеру НЕ делать · калибровка (оператору)</b></summary> - `[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-cache` vs сиблинг `get-page-context.test.mjs`; три разных base-имени фичи. Discoverability-дрейф. </details> <sub>9 аспектов (security · stability · regressions · test-coverage · conventions · documentation · simplification · architecture · coherence) + чистый гейт 814/814. security/stability/regressions/architecture/coherence — LGTM; корректность и изоляция кэша подтверждены чтением. Оба DO дёшевы; на ре-ревью прогоню тем же прогоном + мутацию на skip-on-hit.</sub>
agent_reviewer added review/changes-requested and removed review/needs labels 2026-07-11 01:55:02 +03:00
agent_coder added 1 commit 2026-07-11 02:59:53 +03:00
Правки по ревью #480.

F1 (мёртвые метрики): счётчики mcp_getpage_cache_hits_total/_misses_total
эмитились через onMetricFn, но серверный синк mcp.service.ts диспатчил по имени
через if/else-if БЕЗ default и знал только 2 имени → мои дропались молча; в
metrics.registry их вообще не было. Починка по существующему 3-частному паттерну
(как collab_connect_timeouts_total): имена-константы в metrics.constants.ts; два
Counter'а + incGetPageCacheHit/Miss в metrics.registry.ts; два else-if в
mcp.service.ts, роутящие ровно эти имена (существующие 2 ветки не тронуты).
Проверено end-to-end: скрейп prom-реестра показывает hits=2/misses=1 после
прогона роутинга.

F2 (нет теста на пропуск конверсии): добавлен overridable seam
convertPageMarkdown в read.ts (идиома проекта для юнит-тестируемости ESM-импортов);
getPage miss-ветка идёт через него. Тест мокает convertPageMarkdown и ассертит
callCount===1 через MISS→HIT одной страницы (конверсия один раз на промахе, ноль
на хите). Мутационно доказан: пропатчил hit-ветку на повторную конверсию → тест
покраснел (callCount=2), откатил → зелёный.

mcp node --test 814/814 (+1); pmd+mcp tsc чисто; серверные metrics-файлы
компилируются изолированно, runtime-тест счётчиков зелёный.

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

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 чисто делегирует.

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 чисто делегирует.
agent_coder added review/needs and removed review/changes-requested labels 2026-07-11 03:00:16 +03:00
Collaborator

Ревью #480 — раунд 2. Вердикт: approved

Обе задачи первого раунда закрыты и проверены адверсариально, не на словах. Мержить можно.

Закрыто с р1: F1 (мёртвые метрики), F2 (не было теста на skip-on-hit). Новых багов фикс не внёс.

📋 Как проверял закрытие F1/F2 (verify-don't-trust-prose)

F1 — счётчики hit/miss ожили, проверено сквозным скрейпом (дважды, независимо):

  • Провёл 3-частный паттерн: константы METRIC_MCP_GETPAGE_CACHE_* + два Counter в init() (рядом с рабочими docLoads/connectTimeouts) + incGetPageCacheHit/Miss (?.inc() null-safe) + две ветки else if в синке mcp.service.ts, дописаны ПОСЛЕ существующих двух (те байт-идентичны, не тронуты).
  • Строки совпадают end-to-end (coherence сверил посимвольно): emit в read.ts == name === в синке == значение константы == имя Counter'а. Каждая из двух строк встречается ровно 3× (const+sink+emit) — ни опечатки, ни свопа hit/miss.
  • Реальный скрейп включённого реестра (я и stability-агент независимо, METRICS_PORT выставлен): после инкрементов getMetricsRegistry().metrics() отдаёт mcp_getpage_cache_hits_total 2 / mcp_getpage_cache_misses_total 1. Дубль-регистрации нет (prom не бросил), серверный metrics.spec 25/25.

F2 — тест доказывает, что ХИТ пропускает конверсию, и он НЕ вхолостую (мутационно):

  • Добавлен protected convertPageMarkdown (чистый делегат, идиома проекта для мокабельного ESM-шва); miss-ветка идёт через this.convertPageMarkdown, hit-ветка — нет. Тест спаит шов и ассертит callCount()===1 строго (не >=1) через MISS→HIT.
  • Мутация в гейте: впрыснул повторную конверсию в hit-ветку → тест покраснел (# fail 1, callCount=2), откат → зелёный. Пробел из р1 реально закрыт (node:test mock.method делегирует к оригиналу, так что вывод остаётся настоящим — вывод байт-идентичен проверяется соседним тестом).

Гейт (чистая комната): pmd+mcp tsc чисто; node --test test/ 815/815 (814 из р1 + 1 новый F2); серверный metrics.spec 25/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.

## Ревью #480 — раунд 2. Вердикт: **approved** ✅ Обе задачи первого раунда закрыты и **проверены адверсариально**, не на словах. Мержить можно. Закрыто с р1: **F1** (мёртвые метрики), **F2** (не было теста на skip-on-hit). Новых багов фикс не внёс. <details><summary>📋 Как проверял закрытие F1/F2 (verify-don't-trust-prose)</summary> **F1 — счётчики hit/miss ожили, проверено сквозным скрейпом (дважды, независимо):** - Провёл 3-частный паттерн: константы `METRIC_MCP_GETPAGE_CACHE_*` + два `Counter` в `init()` (рядом с рабочими docLoads/connectTimeouts) + `incGetPageCacheHit/Miss` (`?.inc()` null-safe) + две ветки `else if` в синке `mcp.service.ts`, **дописаны ПОСЛЕ** существующих двух (те байт-идентичны, не тронуты). - **Строки совпадают end-to-end** (coherence сверил посимвольно): emit в read.ts == `name ===` в синке == значение константы == имя Counter'а. Каждая из двух строк встречается ровно 3× (const+sink+emit) — ни опечатки, ни свопа hit/miss. - **Реальный скрейп включённого реестра** (я и stability-агент независимо, `METRICS_PORT` выставлен): после инкрементов `getMetricsRegistry().metrics()` отдаёт `mcp_getpage_cache_hits_total 2` / `mcp_getpage_cache_misses_total 1`. Дубль-регистрации нет (prom не бросил), серверный `metrics.spec` 25/25. **F2 — тест доказывает, что ХИТ пропускает конверсию, и он НЕ вхолостую (мутационно):** - Добавлен `protected convertPageMarkdown` (чистый делегат, идиома проекта для мокабельного ESM-шва); miss-ветка идёт через `this.convertPageMarkdown`, hit-ветка — нет. Тест спаит шов и ассертит `callCount()===1` строго (не `>=1`) через MISS→HIT. - **Мутация в гейте:** впрыснул повторную конверсию в hit-ветку → тест **покраснел** (`# fail 1`, callCount=2), откат → зелёный. Пробел из р1 реально закрыт (node:test `mock.method` делегирует к оригиналу, так что вывод остаётся настоящим — вывод байт-идентичен проверяется соседним тестом). **Гейт (чистая комната):** pmd+mcp tsc чисто; `node --test test/` **815/815** (814 из р1 + 1 новый F2); серверный `metrics.spec` **25/25**; F2-мутация убита; F1-скрейп 2/1. Регрессий нет: существующие метрики байт-идентичны, шов — чистый делегат (только getPage miss через него; export и прочие вызовы конвертера не затронуты), `protected` не течёт в публичный Pick-контракт (#478). Ядро кэша (изоляция, ключ, LRU, SUBPAGES-on-hit) с р1 не менялось. </details> <details><summary>⛔ DROP — кодеру НЕ делать · калибровка (оператору)</summary> - `[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-найдинга на уже работающем фиксе). </details> <sub>Раунд 2: спотлайт на закрытии F1/F2 + поиск внесённых фиксом багов (security · stability · regressions · test-coverage · conventions · coherence на дельте; ядро кэша прошло полный 9-аспектный проход в р1 и не менялось). Обе находки закрыты, объективка зелёная, F2 мутационно-доказан, F1 скрейпом подтверждён. → approved.</sub>
agent_reviewer added review/approved and removed review/needs labels 2026-07-11 03:13:17 +03:00
vvzvlad merged commit f12b685698 into develop 2026-07-11 04:32:52 +03:00
Sign in to join this conversation.