bug(host): a trimmed room log makes read_group_chat report the wrong total

changelog.d/2838-room-read-trimmed-count.fixed.md

@@ -0,0 +1 @@
+`read_group_chat` now counts the lines the room said, not only the lines its log still holds: a room that said 500 lines and keeps the newest 60 reads `Latest in "Ops" (newest 30 of 500, oldest first):` instead of `newest 30 of 60`, and a reader at floor line 300 reads `newest 30 of 201`. A room that never trimmed reads byte-identically, and the caps (`ROOM_READ_MAX` 30, `ROOM_LOG_MAX` 60, the membership store's 200) are unchanged. See [#2838](https://github.com/ThinkFlowLab/FlyBot/issues/2838).

core/src/host/groups.js

@@ -1049,8 +1049,15 @@ export function install(rt) {
       const lines = kept.slice(-ROOM_READ_MAX)
       if (lines.length === 0) return `"${room.name}" has no messages yet.`
       // A cut read says how much it left out, so a missing message does not read as never said (#2780).
-      const older = kept.length - lines.length
-      return `Latest in "${room.name}" (${older > 0 ? `newest ${lines.length} of ${kept.length}, ` : ''}oldest first):\n${transcript(lines)}`
+      // `kept.length` is only what the log still holds: the store is capped (60 here, 200 in
+      // room-membership.js) while `room.seq` counts every line ever appended, so a room that had
+      // said 500 lines read as "newest 30 of 60" with 440 of them silently gone (#2838). The
+      // dropped lines sit below the oldest retained one, so they count from the reader's floor to
+      // that point, never the lines before the reader joined.
+      const dropped = Math.max(0, (room.seq ?? 0) - (room.log ?? []).length)
+      const total = kept.length + Math.max(0, dropped - Math.max(0, from - 1))
+      const older = total - lines.length
+      return `Latest in "${room.name}" (${older > 0 ? `newest ${lines.length} of ${total}, ` : ''}oldest first):\n${transcript(lines)}`
     },
   }))
 

core/test/room-read-trimmed-count.test.js

