diff --git a/changelog.d/2780-room-read-honest-count.fixed.md b/changelog.d/2780-room-read-honest-count.fixed.md new file mode 100644 index 000000000..291621151 --- /dev/null +++ b/changelog.d/2780-room-read-honest-count.fixed.md @@ -0,0 +1 @@ +`read_group_chat` now says how many room lines its 30-line cap left out — `Latest in "" (newest 30 of 45, oldest first):` — instead of printing a cut log as the whole log; an uncut read is byte-identical ([#2780](https://github.com/ThinkFlowLab/FlyBot/issues/2780)). diff --git a/core/src/host/groups.js b/core/src/host/groups.js index d93dd542d..d02002077 100644 --- a/core/src/host/groups.js +++ b/core/src/host/groups.js @@ -1043,8 +1043,12 @@ export function install(rt) { ensureRoomLog(room) const botPart = rt.participantsOf?.(room)?.find(p => p.kind === 'bot' && p.id === self) const from = botPart?.joinedSeq ?? 0 - const lines = room.log.filter(line => (line.seq ?? 0) >= from).slice(-ROOM_READ_MAX) - return lines.length === 0 ? `"${room.name}" has no messages yet.` : `Latest in "${room.name}" (oldest first):\n${transcript(lines)}` + const kept = room.log.filter(line => (line.seq ?? 0) >= from) + 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)}` }, })) diff --git a/core/test/room-read-honest-count.test.js b/core/test/room-read-honest-count.test.js new file mode 100644 index 000000000..b3dbe2e46 --- /dev/null +++ b/core/test/room-read-honest-count.test.js @@ -0,0 +1,83 @@ +// #2780: read_group_chat shows the newest 30 room lines and says nothing about the rest, so a +// cut log reads as the whole log. +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 = 0) => Array.from({ length: count }, (_, i) => line(from + i, from + i === 0 || (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)) + +async function start(t, log, room = {}) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-p658-')) + 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 ?? -1), 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 { tool } +} + +// RED today: 45 stored lines, the newest 30 shown, and nothing says 15 were left out. +test('#2780: a room read that cuts the log says how much it left out', async (t) => { + const host = await start(t, lines(45)) + const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb') + assert.equal(shownLines(out).length, 30) + assert.match(out, /newest 30 of 45/, `no count of what was left out: ${out.split('\n')[0]}`) + assert.match(out, /^Latest in "Ops" \(newest 30 of 45, oldest first\):$/m) +}) + +// GREEN today, must stay byte-for-byte: a log shorter than the cap reads exactly as now. +test('#2780 guard: a room log under the cap is unchanged', async (t) => { + const host = await start(t, lines(20)) + const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb') + assert.equal(out, `Latest in "Ops" (oldest first):\n${lines(20).map(l => `${l.who}: ${l.text}`).join('\n')}`) +}) + +// GREEN today, must stay: lines before the member joined are not "left out" by the cap. +test('#2780 guard: a member that joined late is not told about lines before it joined', async (t) => { + const host = await start(t, lines(45), { participants: [{ kind: 'bot', id: 'a', role: 'admin', displayName: 'Chief', joinedSeq: 0, joinedAt: T0 }, { kind: 'bot', id: 'b', role: 'member', displayName: 'Writer', joinedSeq: 20, joinedAt: T0 }] }) + const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb') + assert.equal(shownLines(out).length, 25) + assert.doesNotMatch(out, /newest \d+ of/) +}) + +// GREEN today, must stay: an empty room keeps its sentence. +test('#2780 guard: an empty room is unchanged', async (t) => { + const host = await start(t, []) + const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb') + assert.equal(out, '"Ops" has no messages yet.') +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index c31d5eafa..37bdf4149 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -430,6 +430,10 @@ Work a member hands a Bot with `message_bot` from its group Session runs in a hi - `chat-index.js` returns the count next to the lines — `searchOwnChat` answers `{ lines, every, more }`, `latestOwnChat` `{ lines, more }` — instead of the lines alone. A search counts the same match in all only once its limit was filled (`termCountQuery`; the per-term fallback uses `anyTermCountQuery`, which unions one branch per term so a line matching two counts once), and a latest read counts the part's visible lines (`countAll` / `countVisible`, the latter leaving out the asking turn). Without the SQLite index the log scan counts every match while it reads, because that count is the only way to say what was left out. - Covered by `core/test/group-work.test.js`; `core/test/history.test.js` still pins `termQuery`'s SQL. +### 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`. + ### Chat anchors name their part (#2710) - `chat-index.js` `anchorOf` prints every anchor as `part N #` — the form `aroundOwnChat` resolves exactly — including while the chat still has one part. The bare `#` it printed there used to resolve against `parts[parts.length - 1]`, so once `parts.js` ended a segment and a fresh part became current, an anchor a Bot had saved opened a different message in the new part and reported `exact: true`. diff --git a/patches/ds-bot/0012-room-membership.patch b/patches/ds-bot/0012-room-membership.patch index 3909a874e..52f69a1ef 100644 --- a/patches/ds-bot/0012-room-membership.patch +++ b/patches/ds-bot/0012-room-membership.patch @@ -2118,17 +2118,23 @@ diff --git a/src/host/groups.js b/src/host/groups.js }, })) -@@ -539,7 +539,9 @@ export function install(rt) { +@@ -539,8 +539,14 @@ export function install(rt) { const { room, error } = rt.pickRoom(args.group, self) if (error) return error ensureRoomLog(room) - const lines = room.log.slice(-ROOM_READ_MAX) +- return lines.length === 0 ? `"${room.name}" has no messages yet.` : `Latest in "${room.name}" (oldest first):\n${transcript(lines)}` + const botPart = rt.participantsOf?.(room)?.find(p => p.kind === 'bot' && p.id === self) + const from = botPart?.joinedSeq ?? 0 -+ const lines = room.log.filter(line => (line.seq ?? 0) >= from).slice(-ROOM_READ_MAX) - return lines.length === 0 ? `"${room.name}" has no messages yet.` : `Latest in "${room.name}" (oldest first):\n${transcript(lines)}` ++ const kept = room.log.filter(line => (line.seq ?? 0) >= from) ++ 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)}` }, })) + @@ -548,46 +548,264 @@ export function install(rt) { // the admin answers alone in admin mode, or first of everyone. What a member posts // after its round moved on is answered by the member that asked for it; work the diff --git a/patches/ds-bot/files/test/room-read-honest-count.test.js b/patches/ds-bot/files/test/room-read-honest-count.test.js new file mode 100644 index 000000000..b3dbe2e46 --- /dev/null +++ b/patches/ds-bot/files/test/room-read-honest-count.test.js @@ -0,0 +1,83 @@ +// #2780: read_group_chat shows the newest 30 room lines and says nothing about the rest, so a +// cut log reads as the whole log. +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 = 0) => Array.from({ length: count }, (_, i) => line(from + i, from + i === 0 || (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)) + +async function start(t, log, room = {}) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-p658-')) + 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 ?? -1), 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 { tool } +} + +// RED today: 45 stored lines, the newest 30 shown, and nothing says 15 were left out. +test('#2780: a room read that cuts the log says how much it left out', async (t) => { + const host = await start(t, lines(45)) + const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb') + assert.equal(shownLines(out).length, 30) + assert.match(out, /newest 30 of 45/, `no count of what was left out: ${out.split('\n')[0]}`) + assert.match(out, /^Latest in "Ops" \(newest 30 of 45, oldest first\):$/m) +}) + +// GREEN today, must stay byte-for-byte: a log shorter than the cap reads exactly as now. +test('#2780 guard: a room log under the cap is unchanged', async (t) => { + const host = await start(t, lines(20)) + const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb') + assert.equal(out, `Latest in "Ops" (oldest first):\n${lines(20).map(l => `${l.who}: ${l.text}`).join('\n')}`) +}) + +// GREEN today, must stay: lines before the member joined are not "left out" by the cap. +test('#2780 guard: a member that joined late is not told about lines before it joined', async (t) => { + const host = await start(t, lines(45), { participants: [{ kind: 'bot', id: 'a', role: 'admin', displayName: 'Chief', joinedSeq: 0, joinedAt: T0 }, { kind: 'bot', id: 'b', role: 'member', displayName: 'Writer', joinedSeq: 20, joinedAt: T0 }] }) + const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb') + assert.equal(shownLines(out).length, 25) + assert.doesNotMatch(out, /newest \d+ of/) +}) + +// GREEN today, must stay: an empty room keeps its sentence. +test('#2780 guard: an empty room is unchanged', async (t) => { + const host = await start(t, []) + const out = await host.tool('read_group_chat', { group: 'Ops' }, 'gb') + assert.equal(out, '"Ops" has no messages yet.') +})