perf(mcp): checkNewComments — параллелизм с капом (#490)

checkNewComments делал O(N) последовательных REST-вызовов listComments по страницам working set — большой space линеен по round-trip'ам. Теперь per-page фетчи идут с ограниченным параллелизмом (cap 6, середина полосы 5–8): независимые чтения не ждут друг друга, но и не заваливают сервер/сокеты. mapWithConcurrency — крошечный пул без зависимости от p-limit: N воркеров тянут следующий индекс с общего курсора. Порядок результатов сохраняется (по входному порядку страниц), поэтому вывод детерминирован независимо от того, какой фетч завершился первым. Серверный batch-эндпоинт «comments updated since T по space» — опционально, отдельным заходом. Тест (mock-HTTP): 13 страниц, задержанный /api/comments — maxInFlight > 1 и <= 6 (последовательная реализация дала бы 1), порядок результатов = порядок обхода. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 11:35:58 +03:00
parent d35956b9e9
commit 9f0e880e0b
2 changed files with 126 additions and 20 deletions
@@ -59,6 +59,39 @@ import {
  mergeFootnoteDefinitions,
 } from "../lib/transforms.js";

+// Max concurrent per-page comment fetches in checkNewComments (#490). The scan is
+// O(N) independent REST reads over the working set; running them one-at-a-time made
+// a large space linear in round-trips. A small cap parallelizes without hammering
+// the server (or exhausting sockets). 6 is a conservative middle of the 5–8 band.
+const COMMENT_SCAN_CONCURRENCY = 6;
+
+/**
+ * Map `items` through `fn` with at most `limit` in flight, preserving INPUT ORDER
+ * in the returned array. A tiny bounded pool (no p-limit dependency): `limit`
+ * workers pull the next index off a shared cursor until the list is drained.
+ */
+async function mapWithConcurrency<T, R>(
+  items: readonly T[],
+  limit: number,
+  fn: (item: T, index: number) => Promise<R>,
+): Promise<R[]> {
+  const results = new Array<R>(items.length);
+  let cursor = 0;
+  const worker = async (): Promise<void> => {
+    for (;;) {
+      const i = cursor++;
+      if (i >= items.length) return;
+      results[i] = await fn(items[i], i);
+    }
+  };
+  const workers = Array.from(
+    { length: Math.max(1, Math.min(limit, items.length)) },
+    () => worker(),
+  );
+  await Promise.all(workers);
+  return results;
+}
+
 // Public method surface of CommentsMixin (issue #450) — a NAMED type so the factory
 // return type is expressible in the emitted .d.ts (the anonymous mixin class
 // carries the base's protected shared state, which would otherwise trip TS4094).
@@ -655,27 +688,32 @@ export function CommentsMixin<TBase extends GConstructor<DocmostClientContext>>(
      parentPageId,
    );

-    // 2. Fetch comments for each page, keep ones created after since
-    const results: any[] = [];
-    for (const page of pagesInScope) {
-      try {
-        // Full feed (incl. resolved): a "new comments since" scan reports all
-        // recent activity; the active-only filter is scoped to listComments.
-        const comments = (await this.listComments(page.id, true)).items;
-        const newComments = comments.filter(
-          (c: any) => new Date(c.createdAt) > sinceDate,
-        );
-        if (newComments.length > 0) {
-          results.push({
-            pageId: page.id,
-            pageTitle: page.title,
-            comments: newComments,
-          });
+    // 2. Fetch comments for each page, keep ones created after since. Runs with
+    // bounded concurrency (#490) instead of one-at-a-time — the per-page reads are
+    // independent, so a large working set no longer costs O(N) serial round-trips.
+    // Order is preserved (mapWithConcurrency keeps input order), so the output is
+    // deterministic regardless of which fetch finishes first.
+    const perPage = await mapWithConcurrency(
+      pagesInScope,
+      COMMENT_SCAN_CONCURRENCY,
+      async (page: any) => {
+        try {
+          // Full feed (incl. resolved): a "new comments since" scan reports all
+          // recent activity; the active-only filter is scoped to listComments.
+          const comments = (await this.listComments(page.id, true)).items;
+          const newComments = comments.filter(
+            (c: any) => new Date(c.createdAt) > sinceDate,
+          );
+          return newComments.length > 0
+            ? { pageId: page.id, pageTitle: page.title, comments: newComments }
+            : null;
+        } catch (e: any) {
+          // Skip pages with errors (e.g. deleted between calls)
+          return null;
        }
-      } catch (e: any) {
-        // Skip pages with errors (e.g. deleted between calls)
-      }
-    }
+      },
+    );
+    const results: any[] = perPage.filter((r): r is any => r !== null);

    const totalNewComments = results.reduce(
      (sum, r) => sum + r.comments.length,
@@ -442,3 +442,71 @@ test("checkNewComments subtree includes the root without a separate getPageRaw",
  assert.equal(result.checkedPages, 2, "root + one descendant scanned");
  assert.equal(result.totalNewComments, 1, "the root's fresh comment found");
 });
+
+// -----------------------------------------------------------------------------
+// 6) checkNewComments parallelism (#490): the per-page comment fetches run with
+//    bounded concurrency (not one-at-a-time), and the results still preserve the
+//    page order deterministically regardless of which fetch finishes first.
+// -----------------------------------------------------------------------------
+test("checkNewComments fetches pages concurrently (bounded) and preserves order", async () => {
+  // A subtree with 12 descendants so the scan has plenty to parallelize.
+  const NODES = [{ id: "parent", title: "Parent", parentPageId: null, hasChildren: true }];
+  for (let i = 0; i < 12; i++) {
+    NODES.push({ id: `k${i}`, title: `Kid ${i}`, parentPageId: "parent", hasChildren: false });
+  }
+
+  let inFlight = 0;
+  let maxInFlight = 0;
+
+  const { baseURL } = await spawn(async (req, res) => {
+    const raw = await readBody(req);
+    if (handleLogin(req, res)) return;
+    if (req.url === "/api/pages/tree") {
+      sendJson(res, 200, { success: true, data: { items: NODES } });
+      return;
+    }
+    if (req.url === "/api/comments") {
+      const body = JSON.parse(raw || "{}");
+      inFlight++;
+      maxInFlight = Math.max(maxInFlight, inFlight);
+      // Hold the response briefly so concurrent fetches actually overlap.
+      setTimeout(() => {
+        inFlight--;
+        // Every page carries one fresh comment so ordering is observable.
+        sendJson(res, 200, {
+          success: true,
+          data: {
+            items: [
+              { id: `c-${body.pageId}`, createdAt: "2030-01-01T00:00:00.000Z", content: null },
+            ],
+            meta: { nextCursor: null },
+          },
+        });
+      }, 25);
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "user@example.com", "pw");
+  const result = await client.checkNewComments(
+    "space-1",
+    "2020-01-01T00:00:00.000Z",
+    "parent",
+  );
+
+  // 13 pages (parent + 12 kids) were scanned; each had a fresh comment.
+  assert.equal(result.checkedPages, 13, "all pages scanned");
+  assert.equal(result.totalNewComments, 13, "one fresh comment per page");
+  // Parallelism: more than one request was in flight at once, but never above the
+  // cap (6). A serial implementation would show maxInFlight === 1.
+  assert.ok(maxInFlight > 1, `expected concurrent fetches, saw max ${maxInFlight}`);
+  assert.ok(maxInFlight <= 6, `concurrency must be bounded, saw ${maxInFlight}`);
+  // Deterministic order: results follow the page-enumeration order (parent first).
+  assert.equal(result.comments[0].pageId, "parent", "results preserve page order");
+  assert.deepEqual(
+    result.comments.map((r) => r.pageId),
+    ["parent", ...Array.from({ length: 12 }, (_, i) => `k${i}`)],
+    "result order matches the enumeration order regardless of finish order",
+  );
+});