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 }) + } + }) +})