room-share: a legacy room's read recomputes the clamp target per line, so a long log reads in quadratic time

changelog.d/2766-room-floor-once.fixed.md

@@ -0,0 +1 @@
+Reading a long room that was saved before rooms carried a `seq` counter no longer walks its log once per line: the read works out the join floor once and filters with that number, so a 20,000-line history reads in linear instead of quadratic time, with the same visible lines and the same one-time warning ([#2766](https://github.com/ThinkFlowLab/FlyBot/issues/2766)).

core/src/host/room-membership.js

@@ -496,7 +496,12 @@ export function install(rt) {
     }
     return seq
   }
-  const linesFrom = (room, actor) => (room.log ?? []).filter(line => (line.seq ?? 0) >= floorOf(room, actor))
+  const linesFrom = (room, actor) => {
+    // The floor is one number for the whole read: a room saved before `seq` existed finds its clamp
+    // target with a reduce over the whole log, so asking per line reduced an n-line log n times.
+    const from = floorOf(room, actor)
+    return (room.log ?? []).filter(line => (line.seq ?? 0) >= from)
+  }
 
   rt.participantsOf = room => currentParticipants(room)
   rt.roomVisibleTo = (actor, roomId) => roomVisibleTo(state, actor, roomId)

core/test/room-floor-once.test.js

@@ -0,0 +1,130 @@
+// A room saved before `seq` existed finds its clamp target with a reduce over the whole log, and
+// `linesFrom` (`core/src/host/room-membership.js`) used to call `floorOf` once per line — so reading
+// an n-line legacy room reduced n times over n lines and the read was quadratic. This counts the
+// reduces instead of timing them.
+//
+//   node --test core/test/room-floor-once.test.js
+import assert from 'node:assert/strict'
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { test } from 'node:test'
+import { install as installStore } from '../src/host/store.js'
+import { install as installMemberAuth } from '../src/host/member-auth.js'
+import { install as installAuthz } from '../src/host/authz.js'
+import { install as installRooms } from '../src/host/room-membership.js'
+import { install as installReach } from '../src/host/reach.js'
+
+function host(t, saved) {
+  const dir = mkdtempSync(join(tmpdir(), 'ds-bot-floor-once-'))
+  const statePath = join(dir, 'state.json')
+  writeFileSync(statePath, JSON.stringify(saved))
+  const warnings = []
+  const disposers = []
+  const rt = {
+    ctx: {
+      logger: { warn: (...args) => warnings.push(args) },
+      effect: execute => { const d = execute(); if (typeof d === 'function') disposers.push(d) },
+    },
+    home: dir,
+    statePath,
+    autoRoomName: () => 'Room',
+    retitleRoom: async () => {},
+    dropGroupSession: () => {},
+    warn: (...args) => warnings.push(args),
+  }
+  installStore(rt)
+  installMemberAuth(rt)
+  installAuthz(rt)
+  installRooms(rt)
+  installReach(rt)
+  t.after(async () => {
+    await Promise.all(disposers.splice(0).map(dispose => dispose()))
+    rmSync(dir, { recursive: true, force: true, maxRetries: 5 })
+  })
+  return { rt, warnings }
+}
+
+const N = 20000
+const BOB = { memberId: 'owner-2', displayName: 'Bob' }
+const ALICE = { memberId: 'owner-1', displayName: 'Alice' }
+
+class CountingLog extends Array {
+  reduce(...args) {
+    CountingLog.reduces += 1
+    return super.reduce(...args)
+  }
+}
+CountingLog.reduces = 0
+
+function fixture(withSeq = true) {
+  const log = CountingLog.from({ length: N }, (_, i) => ({
+    who: 'Alice', text: `line ${i + 1}`, seq: i + 1, authorId: 'owner-1', authorKind: 'human',
+  }))
+  return {
+    version: 2, revision: 1, mainBotIds: ['chief-a'],
+    members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'chief-a' } },
+    bots: {
+      'chief-a': {
+        id: 'chief-a', sessionId: 'chief-a', name: 'Chief A', createdAt: 1, ownerId: 'owner-1',
+      },
+    },
+    rooms: {
+      legacy: {
+        id: 'legacy', name: 'Legacy', members: ['chief-a'], admin: 'chief-a', named: true, log, seen: {},
+        // A room saved before the counter existed has no `seq` of its own; `withSeq: false` keeps it
+        // that way (a room with a counter is the control case).
+        ...(withSeq ? { seq: N } : {}),
+        participants: [
+          { kind: 'human', id: 'owner-1', role: 'admin', displayName: 'Alice', joinedSeq: 0, joinedAt: 1 },
+          { kind: 'human', id: 'owner-2', role: 'member', displayName: 'Bob', joinedSeq: 0, joinedAt: 1 },
+        ],
+      },
+    },
+    grants: {}, agents: {}, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {},
+  }
+}
+
+// Load, then hand the room a counting log and (for the legacy case) drop the counter the load gives
+// it — the shape this bench is about is the one that was saved before `seq` existed.
+async function loadedLegacy(t, withSeq) {
+  const { rt, warnings } = await host(t, fixture(withSeq))
+  await rt.load()
+  const room = rt.state.rooms.legacy
+  room.log = CountingLog.from(room.log)
+  if (!withSeq) delete room.seq
+  CountingLog.reduces = 0
+  return { rt, room, warnings }
+}
+
+test('a legacy room read computes its clamp target once, not once per line', async (t) => {
+  const { rt, room, warnings } = await loadedLegacy(t, false)
+  const started = Date.now()
+  const seen = rt.linesVisibleTo(room, BOB)
+  const ms = Date.now() - started
+  console.log(`legacy linesVisibleTo n=${N}: ${ms}ms, reduce calls=${CountingLog.reduces}`)
+  assert.equal(seen.length, N, 'every line is readable')
+  assert.equal(CountingLog.reduces, 1, `the clamp target is computed once per read, saw ${CountingLog.reduces}`)
+  assert.deepEqual(warnings, [], 'a room whose stored floor is healthy says nothing')
+})
+
+test('a legacy room history read computes its clamp target once too', async (t) => {
+  const { rt } = await loadedLegacy(t, false)
+  const started = Date.now()
+  const seen = rt.roomHistory('legacy', ALICE)
+  const ms = Date.now() - started
+  console.log(`legacy roomHistory n=${N}: ${ms}ms, reduce calls=${CountingLog.reduces}`)
+  assert.equal(seen.length, N)
+  assert.equal(CountingLog.reduces, 1, `the clamp target is computed once per read, saw ${CountingLog.reduces}`)
+})
+
+test('a room with a counter reads the same window without a per-line reduce', async (t) => {
+  const { rt, room, warnings } = await loadedLegacy(t, true)
+  const started = Date.now()
+  const seen = rt.linesVisibleTo(room, BOB)
+  const ms = Date.now() - started
+  console.log(`modern linesVisibleTo n=${N}: ${ms}ms, reduce calls=${CountingLog.reduces}`)
+  assert.equal(seen.length, N)
+  assert.equal(CountingLog.reduces, 0, 'a room with a counter never walks the log to find one')
+  assert.deepEqual(warnings, [])
+})