@@ -0,0 +1,97 @@
+// P-684: a busy room's read counts what the disk still holds, not what the room said. `room.log` is
+// capped at ROOM_LOG_MAX (60; 200 from room-membership.js) while `room.seq` keeps counting every line
+// ever appended, so `kept.length` is a window — yet the header printed it as the total, telling a Bot
+// `newest 30 of 60` for a room that had said 500 lines.
+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 { createRuntime, provideFlyBot } from '../index.js'
+import { fakeContext } from './fake-ctx.js'
+
+const T0 = 1_760_000_000_000
+const line = (seq, who, text) => ({ seq, who, text, time: T0 + seq * 1000 })
+const lines = (count, from = 1) => Array.from({ length: count }, (_, i) => line(from + i, (from + i) % 2 === 0 ? 'Chief' : 'Writer', `message ${from + i}: the merge queue slipped again`))
+const shownLines = out => out.split('\n').filter(l => /^(Chief|Writer): /.test(l))
+const seat = (id, joinedSeq) => ({ kind: 'bot', id, role: id === 'a' ? 'admin' : 'member', displayName: id === 'a' ? 'Chief' : 'Writer', joinedSeq, joinedAt: T0 })
+
+async function start(t, log, room = {}) {
+  const home = mkdtempSync(join(tmpdir(), 'ds-bot-p684-'))
+  const seed = {
+    version: 1,
+    mainBotIds: ['a'],
+    bots: {
+      a: { id: 'a', name: 'Chief', role: 'Boss', sessionId: 'a', createdAt: 1 },
+      b: { id: 'b', name: 'Writer', role: 'Write', sessionId: 'b', createdAt: 2 },
+    },
+    rooms: { r1: { id: 'r1', name: 'Ops', members: ['a', 'b'], mode: 'admin', admin: 'a', named: true, sessions: { a: 'ga', b: 'gb' }, log, seq: (log.at(-1)?.seq ?? 0), seen: {}, ...room } },
+    agents: {},
+    exchanges: [],
+    exchangeTurns: {},
+    questions: {},
+    prefs: {},
+    revision: 0,
+  }
+  writeFileSync(join(home, 'state.json'), JSON.stringify(seed))
+  const ctx = fakeContext()
+  const agentOf = id => ({ session: { id, seq: 0, deriveMessages: () => [] }, calls: [], inbox: { nextStep: [], nextTurn: [], remove: () => false }, followup: () => {}, steer: () => {}, cancel() {} })
+  Object.assign(ctx, {
+    llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) },
+    workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} },
+    sessionController: { create: async () => ({ sessionId: 's1' }), resolveAgent: async id => ({ agent: agentOf(id) }), selectModel: async () => {}, page: async () => ({ records: [], hasMore: false }) },
+    sessionTitle: { rename: async () => {} },
+    sessions: { flush: async () => {} },
+  })
+  const rt = createRuntime(ctx, { home })
+  provideFlyBot(ctx, rt.flybot)
+  t.after(async () => {
+    await ctx.dispose()
+    rmSync(home, { recursive: true, force: true, maxRetries: 5, retryDelay: 20 })
+  })
+  const tool = (name, args, sessionId) => ctx.registeredTools.get(name).execute(args, { agent: { id: sessionId } })
+  await tool('list_bots', {}, 'a')
+  return { rt, tool }
+}
+
+// RED today: 500 lines were said, the cap kept the newest 60, and the read calls the kept 60 the total.
+test('P-684: a trimmed room read reports the lines the room said, not the lines it kept', async (t) => {
+  const host = await start(t, lines(60, 441), { seq: 500 })
+  const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb')
+  assert.equal(shownLines(out).length, 30)
+  assert.match(out, /^Latest in "Ops" \(newest 30 of 500, oldest first\):$/m, `the total counts what the log still holds, not what the room said: ${out.split('\n')[0]}`)
+  assert.match(out, /^(Chief|Writer): message 500: /m, 'the newest line is still shown')
+})
+
+// RED today: the hidden lines between the reader's floor and the oldest retained line are part of the
+// total, but the lines the reader joined after are not.
+test('P-684: the total is counted from the reader\'s floor, not from the start of the room', async (t) => {
+  const host = await start(t, lines(60, 441), { seq: 500, participants: [seat('a', 0), seat('b', 300)] })
+  const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb')
+  assert.equal(shownLines(out).length, 30)
+  assert.match(out, /^Latest in "Ops" \(newest 30 of 201, oldest first\):$/m, `the total ignores the reader's floor: ${out.split('\n')[0]}`)
+})
+
+// GREEN today, must stay: a reader whose floor is above every dropped line sees all of its window, so
+// there is nothing to report as left out.
+test('P-684 guard: a late joiner in a trimmed room is not told about lines it joined after', async (t) => {
+  const host = await start(t, lines(60, 441), { seq: 500, participants: [seat('a', 0), seat('b', 480)] })
+  const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb')
+  assert.equal(shownLines(out).length, 21)
+  assert.doesNotMatch(out, /newest \d+ of \d+/)
+})
+
+// GREEN today, must stay: a log that reached the cap without dropping a line still counts its own 60.
+test('P-684 guard: a room at exactly the cap is unchanged', async (t) => {
+  const host = await start(t, lines(60))
+  const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb')
+  assert.match(out, /^Latest in "Ops" \(newest 30 of 60, oldest first\):$/m)
+})
+
+// GREEN today, must stay: a room under the cap reads exactly as the uncut room it is.
+test('P-684 guard: a room under the cap is byte-identical', async (t) => {
+  const said = lines(20)
+  const host = await start(t, said)
+  const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb')
+  assert.equal(out, `Latest in "Ops" (oldest first):\n${said.map(l => `${l.who}: ${l.text}`).join('\n')}`)
+})

docs/subsystems/core-host.md

