diff --git a/changelog.d/2443-preview-fence-words.fixed.md b/changelog.d/2443-preview-fence-words.fixed.md new file mode 100644 index 000000000..28e916cad --- /dev/null +++ b/changelog.d/2443-preview-fence-words.fixed.md @@ -0,0 +1 @@ +A preview now keeps the words on either side of a quoted remote line: a reply that quoted a line and then answered it showed the quote alone in the sidebar and the palette row. diff --git a/core/client.js b/core/client.js index 0302a97ec..ec102d2cc 100644 --- a/core/client.js +++ b/core/client.js @@ -2467,10 +2467,12 @@ function visibleText(text2) { for (let previous; previous !== out; ) { previous = out; const open = out.indexOf(""); - const inner = close > open ? out.slice(out.indexOf(">", open) + 1, close) : out; - out = stripFence(inner); + if (open < 0 || close <= open) break; + const head = out.slice(0, open); + const inner = out.slice(out.indexOf(">", open) + 1, close); + const tail = out.slice(close + "".length); + out = stripFence(head) + inner + stripFence(tail); } return stripFence(out).replace(/\s+/g, " ").trim(); } diff --git a/core/src/shared/preview.js b/core/src/shared/preview.js index 980e1e426..86c606dbb 100644 --- a/core/src/shared/preview.js +++ b/core/src/shared/preview.js @@ -27,16 +27,20 @@ export function stripFence(text) { } // The words of a stored line as a preview shows them: inner text of a wrap, no marker of any shape, -// and one line, because a row is one line. +// and one line, because a row is one line. A wrap in the middle of a line loses its markers in place, +// so the sentence around it survives: a reply that quotes a remote line and then answers it reads as +// the reply, not as the quote. export function visibleText(text) { let out = String(text ?? '') for (let previous; previous !== out;) { previous = out const open = out.indexOf('') - const inner = close > open ? out.slice(out.indexOf('>', open) + 1, close) : out - out = stripFence(inner) + if (open < 0 || close <= open) break + const head = out.slice(0, open) + const inner = out.slice(out.indexOf('>', open) + 1, close) + const tail = out.slice(close + ''.length) + out = stripFence(head) + inner + stripFence(tail) } return stripFence(out).replace(/\s+/g, ' ').trim() } diff --git a/core/test/preview-fence-words.test.js b/core/test/preview-fence-words.test.js new file mode 100644 index 000000000..22d06805a --- /dev/null +++ b/core/test/preview-fence-words.test.js @@ -0,0 +1,124 @@ +// A preview is still the line someone said (#1347): the fence is data, but so are the words on +// either side of it. Four readers show a stored line to a person — the Bot chat's row +// (`readableLine`/`previewOf`), a sidebar row, the exchange sheet under ⌘K, and the host's room +// preview on the /api/bot state answer — and every one of them must keep the words around a +// complete wrap, not only the quoted half. +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { fileURLToPath } from 'node:url' +import { runInNewContext } from 'node:vm' +import { build } from 'esbuild' +import { install } from '../src/host/channel.js' +import { previewOf, readableLine } from '../src/client/text.js' + +const MIXED = 'He said quoted and I answered done.' +const KEPT = 'He said quoted and I answered done.' + +const root = fileURLToPath(new URL('..', import.meta.url)) +const h = (type, props, ...children) => ({ type, props: { ...(props ?? {}), children: children.flat(Infinity) } }) +class Component { constructor(props) { this.props = props } setState() {} } +const react = { + createElement: h, Fragment: Symbol('Fragment'), Component, PureComponent: Component, + createRef: () => ({ current: null }), useState: value => [typeof value === 'function' ? value() : value, () => {}], + useEffect() {}, useLayoutEffect() {}, useRef: value => ({ current: value }), useCallback: fn => fn, + useMemo: fn => fn(), useReducer: (fn, initial) => [initial, () => {}], +} +const slots = { entries: () => [], subscribe: () => () => {}, version: () => 0 } + +// A client module is bundled and loaded the way the client tests load one (sidebar-readonly.test.js): +// esbuild for the browser half, a stub react in the context, so the module's own code runs here. +async function clientModule(contents) { + const { outputFiles } = await build({ + stdin: { contents, resolveDir: root }, + bundle: true, format: 'cjs', platform: 'browser', write: false, logLevel: 'silent', + external: ['react', 'react-dom', '@deepseek-ai/dsh-client-ui-primitives'], + }) + const mod = { exports: {} } + runInNewContext(outputFiles[0].text, { + module: mod, exports: mod.exports, console, Date, JSON, Promise, Symbol, setTimeout, clearTimeout, + __flybot__: { slots, call: () => {} }, + require: name => (name === 'react' ? react : name === 'react-dom' ? { createPortal: node => node } : {}), + }) + return mod.exports +} + +// Elements stay data; a function component is called so a row's own code runs, and the strings it +// draws are collected in order. +function drawnText(node, seen = []) { + if (node === null || node === undefined) return seen + if (typeof node === 'string') { seen.push(node); return seen } + if (typeof node !== 'object') return seen + if (Array.isArray(node)) { for (const child of node) drawnText(child, seen); return seen } + if (typeof node.type === 'function') { drawnText(node.type(node.props), seen); return seen } + for (const child of node.props?.children ?? []) drawnText(child, seen) + return seen +} + +test('the Bot chat row keeps the words before and after a complete wrap', () => { + assert.equal(readableLine(MIXED), KEPT) + const list = { projectionsBySession: { s1: { values: { turnOutline: [{ turn: 1, response: MIXED }] } } }, byId: {} } + assert.equal(previewOf(list, 's1'), KEPT) + assert.equal(readableLine('a one b two c'), 'a one b two c') +}) + +test('the sidebar row of a room keeps them too', async () => { + const { TeamSidebar } = await clientModule("export { TeamSidebar } from './src/client/sidebar.js'") + const room = { id: 'r1', name: 'Team room', kind: 'group', createdAt: 1, preview: MIXED } + const list = { ids: ['r1'], byId: { r1: { updatedAt: 1 } } } + const useSessions = selector => selector(list) + const useUi = selector => selector({ unread: {} }) + const tree = TeamSidebar({ + wide: true, + useRoster: selector => selector({ bots: [], byId: {}, rooms: [room], mainBotId: 'm', agents: [] }), + useUi, + useSessions, + useRegistry: useUi, + useWork: selector => selector({}), + useSessionStatus: selector => selector(new Map()), + useSessionRetainInfo: (id, selector) => selector({}), + useActivity: selector => selector({}), + useUpdate: selector => selector({}), + actions: {}, + }) + const words = drawnText(tree) + assert.ok(words.includes(KEPT), `the row drew ${JSON.stringify(words.filter(word => /quoted|He said|answered/.test(word)))}`) +}) + +test('the host room preview on the state answer keeps them too', async () => { + const state = { + revision: 3, workspaceId: null, mainBotIds: ['main'], + bots: { main: { id: 'main', name: 'Main' } }, + rooms: { r1: { id: 'r1', name: 'Team room', createdAt: 1, log: [{ text: MIXED, who: 'User', authorKind: 'human' }] } }, + humans: {}, agents: {}, questions: {}, prefs: {}, secretRequests: {}, + } + let route + const rt = { + state, + ctx: { effect: (fn) => { fn(); return () => {} }, connection: { fetch: { register: (r) => { route = r; return () => {} } } } }, + warn: () => {}, noteFailure: () => {}, + readOnly: () => undefined, authenticate: async () => null, authorize: () => ({ ok: true }), + bootstrap: async () => {}, catalog: async () => [], loadSecrets: async () => {}, + cachedCatalog: () => [], cachedDefault: () => null, + chatOf: id => `chat-${id}`, ownedLive: full => full, ownedView: full => full, load: async () => {}, refresh: async () => {}, + pluginToolNames: () => [], publicHumans: () => [], presentRoom: room => room, + secretsView: () => [], capabilityLibraryView: () => ({}), flybotRemoteMembers: () => [], + permissionCatalog: () => null, browserEnabled: false, memoryEnabled: true, authzEnforced: () => false, + listRoomApprovals: () => [], roomBusyAcksView: () => ({}), + activityView: () => [], busyAcksView: () => ({}), workView: () => ({}), roomWorkView: () => ({}), + approvalsView: () => ({}), endedView: () => [], memoryNews: () => [], memoryReviews: () => [], + connectorsView: () => [], capabilityStatusesView: () => [], stateLinkReplaced: () => ({ gen: 'g', count: 0 }), + } + install(rt) + const answer = await route.fetch(new Request('http://host/api/bot', { + method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify({ endpoint: 'state' }), + })) + const body = await answer.json() + assert.equal(body.ok, true, JSON.stringify(body.error)) + assert.equal(body.value.rooms[0].preview, KEPT) +}) + +test('the exchange sheet under the palette already kept them (control, unchanged by this fix)', async () => { + const { exchangeText } = await clientModule("export { exchangeText } from './src/client/exchange.js'") + assert.equal(exchangeText(MIXED), KEPT) + assert.equal(exchangeText('\nhello there\n'), 'hello there') +}) diff --git a/patches/ds-bot/0008-client-bundle.patch b/patches/ds-bot/0008-client-bundle.patch index f379ff6bf..2542548ae 100644 --- a/patches/ds-bot/0008-client-bundle.patch +++ b/patches/ds-bot/0008-client-bundle.patch @@ -1908,7 +1908,7 @@ diff --git a/client.js b/client.js const skip = [ ...[...value.matchAll(LITERAL)].map((match) => ({ start: match.index, end: match.index + match[0].length })), ...referenceSpans(value, references) -@@ -1445,24 +1445,86 @@ function mentionSpans(text, bots, { references = [] } = {}) { +@@ -1445,24 +1445,88 @@ function mentionSpans(text, bots, { references = [] } = {}) { } return spans; } @@ -1952,10 +1952,12 @@ diff --git a/client.js b/client.js + for (let previous; previous !== out; ) { + previous = out; + const open = out.indexOf(""); -+ const inner = close > open ? out.slice(out.indexOf(">", open) + 1, close) : out; -+ out = stripFence(inner); ++ if (open < 0 || close <= open) break; ++ const head = out.slice(0, open); ++ const inner = out.slice(out.indexOf(">", open) + 1, close); ++ const tail = out.slice(close + "".length); ++ out = stripFence(head) + inner + stripFence(tail); + } + return stripFence(out).replace(/\s+/g, " ").trim(); +} diff --git a/patches/ds-bot/files/src/shared/preview.js b/patches/ds-bot/files/src/shared/preview.js index 980e1e426..86c606dbb 100644 --- a/patches/ds-bot/files/src/shared/preview.js +++ b/patches/ds-bot/files/src/shared/preview.js @@ -27,16 +27,20 @@ export function stripFence(text) { } // The words of a stored line as a preview shows them: inner text of a wrap, no marker of any shape, -// and one line, because a row is one line. +// and one line, because a row is one line. A wrap in the middle of a line loses its markers in place, +// so the sentence around it survives: a reply that quotes a remote line and then answers it reads as +// the reply, not as the quote. export function visibleText(text) { let out = String(text ?? '') for (let previous; previous !== out;) { previous = out const open = out.indexOf('') - const inner = close > open ? out.slice(out.indexOf('>', open) + 1, close) : out - out = stripFence(inner) + if (open < 0 || close <= open) break + const head = out.slice(0, open) + const inner = out.slice(out.indexOf('>', open) + 1, close) + const tail = out.slice(close + ''.length) + out = stripFence(head) + inner + stripFence(tail) } return stripFence(out).replace(/\s+/g, ' ').trim() } diff --git a/patches/ds-bot/files/test/preview-fence-words.test.js b/patches/ds-bot/files/test/preview-fence-words.test.js new file mode 100644 index 000000000..22d06805a --- /dev/null +++ b/patches/ds-bot/files/test/preview-fence-words.test.js @@ -0,0 +1,124 @@ +// A preview is still the line someone said (#1347): the fence is data, but so are the words on +// either side of it. Four readers show a stored line to a person — the Bot chat's row +// (`readableLine`/`previewOf`), a sidebar row, the exchange sheet under ⌘K, and the host's room +// preview on the /api/bot state answer — and every one of them must keep the words around a +// complete wrap, not only the quoted half. +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { fileURLToPath } from 'node:url' +import { runInNewContext } from 'node:vm' +import { build } from 'esbuild' +import { install } from '../src/host/channel.js' +import { previewOf, readableLine } from '../src/client/text.js' + +const MIXED = 'He said quoted and I answered done.' +const KEPT = 'He said quoted and I answered done.' + +const root = fileURLToPath(new URL('..', import.meta.url)) +const h = (type, props, ...children) => ({ type, props: { ...(props ?? {}), children: children.flat(Infinity) } }) +class Component { constructor(props) { this.props = props } setState() {} } +const react = { + createElement: h, Fragment: Symbol('Fragment'), Component, PureComponent: Component, + createRef: () => ({ current: null }), useState: value => [typeof value === 'function' ? value() : value, () => {}], + useEffect() {}, useLayoutEffect() {}, useRef: value => ({ current: value }), useCallback: fn => fn, + useMemo: fn => fn(), useReducer: (fn, initial) => [initial, () => {}], +} +const slots = { entries: () => [], subscribe: () => () => {}, version: () => 0 } + +// A client module is bundled and loaded the way the client tests load one (sidebar-readonly.test.js): +// esbuild for the browser half, a stub react in the context, so the module's own code runs here. +async function clientModule(contents) { + const { outputFiles } = await build({ + stdin: { contents, resolveDir: root }, + bundle: true, format: 'cjs', platform: 'browser', write: false, logLevel: 'silent', + external: ['react', 'react-dom', '@deepseek-ai/dsh-client-ui-primitives'], + }) + const mod = { exports: {} } + runInNewContext(outputFiles[0].text, { + module: mod, exports: mod.exports, console, Date, JSON, Promise, Symbol, setTimeout, clearTimeout, + __flybot__: { slots, call: () => {} }, + require: name => (name === 'react' ? react : name === 'react-dom' ? { createPortal: node => node } : {}), + }) + return mod.exports +} + +// Elements stay data; a function component is called so a row's own code runs, and the strings it +// draws are collected in order. +function drawnText(node, seen = []) { + if (node === null || node === undefined) return seen + if (typeof node === 'string') { seen.push(node); return seen } + if (typeof node !== 'object') return seen + if (Array.isArray(node)) { for (const child of node) drawnText(child, seen); return seen } + if (typeof node.type === 'function') { drawnText(node.type(node.props), seen); return seen } + for (const child of node.props?.children ?? []) drawnText(child, seen) + return seen +} + +test('the Bot chat row keeps the words before and after a complete wrap', () => { + assert.equal(readableLine(MIXED), KEPT) + const list = { projectionsBySession: { s1: { values: { turnOutline: [{ turn: 1, response: MIXED }] } } }, byId: {} } + assert.equal(previewOf(list, 's1'), KEPT) + assert.equal(readableLine('a one b two c'), 'a one b two c') +}) + +test('the sidebar row of a room keeps them too', async () => { + const { TeamSidebar } = await clientModule("export { TeamSidebar } from './src/client/sidebar.js'") + const room = { id: 'r1', name: 'Team room', kind: 'group', createdAt: 1, preview: MIXED } + const list = { ids: ['r1'], byId: { r1: { updatedAt: 1 } } } + const useSessions = selector => selector(list) + const useUi = selector => selector({ unread: {} }) + const tree = TeamSidebar({ + wide: true, + useRoster: selector => selector({ bots: [], byId: {}, rooms: [room], mainBotId: 'm', agents: [] }), + useUi, + useSessions, + useRegistry: useUi, + useWork: selector => selector({}), + useSessionStatus: selector => selector(new Map()), + useSessionRetainInfo: (id, selector) => selector({}), + useActivity: selector => selector({}), + useUpdate: selector => selector({}), + actions: {}, + }) + const words = drawnText(tree) + assert.ok(words.includes(KEPT), `the row drew ${JSON.stringify(words.filter(word => /quoted|He said|answered/.test(word)))}`) +}) + +test('the host room preview on the state answer keeps them too', async () => { + const state = { + revision: 3, workspaceId: null, mainBotIds: ['main'], + bots: { main: { id: 'main', name: 'Main' } }, + rooms: { r1: { id: 'r1', name: 'Team room', createdAt: 1, log: [{ text: MIXED, who: 'User', authorKind: 'human' }] } }, + humans: {}, agents: {}, questions: {}, prefs: {}, secretRequests: {}, + } + let route + const rt = { + state, + ctx: { effect: (fn) => { fn(); return () => {} }, connection: { fetch: { register: (r) => { route = r; return () => {} } } } }, + warn: () => {}, noteFailure: () => {}, + readOnly: () => undefined, authenticate: async () => null, authorize: () => ({ ok: true }), + bootstrap: async () => {}, catalog: async () => [], loadSecrets: async () => {}, + cachedCatalog: () => [], cachedDefault: () => null, + chatOf: id => `chat-${id}`, ownedLive: full => full, ownedView: full => full, load: async () => {}, refresh: async () => {}, + pluginToolNames: () => [], publicHumans: () => [], presentRoom: room => room, + secretsView: () => [], capabilityLibraryView: () => ({}), flybotRemoteMembers: () => [], + permissionCatalog: () => null, browserEnabled: false, memoryEnabled: true, authzEnforced: () => false, + listRoomApprovals: () => [], roomBusyAcksView: () => ({}), + activityView: () => [], busyAcksView: () => ({}), workView: () => ({}), roomWorkView: () => ({}), + approvalsView: () => ({}), endedView: () => [], memoryNews: () => [], memoryReviews: () => [], + connectorsView: () => [], capabilityStatusesView: () => [], stateLinkReplaced: () => ({ gen: 'g', count: 0 }), + } + install(rt) + const answer = await route.fetch(new Request('http://host/api/bot', { + method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify({ endpoint: 'state' }), + })) + const body = await answer.json() + assert.equal(body.ok, true, JSON.stringify(body.error)) + assert.equal(body.value.rooms[0].preview, KEPT) +}) + +test('the exchange sheet under the palette already kept them (control, unchanged by this fix)', async () => { + const { exchangeText } = await clientModule("export { exchangeText } from './src/client/exchange.js'") + assert.equal(exchangeText(MIXED), KEPT) + assert.equal(exchangeText('\nhello there\n'), 'hello there') +})