perf(comment): унести Yjs-обновление comment-mark с HTTP-критического пути (#399) #438
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#438
Reference in New Issue
Block a user
Delete Branch "perf/399-comment-resolve-async"
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
Уносит Yjs-обновление comment-mark с HTTP-критического пути. resolve/unresolve/delete comment-mark раньше синхронно дёргали collab-gateway (
handleYjsEvent) прямо внутри HTTP-запроса — лишний сетевой раунд-трип к collab на горячем пути. Теперь DB-строка пишется синхронно (источник истины, общий таймстамп), а сама mark-мутация уходит идемпотентнымCOMMENT_MARK_UPDATE-джобом наGENERAL_QUEUE; воркер проигрывает тот жеhandleYjsEvent(байт-идентичная mark-операция, отuserне зависит).closes #399
Ключевое:
void enqueue().catch(warn), best-effort); delete —awaitэнкью ДО необратимого hard-delete.ts <= updatedAtстроки И состояние строки расходится с намерением джоба) пропускается, а не флипает mark в устаревшее состояние. Согласованный (идемпотентный) реплей всегда применяется.moduleRef.get(CollaborationGateway, {strict:false})— статического import-ребра в @Global QueueModule нет.How verified
Прогон на стенде (worktree, jest — рантаймер сервера):
npx jest general-queue.processor.comment-mark comment.service→ 7 suites, 54 tests passed.not.toHaveBeenCalled), payload+ts (ts == resolvedAt == updatedAt), маппинг resolve/unresolve/delete, stale-skip в обе стороны, согласованный старый реплей применяется, vanished-row без throw, delete-enqueue предшествует hard-delete, apply-текст остаётся sync.tscчистый.Внутренний цикл: 2 прохода. Правки по внутреннему ревью:
<→<=в race-guard (безопасная обработка суб-миллисекундной ничьи двух противоположных тогглов в один мс); задокументирован сознательный компромисс — транзиентныйpage.updated-broadcast из воркера несёт только{id}, теряяname/avatarUrl«кто редактировал» (lastUpdatedByIdвыставляется верно; косметика, самочинится на следующем реальном редактировании).Форк на заметку ревьюеру: воркер
GENERAL_QUEUEпредполагается живущим в процессе с зарегистрированнымCollaborationGateway(сейчас так — очереди обрабатываются in-process; при выделении отдельного worker-процесса безCollaborationModuleпервыйCOMMENT_MARK_UPDATE-джоб бросит и повторится).Checklist
resolve/unresolve/delete comment-mark синхронно дёргали collab-gateway (handleYjsEvent) прямо в HTTP-запросе — сетевой раунд-трип к collab на горячем пути. Теперь: - DB-строка пишется синхронно (источник истины) с общим таймстампом; - сама mark-операция уходит идемпотентным COMMENT_MARK_UPDATE-джобом на GENERAL_QUEUE, воркер проигрывает тот же handleYjsEvent; - resolve/unresolve — fire-and-forget (best-effort), delete — await энкью ДО необратимого hard-delete (durability split); - race-guard: устаревшее ПРОТИВОПОЛОЖНОЕ событие (ts <= updatedAt строки и состояние расходится) пропускается, а не флипает mark в устаревшее; - DI-цикл обойдён ленивым moduleRef.get(CollaborationGateway, strict:false). Внутренний цикл: 2 прохода. Правки по внутреннему ревью: `<` → `<=` в race-guard (безопасная обработка суб-миллисекундной ничьи двух противоположных тогглов); задокументирован сознательный компромисс — транзиентный page.updated-broadcast из воркера несёт только {id}, теряя name/avatarUrl «кто редактировал» (lastUpdatedById выставляется верно, косметика, самочинится на следующем реальном редактировании). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Ревью — #438
perf(comment): унести Yjs-обновление comment-mark с HTTP-критического пути (#399)Вердикт: ПРАВКИ (CHANGES) — 1 правка (тест fire-and-forget на resolve-пути).
Полный веер из 9 аспектов по всему диффу PR (merge-base
ee33a293). Гейт зелёный: server tsc 0; comment+processor спеки 47/47 (прогон-w 1); mcp build 0 +client-host-contract3/3 (дрейф-гард drawio).Правка (сделать, потом ре-ревью)
comment.service.ts:330(void this.enqueueCommentMarkUpdate(...).catch(...)). Delete-путь протестирован (enqueue awaited перед hard-delete; отказ enqueue → строка НЕ удаляется — консистентность). Но resolve-путь fire-and-forget: еслиgeneralQueue.addреджектит (очередь недоступна),resolveCommentдолжен ВСЁ РАВНО успешно вернуться (HTTP не падает) — а это и есть смысл PR (унести работу с HTTP-пути). Тест на это отсутствует (проверил: вmark-async.specнет reject-кейса для resolve). Fix:generalQueue.add.mockRejectedValue(...), вызватьresolveComment, ассертитьresolves(не throws) + варнинг залогирован.Что проверено и корректно (клик)
rowResolved !== wantResolved && ts <= rowMutatedAt): пропускает устаревшую out-of-order джобу, применяет актуальную — last-write-wins по timestamp строки, НЕ по порядку очереди. Мутационно покрыт (отключить гард → 2 теста «SKIPS a stale resolve/unresolve» падают). Идемпотентный реплей безопасен.deleteCommentIfChildless; отказ enqueue кидает до удаления → строка+mark консистентны (нет orphan-якоря). Покрыто.DocmostClientLike, ИHOST_CONTRACT_METHODS— дрейф-гард зелёный. Architecture/conventions/documentation/simplification — LGTM.⚠️ К сведению (не блокирую, не escalate)
PR бандлит два разных концерна:
6bf83619(perf-comment, ядро #399) +f46d89ea(drawio in-app tool-wiring, follow-up паритета #423/#434). Оба корректны по-отдельности. Чисто ради бисекции их можно было бы разнести на 2 PR — но оба проверены, не блокирую.DO/ESCALATE нет сверх покрытия. Async-механика корректна (staleness мутационно-держится, orphan косметичен+виден, порядок delete сохранён, create синхронен); недостаёт теста fire-and-forget resolve. После правки — PASS.
9 аспектов. Staleness-гард/delete-порядок/HTTP-окно/create-sync/drawio-дрейф проверены (мутация staleness + прямое чтение); fire-and-forget-гэп на resolve подтверждён мной (grep — reject-кейса нет). Параллельный SIGKILL = OOM стенда,
-w 1= 47/47. Escalate нет.Поправил единственную правку.
[test-coverage] fire-and-forget resolve-путь при отказе очереди — покрыл. Новый кейс в
comment.service.mark-async.spec.ts:generalQueue.add.mockRejectedValue(...), вызываюresolveComment→ ассертю.resolves(не throws) +Logger.warnвызван с ожидаемым сообщением и объектом ошибки. Реджект проглатывается на микротаске ПОСЛЕ возврата метода (void ...catch(warn)), поэтому перед ассертомflushMicrotasks()(setImmediate).Мутационно проверено: заменить
void enqueue().catch(warn)наawait enqueue()(регрессия — reject пробивается на HTTP-путь) → ровно этот тест краснеет, остальные 4 зелёные.Гейт:
jest comment.service.mark-async→ 5 passed (было 4).Ревью (раунд 2) — #438
perf(comment): унести Yjs-обновление comment-mark с HTTP-критического пути (#399)Вердикт: PASS ✅ — правка раунда 1 закрыта.
Полный веер из 9 аспектов. Гейт зелёный: server tsc 0,
comment.service.mark-async5/5 (-w 1; параллельный SIGKILL раунда 1 — OOM стенда, не дефект). Дельта раунда 2 — ТЕСТ-ОНЛИ (comment.service.ts+queue/байт-идентичны раунду 1, diff=0).Закрытие правки раунда 1 (мутационно подтверждено)
generalQueue.addв reject, ассертит чтоresolveCommentрезолвится (не throw) И залогирован warn; корректный flush микротасков (setImmediate) чтобы.catchотработал до ассертов. Мутация: заменилvoid enqueue().catch()наawait enqueue()(убрал fire-and-forget) → тест падает (1 failed/4 passed). Держит. Проверил сам + субагент независимо.DO/ESCALATE нет. Правка раунда 1 закрыта корректно и мутационно-держится; impl не менялся; гейт зелёный. Approve. (К сведению из раунда 1 остаётся: PR бандлит comment-perf + drawio-wiring — оба корректны, не блок.)
9 аспектов. Тест проверен адверсариально (await-мутация → падает) + консолидированным субагентом (soundness, нет полюции, нет impl-изменений). Escalate нет.