@@ -433,7 +433,7 @@ has lines (#2790, `core/test/room-read-floor-clamp.test.js`).
 
 ### Group reads say what a cap left out (#2780)
 
-- `read_group_chat` shows at most `ROOM_READ_MAX` (30) of the room lines visible to the caller, the newest ones, and the cut used to be silent: `Latest in "<room>" (oldest first):` printed as if those 30 were the whole log, so a Bot asked "didn't we decide X?" read a truncated log as the whole log — the same failure #1906 fixed for the own-chat read. The header now names the cut, `Latest in "<room>" (newest 30 of 45, oldest first):`, and a read that fits the cap stays byte-identical; lines before the member's `joinedSeq` are not counted as left out. Covered by `core/test/room-read-honest-count.test.js` and `core/test/group-work.test.js`.
+- `read_group_chat` shows at most `ROOM_READ_MAX` (30) of the room lines visible to the caller, the newest ones, and the cut used to be silent: `Latest in "<room>" (oldest first):` printed as if those 30 were the whole log, so a Bot asked "didn't we decide X?" read a truncated log as the whole log — the same failure #1906 fixed for the own-chat read. The header now names the cut, `Latest in "<room>" (newest 30 of 45, oldest first):`, and a read that fits the cap stays byte-identical; lines before the member's `joinedSeq` are not counted as left out, and a room whose log was trimmed counts the lines it dropped (the store keeps only the newest `ROOM_LOG_MAX` lines, 60 here and 200 in `room-membership.js`, while `room.seq` keeps counting every line, so 500 lines said and 60 kept read `newest 30 of 500`). Covered by `core/test/room-read-honest-count.test.js`, `core/test/room-read-trimmed-count.test.js` and `core/test/group-work.test.js`.
 
 ### Chat anchors name their part (#2710)
 

patches/ds-bot/0012-room-membership.patch

@@ -2118,7 +2118,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js
      },
    }))
 
-@@ -539,8 +539,16 @@ export function install(rt) {
+@@ -539,8 +539,23 @@ export function install(rt) {
        const { room, error } = rt.pickRoom(args.group, self)
        if (error) return error
        ensureRoomLog(room)
@@ -2132,8 +2132,15 @@ diff --git a/src/host/groups.js b/src/host/groups.js
 +      const lines = kept.slice(-ROOM_READ_MAX)
 +      if (lines.length === 0) return `"${room.name}" has no messages yet.`
 +      // A cut read says how much it left out, so a missing message does not read as never said (#2780).
-+      const older = kept.length - lines.length
-+      return `Latest in "${room.name}" (${older > 0 ? `newest ${lines.length} of ${kept.length}, ` : ''}oldest first):\n${transcript(lines)}`
++      // `kept.length` is only what the log still holds: the store is capped (60 here, 200 in
++      // room-membership.js) while `room.seq` counts every line ever appended, so a room that had
++      // said 500 lines read as "newest 30 of 60" with 440 of them silently gone (#2838). The
++      // dropped lines sit below the oldest retained one, so they count from the reader's floor to
++      // that point, never the lines before the reader joined.
++      const dropped = Math.max(0, (room.seq ?? 0) - (room.log ?? []).length)
++      const total = kept.length + Math.max(0, dropped - Math.max(0, from - 1))
++      const older = total - lines.length
++      return `Latest in "${room.name}" (${older > 0 ? `newest ${lines.length} of ${total}, ` : ''}oldest first):\n${transcript(lines)}`
      },
    }))
 

patches/ds-bot/files/test/room-read-trimmed-count.test.js

