From f67e2b07f67d26079b61e2be7aaafbd70ccf4a8c Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Wed, 11 Nov 2026 14:39:55 +0800 Subject: [PATCH] bug(host): a trimmed room log makes read_group_chat report the wrong total (#2838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header was built from `kept.length`, which is only what the room log still holds: the store is capped at ROOM_LOG_MAX (60 here, 200 in room-membership.js) while `room.seq` counts every line ever appended. A room that had said 500 lines read as "newest 30 of 60", hiding the 440 lines the trim threw away — and the same arithmetic ignored the reader's floor, so a reader at line 300 read "newest 30 of 60" when 201 lines were said since it joined. The read now counts `dropped = max(0, room.seq - room.log.length)` and adds only the dropped lines at or above the reader's floor: `total = kept.length + max(0, dropped - max(0, from - 1))`. The caps stay where they are, the store still trims, and a room that never trimmed (or sits at or under the cap) reads byte-identically. Fixes #2838 Owner: Dev-1 Reviewer: Birfy Written-by: Dev-1 Bot: Dev-1 Claim-Id: 63eb2d9dca664fab94de573fdd70b3e0 Risk tier: T2 Base: e16c1d3caeeb0606f7babbe9fa37abb0e80e9739 --- .../2838-room-read-trimmed-count.fixed.md | 1 + core/src/host/groups.js | 11 ++- core/test/room-read-trimmed-count.test.js | 97 +++++++++++++++++++ docs/subsystems/core-host.md | 2 +- patches/ds-bot/0012-room-membership.patch | 13 ++- .../test/room-read-trimmed-count.test.js | 97 +++++++++++++++++++ 6 files changed, 215 insertions(+), 6 deletions(-) create mode 100644 changelog.d/2838-room-read-trimmed-count.fixed.md create mode 100644 core/test/room-read-trimmed-count.test.js create mode 100644 patches/ds-bot/files/test/room-read-trimmed-count.test.js diff --git a/changelog.d/2838-room-read-trimmed-count.fixed.md b/changelog.d/2838-room-read-trimmed-count.fixed.md new file mode 100644 index 000000000..3ad4594b6 --- /dev/null +++ b/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). diff --git a/core/src/host/groups.js b/core/src/host/groups.js index ce6a73080..379cb43fe 100644 --- a/core/src/host/groups.js +++ b/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)}` }, })) diff --git a/core/test/room-read-trimmed-count.test.js b/core/test/room-read-trimmed-count.test.js new file mode 100644 index 000000000..a65c5d815 --- /dev/null +++ b/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')}`) +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index b3db774f5..371c8ec29 100644 --- a/docs/subsystems/core-host.md +++ b/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 "" (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 "" (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 "" (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 "" (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) diff --git a/patches/ds-bot/0012-room-membership.patch b/patches/ds-bot/0012-room-membership.patch index 41c288b71..0f172effb 100644 --- a/patches/ds-bot/0012-room-membership.patch +++ b/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)}` }, })) diff --git a/patches/ds-bot/files/test/room-read-trimmed-count.test.js b/patches/ds-bot/files/test/room-read-trimmed-count.test.js new file mode 100644 index 000000000..a65c5d815 --- /dev/null +++ b/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')}`) +})