From 6721c52ad7913a3b368d39420b85b416732b08a5 Mon Sep 17 00:00:00 2001 From: David Snelling Date: Mon, 22 Jun 2026 18:00:01 -0700 Subject: [PATCH] fix: getNouns cursor pagination re-scanned the first page forever (permanent CPU loop) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shard-scan pagination adapter is offset-based and ignored the cursor, so getNouns({ pagination: { cursor } }) re-returned the first page on every cursor call. Harmless until 7.32.1 made totalCount the true dataset total — after which hasMore correctly stays true until a caller has paged through everything. A caller that paginates by cursor (cursor = page.nextCursor) — notably aggregate backfill over an already-populated store — then looped forever, re-walking the entire entity shard tree each iteration and pegging 1-2 CPU cores permanently on the JS main thread, with zero queries or traffic. (Pre-7.32.1 the same loop ended after one page, silently backfilling only the first 500 entities — an incomplete aggregate.) getNouns() now treats the cursor as an opaque, advancing offset token, so cursor pagination advances and terminates exactly like offset pagination — and an aggregate backfill streams the whole corpus exactly once (no longer truncated, no longer looping). Hardened the backfill loop to pure offset pagination as defense in depth. Regression (reproduces the infinite loop, fails fast on any re-scan): tests/unit/storage/getNouns-cursor-pagination.test.ts --- RELEASES.md | 29 ++++ src/brainy.ts | 15 ++- src/storage/baseStorage.ts | 61 +++++++-- .../getNouns-cursor-pagination.test.ts | 125 ++++++++++++++++++ 4 files changed, 209 insertions(+), 21 deletions(-) create mode 100644 tests/unit/storage/getNouns-cursor-pagination.test.ts diff --git a/RELEASES.md b/RELEASES.md index 72cb7d48..3730c400 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -10,6 +10,35 @@ Full auto-generated changelog: `CHANGELOG.md` · Releases: https://github.com/so --- +## v7.33.1 — 2026-06-22 + +**Affected products:** any consumer on `filesystem` / `mmap-filesystem` storage with **more than one +page (> 500) of entities** that defines an **aggregate** over an already-populated brain (the common +reopen-then-`defineAggregate()`/`queryAggregate()` case). **Critical fix** — a permanent post-init +CPU loop. Drop-in; no API or data changes. **Upgrade from 7.32.1 / 7.33.0 immediately.** + +### Fix — `getNouns()` cursor pagination no longer re-scans the first page forever (permanent CPU loop) + +`storage.getNouns({ pagination: { cursor } })` paginated by **cursor** never advanced: the shard-scan +adapter is offset-based and **ignored** the cursor, so every cursor call re-returned the first page. +On its own that was harmless — until v7.32.1 correctly made `totalCount` the true dataset total, which +makes `hasMore` stay `true` until a caller has paged through everything. A caller that paginates by +cursor (`cursor = page.nextCursor`) — notably **aggregate backfill** (`defineAggregate()` / +`queryAggregate()` over a pre-populated store) — therefore looped **forever**, re-walking the entire +entity shard tree on every iteration. With more than one page of entities this pegged 1–2 CPU cores +permanently on the JS main thread — with zero queries or traffic — and never settled. (Before v7.32.1 +the very same loop terminated after one page, silently backfilling only the first 500 entities — an +incomplete aggregate.) + +`getNouns()` now treats the cursor as an **opaque, advancing offset token**, so cursor pagination +behaves exactly like offset pagination — it advances and terminates — and an aggregate backfill now +streams the **whole** corpus exactly once (no longer truncated at 500, no longer looping). The backfill +loop was additionally hardened to pure offset pagination as defense in depth. Regression that +reproduces the infinite loop and fails fast on any re-scan: +`tests/unit/storage/getNouns-cursor-pagination.test.ts`. + +--- + ## v7.32.1 — 2026-06-17 **Affected products:** consumers on `filesystem` / `mmap-filesystem` storage whose logs showed diff --git a/src/brainy.ts b/src/brainy.ts index 3c1a6ac0..2c289af7 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -10089,22 +10089,23 @@ export class Brainy implements BrainyInterface { index.beginBackfill(name) + // Pure OFFSET pagination: advance by the number of items actually returned + // and stop as soon as the store reports no more (or returns an empty page). + // Driving this off `offset` alone — never re-feeding `nextCursor` — makes + // the loop provably terminating regardless of how the adapter implements + // cursors (an earlier cursor-fed variant re-fetched page 0 forever on a + // store larger than one page). const PAGE = 500 let offset = 0 - let cursor: string | undefined for (;;) { const page = await this.storage.getNouns({ - pagination: cursor ? { limit: PAGE, cursor } : { limit: PAGE, offset } + pagination: { limit: PAGE, offset } }) for (const noun of page.items) { index.backfillEntity(name, noun as unknown as Record) } + offset += page.items.length if (!page.hasMore || page.items.length === 0) break - if (page.nextCursor) { - cursor = page.nextCursor - } else { - offset += page.items.length - } } index.finishBackfill(name) diff --git a/src/storage/baseStorage.ts b/src/storage/baseStorage.ts index c7daf1e8..48f2ae3d 100644 --- a/src/storage/baseStorage.ts +++ b/src/storage/baseStorage.ts @@ -198,6 +198,33 @@ function idFromMetadataPath(path: string): string | undefined { return segments[segments.length - 2] } +/** + * Prefix marking a `getNouns()` pagination cursor as an opaque resume-OFFSET + * token. The shard-scan adapter (`getNounsWithPagination`) is offset-based and + * ignores a raw cursor, so `getNouns()` encodes the next offset INTO the cursor + * and decodes it on the way back in — making cursor pagination actually advance. + * Without this a cursor-paginating caller re-fetched page 0 every iteration and + * (once `totalCount` reported the true total) `hasMore` stayed true forever — a + * permanent re-scan loop on any store larger than one page. + */ +const OFFSET_CURSOR_PREFIX = 'off:' + +/** Encode a resume offset as an opaque `getNouns()` cursor token. */ +function encodeOffsetCursor(offset: number): string { + return `${OFFSET_CURSOR_PREFIX}${offset}` +} + +/** + * Decode a `getNouns()` cursor into its resume offset. Returns `undefined` for + * `undefined`/legacy/unrecognized cursors so the caller falls back to the + * explicit `offset` argument (never to a silent re-scan of page 0). + */ +function decodeOffsetCursor(cursor: string | undefined): number | undefined { + if (cursor === undefined || !cursor.startsWith(OFFSET_CURSOR_PREFIX)) return undefined + const n = Number(cursor.slice(OFFSET_CURSOR_PREFIX.length)) + return Number.isInteger(n) && n >= 0 ? n : undefined +} + /** * Base storage adapter that implements common functionality * This is an abstract class that should be extended by specific storage adapters @@ -1353,22 +1380,19 @@ export abstract class BaseStorage extends BaseStorageAdapter { // Get nouns by type directly (already combines with metadata) const nounsByType = await this.getNounsByNounType(nounType) - // Apply pagination - const paginatedNouns = nounsByType.slice(offset, offset + limit) - const hasMore = offset + limit < nounsByType.length - - // Set next cursor if there are more items - let nextCursor: string | undefined = undefined - if (hasMore && paginatedNouns.length > 0) { - const lastItem = paginatedNouns[paginatedNouns.length - 1] - nextCursor = lastItem.id - } + // Apply pagination. A cursor resumes by OFFSET (opaque offset token) so + // cursor pagination advances; fall back to the explicit offset otherwise. + const startOffset = decodeOffsetCursor(cursor) ?? offset + const paginatedNouns = nounsByType.slice(startOffset, startOffset + limit) + const hasMore = startOffset + limit < nounsByType.length return { items: paginatedNouns, totalCount: nounsByType.length, hasMore, - nextCursor + // Next resume offset, so a cursor-paginating caller advances instead + // of re-fetching the same page. + nextCursor: hasMore ? encodeOffsetCursor(startOffset + paginatedNouns.length) : undefined } } } @@ -1390,11 +1414,17 @@ export abstract class BaseStorage extends BaseStorageAdapter { // Check if the adapter has a paginated method for getting nouns if (typeof (this as any).getNounsWithPagination === 'function') { + // A cursor resumes by OFFSET: the shard-scan adapter is offset-based and + // ignores the raw cursor, so decode the resume offset the cursor carries. + // Without this, a cursor-paginating caller re-fetched offset 0 every call + // and `hasMore` stayed true forever → a permanent re-scan loop. Falls + // back to the explicit `offset` for a first page / legacy cursor. + const resolvedOffset = decodeOffsetCursor(cursor) ?? offset + // Use the adapter's paginated method - pass offset directly to adapter const result = await (this as any).getNounsWithPagination({ limit, - offset, // Let the adapter handle offset for O(1) operation - cursor, + offset: resolvedOffset, // Let the adapter handle offset for O(1) operation filter: options?.filter }) @@ -1422,7 +1452,10 @@ export abstract class BaseStorage extends BaseStorageAdapter { items, totalCount: finalTotalCount, hasMore: safeHasMore, - nextCursor: result.nextCursor + // Re-issue the cursor as the NEXT resume offset so cursor pagination + // advances exactly like offset pagination (it is offset pagination + // under the hood). Omitted when there is no next page. + nextCursor: safeHasMore ? encodeOffsetCursor(resolvedOffset + items.length) : undefined } } diff --git a/tests/unit/storage/getNouns-cursor-pagination.test.ts b/tests/unit/storage/getNouns-cursor-pagination.test.ts new file mode 100644 index 00000000..c4f95527 --- /dev/null +++ b/tests/unit/storage/getNouns-cursor-pagination.test.ts @@ -0,0 +1,125 @@ +/** + * @module tests/unit/storage/getNouns-cursor-pagination + * @description Regression for the 7.33.0 permanent-CPU-loop (a production + * deployment pegged 1-2 cores forever after init). Root cause: the shard-scan + * pagination (`getNounsWithPagination`) is offset-based and IGNORES the cursor, + * while 7.32.1 (correctly) made `totalCount` the true dataset total — so + * `hasMore` stays `true` until fully paginated. A caller paginating by CURSOR + * (`cursor = page.nextCursor`, e.g. aggregate backfill) therefore re-fetched + * offset 0 every iteration — `hasMore` never went false — an infinite re-scan, + * each pass a full O(N) shard-tree walk that pegged the JS main thread. + * + * The fix makes the `getNouns()` cursor an opaque, advancing offset token, so + * cursor pagination behaves exactly like offset pagination (advances + ends). + * The hard iteration guard below makes a regression FAIL fast instead of hang. + */ +import { describe, it, expect } from 'vitest' +import { mkdtempSync, rmSync } from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' +import { FileSystemStorage } from '../../../src/storage/adapters/fileSystemStorage.js' +import type { NounMetadata } from '../../../src/coreTypes.js' + +const DIM = 8 + +/** A deterministic non-zero vector so the noun record is well-formed. */ +function vec(seed: number): number[] { + return Array.from({ length: DIM }, (_, i) => ((seed + i) % 7) / 7 - 0.5) +} + +/** Storage shards by UUID, so ids must be 32 hex chars. */ +function uuid(i: number): string { + return i.toString(16).padStart(32, '0') +} + +/** Seed `n` unique nouns through the real metadata+record save path. */ +async function seed(storage: FileSystemStorage, n: number): Promise { + for (let i = 0; i < n; i++) { + const id = uuid(i) + await storage.saveNounMetadata(id, { + noun: 'thing', + createdAt: Date.now(), + updatedAt: Date.now() + } as NounMetadata) + await storage.saveNoun({ id, vector: vec(i), connections: new Map(), level: 0 }) + } +} + +describe('FileSystemStorage.getNouns cursor pagination (7.33.0 CPU-loop regression)', () => { + it('cursor pagination ADVANCES across pages and terminates (no infinite re-scan)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'brainy-cursor-')) + try { + const storage = new FileSystemStorage(dir) + await storage.init() + const TOTAL = 120 + const PAGE = 40 // → 3 full pages, so the cursor MUST advance to finish + await seed(storage, TOTAL) + + // The exact loop shape the aggregate backfill uses: drive purely off the + // returned cursor. Pre-fix this never terminated; the guard turns a + // regression into a fast failure instead of a hung suite. + const seen = new Set() + let cursor: string | undefined + let iterations = 0 + for (;;) { + if (++iterations > 1000) { + throw new Error( + 'getNouns cursor pagination did not terminate — the 7.33.0 infinite re-scan regressed' + ) + } + const page = await storage.getNouns({ + pagination: cursor ? { limit: PAGE, cursor } : { limit: PAGE } + }) + for (const n of page.items) seen.add(n.id) + if (!page.hasMore || page.items.length === 0) break + expect(page.nextCursor).toBeDefined() + cursor = page.nextCursor + } + + // Complete (every entity once) AND bounded (≈ TOTAL/PAGE pages, not ∞). + expect(seen.size).toBe(TOTAL) + expect(iterations).toBeLessThanOrEqual(Math.ceil(TOTAL / PAGE) + 1) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) + + it('the final page reports hasMore=false and no nextCursor', async () => { + const dir = mkdtempSync(join(tmpdir(), 'brainy-cursor-last-')) + try { + const storage = new FileSystemStorage(dir) + await storage.init() + await seed(storage, 30) + + // One page that covers the whole (tiny) corpus. + const page = await storage.getNouns({ pagination: { limit: 100 } }) + expect(page.items.length).toBe(30) + expect(page.hasMore).toBe(false) + expect(page.nextCursor).toBeUndefined() + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) + + it('a cursor and the equivalent offset return the same page', async () => { + const dir = mkdtempSync(join(tmpdir(), 'brainy-cursor-eq-')) + try { + const storage = new FileSystemStorage(dir) + await storage.init() + await seed(storage, 100) + + const first = await storage.getNouns({ pagination: { limit: 40 } }) + const viaCursor = await storage.getNouns({ + pagination: { limit: 40, cursor: first.nextCursor } + }) + const viaOffset = await storage.getNouns({ pagination: { limit: 40, offset: 40 } }) + + expect(viaCursor.items.map((n) => n.id)).toEqual(viaOffset.items.map((n) => n.id)) + // And the cursor page does NOT repeat the first page (the loop's failure mode). + const firstIds = new Set(first.items.map((n) => n.id)) + expect(viaCursor.items.some((n) => firstIds.has(n.id))).toBe(false) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) +})