@@ -0,0 +1,97 @@
+// P-684: a busy room's read counts what the disk still holds, not what the room said. `room.log` is
+// capped at ROOM_LOG_MAX (60; 200 from room-membership.js) while `room.seq` keeps counting every line
+// ever appended, so `kept.length` is a window — yet the header printed it as the total, telling a Bot
+// `newest 30 of 60` for a room that had said 500 lines.
+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 { createRuntime, provideFlyBot } from '../index.js'
+import { fakeContext } from './fake-ctx.js'
+
+const T0 = 1_760_000_000_000
+const line = (seq, who, text) => ({ seq, who, text, time: T0 + seq * 1000 })
+const lines = (count, from = 1) => Array.from({ length: count }, (_, i) => line(from + i, (from + i) % 2 === 0 ? 'Chief' : 'Writer', `message ${from + i}: the merge queue slipped again`))
+const shownLines = out => out.split('\n').filter(l => /^(Chief|Writer): /.test(l))
+const seat = (id, joinedSeq) => ({ kind: 'bot', id, role: id === 'a' ? 'admin' : 'member', displayName: id === 'a' ? 'Chief' : 'Writer', joinedSeq, joinedAt: T0 })
+
+async function start(t, log, room = {}) {
+  const home = mkdtempSync(join(tmpdir(), 'ds-bot-p684-'))
+  const seed = {
+    version: 1,
+    mainBotIds: ['a'],
+    bots: {
+      a: { id: 'a', name: 'Chief', role: 'Boss', sessionId: 'a', createdAt: 1 },
+      b: { id: 'b', name: 'Writer', role: 'Write', sessionId: 'b', createdAt: 2 },
+    },
+    rooms: { r1: { id: 'r1', name: 'Ops', members: ['a', 'b'], mode: 'admin', admin: 'a', named: true, sessions: { a: 'ga', b: 'gb' }, log, seq: (log.at(-1)?.seq ?? 0), seen: {}, ...room } },
+    agents: {},
+    exchanges: [],
+    exchangeTurns: {},
+    questions: {},
+    prefs: {},
+    revision: 0,
+  }
+  writeFileSync(join(home, 'state.json'), JSON.stringify(seed))
+  const ctx = fakeContext()
+  const agentOf = id => ({ session: { id, seq: 0, deriveMessages: () => [] }, calls: [], inbox: { nextStep: [], nextTurn: [], remove: () => false }, followup: () => {}, steer: () => {}, cancel() {} })
+  Object.assign(ctx, {
+    llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) },
+    workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} },
+    sessionController: { create: async () => ({ sessionId: 's1' }), resolveAgent: async id => ({ agent: agentOf(id) }), selectModel: async () => {}, page: async () => ({ records: [], hasMore: false }) },
+    sessionTitle: { rename: async () => {} },
+    sessions: { flush: async () => {} },
+  })
+  const rt = createRuntime(ctx, { home })
+  provideFlyBot(ctx, rt.flybot)
+  t.after(async () => {
+    await ctx.dispose()
+    rmSync(home, { recursive: true, force: true, maxRetries: 5, retryDelay: 20 })
+  })
+  const tool = (name, args, sessionId) => ctx.registeredTools.get(name).execute(args, { agent: { id: sessionId } })
+  await tool('list_bots', {}, 'a')
+  return { rt, tool }
+}
+
+// RED today: 500 lines were said, the cap kept the newest 60, and the read calls the kept 60 the total.
+test('P-684: a trimmed room read reports the lines the room said, not the lines it kept', async (t) => {
+  const host = await start(t, lines(60, 441), { seq: 500 })
+  const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb')
+  assert.equal(shownLines(out).length, 30)
+  assert.match(out, /^Latest in "Ops" \(newest 30 of 500, oldest first\):$/m, `the total counts what the log still holds, not what the room said: ${out.split('\n')[0]}`)
+  assert.match(out, /^(Chief|Writer): message 500: /m, 'the newest line is still shown')
+})
+
+// RED today: the hidden lines between the reader's floor and the oldest retained line are part of the
+// total, but the lines the reader joined after are not.
+test('P-684: the total is counted from the reader\'s floor, not from the start of the room', async (t) => {
+  const host = await start(t, lines(60, 441), { seq: 500, participants: [seat('a', 0), seat('b', 300)] })
+  const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb')
+  assert.equal(shownLines(out).length, 30)
+  assert.match(out, /^Latest in "Ops" \(newest 30 of 201, oldest first\):$/m, `the total ignores the reader's floor: ${out.split('\n')[0]}`)
+})
+
+// GREEN today, must stay: a reader whose floor is above every dropped line sees all of its window, so
+// there is nothing to report as left out.
+test('P-684 guard: a late joiner in a trimmed room is not told about lines it joined after', async (t) => {
+  const host = await start(t, lines(60, 441), { seq: 500, participants: [seat('a', 0), seat('b', 480)] })
+  const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb')
+  assert.equal(shownLines(out).length, 21)
+  assert.doesNotMatch(out, /newest \d+ of \d+/)
+})
+
+// GREEN today, must stay: a log that reached the cap without dropping a line still counts its own 60.
+test('P-684 guard: a room at exactly the cap is unchanged', async (t) => {
+  const host = await start(t, lines(60))
+  const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb')
+  assert.match(out, /^Latest in "Ops" \(newest 30 of 60, oldest first\):$/m)
+})
+
+// GREEN today, must stay: a room under the cap reads exactly as the uncut room it is.
+test('P-684 guard: a room under the cap is byte-identical', async (t) => {
+  const said = lines(20)
+  const host = await start(t, said)
+  const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb')
+  assert.equal(out, `Latest in "Ops" (oldest first):\n${said.map(l => `${l.who}: ${l.text}`).join('\n')}`)
+})