docs/subsystems/core-host.md

@@ -398,7 +398,7 @@ Work a member hands a Bot with `message_bot` from its group Session runs in a hi
 - `syncFaceParticipants` / `faceRemoteRecord` keep optional `color` / `avatar` from the Bot Social member payload on the remote participant. The client seeds a distinct generated look from the handle when those fields are absent (see [core-client](core-client.md)). Face-room REST still does not require a person-grant.
 - `rt.uiLanguage()` defaults to `English` until a browser reports a locale; the memory tools use it to name the language the owner writes in on a fresh home (#173).
 - Face titles (#172): `core/src/shared/face-title.js` lists every participant owner in stable handle order and `isAutoFaceTitle` marks auto titles, so a roster change refreshes an auto-named room but never a renamed one; both homes derive the same title. A hosted face room that falls off the Bot Social list keeps its local transcript (poll cleanup no longer calls `dropFace`), so Past rooms **Open** can reopen it (#171).
-- A member that left and joins again is the same participant record with `leftSeq` cleared and a new `joinedSeq` — not a second live row. `upsertFaceRoom` treats `startedAt` (or an explicit `open`) as an open room so the Friends' Bots card does not stay on the matching countdown after **Enter chat**. A later snapshot whose status is not `open`, `closed`, or `left`, and that omits `startedAt`, does not demote a room that is already open, closed, or left back to `candidate` (#1072). The membership window clamps a stored `joinedSeq` above the room's own `room.seq` to `room.seq` on the read path (to the log's last seq for a room saved before `seq` existed), warns once per room, and never rewrites the saved state; the clamped window is memoised per room and reader and stays open once `room.seq` reaches and passes the stored floor, a rejoin being the one thing that drops it (#2754).
+- A member that left and joins again is the same participant record with `leftSeq` cleared and a new `joinedSeq` — not a second live row. `upsertFaceRoom` treats `startedAt` (or an explicit `open`) as an open room so the Friends' Bots card does not stay on the matching countdown after **Enter chat**. A later snapshot whose status is not `open`, `closed`, or `left`, and that omits `startedAt`, does not demote a room that is already open, closed, or left back to `candidate` (#1072). The membership window clamps a stored `joinedSeq` above the room's own `room.seq` to `room.seq` on the read path (to the log's last seq for a room saved before `seq` existed), warns once per room, and never rewrites the saved state; the clamped window is memoised per room and reader and stays open once `room.seq` reaches and passes the stored floor, a rejoin being the one thing that drops it (#2754). The floor is resolved once per read (#2766): `linesFrom` works it out before the filter rather than asking `floorOf` for every line, so a room saved before `seq` existed — whose target is a reduce over the whole log — reads in linear instead of quadratic time, with the same visible lines and the same one-time warning. Covered by `core/test/room-floor-once.test.js`.
 
 ### FlyBot display copy
 

patches/ds-bot/files/src/host/room-membership.js

@@ -496,7 +496,12 @@ export function install(rt) {
     }
     return seq
   }
-  const linesFrom = (room, actor) => (room.log ?? []).filter(line => (line.seq ?? 0) >= floorOf(room, actor))
+  const linesFrom = (room, actor) => {
+    // The floor is one number for the whole read: a room saved before `seq` existed finds its clamp
+    // target with a reduce over the whole log, so asking per line reduced an n-line log n times.
+    const from = floorOf(room, actor)
+    return (room.log ?? []).filter(line => (line.seq ?? 0) >= from)
+  }
 
   rt.participantsOf = room => currentParticipants(room)
   rt.roomVisibleTo = (actor, roomId) => roomVisibleTo(state, actor, roomId)

patches/ds-bot/files/test/room-floor-once.test.js

@@ -0,0 +1,130 @@
+// A room saved before `seq` existed finds its clamp target with a reduce over the whole log, and
+// `linesFrom` (`core/src/host/room-membership.js`) used to call `floorOf` once per line — so reading
+// an n-line legacy room reduced n times over n lines and the read was quadratic. This counts the
+// reduces instead of timing them.
+//
+//   node --test core/test/room-floor-once.test.js
+import assert from 'node:assert/strict'
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { test } from 'node:test'
+import { install as installStore } from '../src/host/store.js'
+import { install as installMemberAuth } from '../src/host/member-auth.js'
+import { install as installAuthz } from '../src/host/authz.js'
+import { install as installRooms } from '../src/host/room-membership.js'
+import { install as installReach } from '../src/host/reach.js'
+
+function host(t, saved) {
+  const dir = mkdtempSync(join(tmpdir(), 'ds-bot-floor-once-'))
+  const statePath = join(dir, 'state.json')
+  writeFileSync(statePath, JSON.stringify(saved))
+  const warnings = []
+  const disposers = []
+  const rt = {
+    ctx: {
+      logger: { warn: (...args) => warnings.push(args) },
+      effect: execute => { const d = execute(); if (typeof d === 'function') disposers.push(d) },
+    },
+    home: dir,
+    statePath,
+    autoRoomName: () => 'Room',
+    retitleRoom: async () => {},
+    dropGroupSession: () => {},
+    warn: (...args) => warnings.push(args),
+  }
+  installStore(rt)
+  installMemberAuth(rt)
+  installAuthz(rt)
+  installRooms(rt)
+  installReach(rt)
+  t.after(async () => {
+    await Promise.all(disposers.splice(0).map(dispose => dispose()))
+    rmSync(dir, { recursive: true, force: true, maxRetries: 5 })
+  })
+  return { rt, warnings }
+}
+
+const N = 20000
+const BOB = { memberId: 'owner-2', displayName: 'Bob' }
+const ALICE = { memberId: 'owner-1', displayName: 'Alice' }
+
+class CountingLog extends Array {
+  reduce(...args) {
+    CountingLog.reduces += 1
+    return super.reduce(...args)
+  }
+}
+CountingLog.reduces = 0
+
+function fixture(withSeq = true) {
+  const log = CountingLog.from({ length: N }, (_, i) => ({
+    who: 'Alice', text: `line ${i + 1}`, seq: i + 1, authorId: 'owner-1', authorKind: 'human',
+  }))
+  return {
+    version: 2, revision: 1, mainBotIds: ['chief-a'],
+    members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'chief-a' } },
+    bots: {
+      'chief-a': {
+        id: 'chief-a', sessionId: 'chief-a', name: 'Chief A', createdAt: 1, ownerId: 'owner-1',
+      },
+    },
+    rooms: {
+      legacy: {
+        id: 'legacy', name: 'Legacy', members: ['chief-a'], admin: 'chief-a', named: true, log, seen: {},
+        // A room saved before the counter existed has no `seq` of its own; `withSeq: false` keeps it
+        // that way (a room with a counter is the control case).
+        ...(withSeq ? { seq: N } : {}),
+        participants: [
+          { kind: 'human', id: 'owner-1', role: 'admin', displayName: 'Alice', joinedSeq: 0, joinedAt: 1 },
+          { kind: 'human', id: 'owner-2', role: 'member', displayName: 'Bob', joinedSeq: 0, joinedAt: 1 },
+        ],
+      },
+    },
+    grants: {}, agents: {}, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {},
+  }
+}
+
+// Load, then hand the room a counting log and (for the legacy case) drop the counter the load gives
+// it — the shape this bench is about is the one that was saved before `seq` existed.
+async function loadedLegacy(t, withSeq) {
+  const { rt, warnings } = await host(t, fixture(withSeq))
+  await rt.load()
+  const room = rt.state.rooms.legacy
+  room.log = CountingLog.from(room.log)
+  if (!withSeq) delete room.seq
+  CountingLog.reduces = 0
+  return { rt, room, warnings }
+}
+
+test('a legacy room read computes its clamp target once, not once per line', async (t) => {
+  const { rt, room, warnings } = await loadedLegacy(t, false)
+  const started = Date.now()
+  const seen = rt.linesVisibleTo(room, BOB)
+  const ms = Date.now() - started
+  console.log(`legacy linesVisibleTo n=${N}: ${ms}ms, reduce calls=${CountingLog.reduces}`)
+  assert.equal(seen.length, N, 'every line is readable')
+  assert.equal(CountingLog.reduces, 1, `the clamp target is computed once per read, saw ${CountingLog.reduces}`)
+  assert.deepEqual(warnings, [], 'a room whose stored floor is healthy says nothing')
+})
+
+test('a legacy room history read computes its clamp target once too', async (t) => {
+  const { rt } = await loadedLegacy(t, false)
+  const started = Date.now()
+  const seen = rt.roomHistory('legacy', ALICE)
+  const ms = Date.now() - started
+  console.log(`legacy roomHistory n=${N}: ${ms}ms, reduce calls=${CountingLog.reduces}`)
+  assert.equal(seen.length, N)
+  assert.equal(CountingLog.reduces, 1, `the clamp target is computed once per read, saw ${CountingLog.reduces}`)
+})
+
+test('a room with a counter reads the same window without a per-line reduce', async (t) => {
+  const { rt, room, warnings } = await loadedLegacy(t, true)
+  const started = Date.now()
+  const seen = rt.linesVisibleTo(room, BOB)
+  const ms = Date.now() - started
+  console.log(`modern linesVisibleTo n=${N}: ${ms}ms, reduce calls=${CountingLog.reduces}`)
+  assert.equal(seen.length, N)
+  assert.equal(CountingLog.reduces, 0, 'a room with a counter never walks the log to find one')
+  assert.deepEqual(warnings, [])
+})