Skip to content

Commit 68f740d

Browse files
authored
fix(chat): preserve queued edits and render quoted source chips (#7452)
1 parent 67e3f6d commit 68f740d

10 files changed

Lines changed: 423 additions & 119 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it } from 'vitest'
5+
import { sanitizeChatDisplayContent } from '@/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-sanitize'
56
import { scalingRatioOver4x } from '@/app/workspace/[workspaceId]/home/components/message-content/components/scaling-test-helpers'
6-
import { sanitizeChatDisplayContent } from './chat-sanitize'
77

88
describe('sanitizeChatDisplayContent', () => {
99
it('unwraps workspace resource tags from inline code spans', () => {
@@ -23,6 +23,35 @@ describe('sanitizeChatDisplayContent', () => {
2323
)
2424
})
2525

26+
it.each(['source', 'workspace_resource'])('preserves backticks inside %s JSON strings', (tag) => {
27+
const payload = JSON.stringify({
28+
title: 'Run `bun test`',
29+
snippet: 'Quoted "commands" and a \\path with `backticks`',
30+
})
31+
const chip = `<${tag}>${payload}</${tag}>`
32+
33+
expect(sanitizeChatDisplayContent(`\`Evidence ${chip}.\``)).toBe(`Evidence ${chip}.`)
34+
expect(sanitizeChatDisplayContent(`\`${chip} done`)).toBe(`${chip} done`)
35+
expect(sanitizeChatDisplayContent(`${chip}\` done`)).toBe(`${chip} done`)
36+
expect(sanitizeChatDisplayContent(`\`before\`${chip}\`after\``)).toBe(
37+
`\`before\`${chip}\`after\``
38+
)
39+
})
40+
41+
it('treats tag markers inside JSON strings as payload', () => {
42+
const payload = JSON.stringify({ snippet: 'Use `<source>` and `</source>` markers' })
43+
const chip = `<source>${payload}</source>`
44+
45+
expect(sanitizeChatDisplayContent(`\`See ${chip}\``)).toBe(`See ${chip}`)
46+
})
47+
48+
it('leaves a fenced source example with payload backticks intact', () => {
49+
const payload = JSON.stringify({ snippet: 'Run `bun test`' })
50+
const content = `Example:\n\`\`\`json\n<source>${payload}</source>\n\`\`\`\nDone.`
51+
52+
expect(sanitizeChatDisplayContent(content)).toBe(content)
53+
})
54+
2655
it('removes hidden internal references wrapped in inline code', () => {
2756
const content = 'Read `internal/tool-results/read-1.md` and found the issue.'
2857

@@ -105,6 +134,16 @@ describe('sanitizeChatDisplayContent', () => {
105134
expect(scalingRatioOver4x((content) => sanitizeChatDisplayContent(content))).toBeLessThan(8)
106135
})
107136

137+
it('stays linear on repeated unclosed JSON chip bodies', () => {
138+
expect(
139+
scalingRatioOver4x((content) =>
140+
sanitizeChatDisplayContent(
141+
content.replaceAll('The <workspace_resource> tag is used here. ', '<source>{"snippet":"')
142+
)
143+
)
144+
).toBeLessThan(8)
145+
})
146+
108147
it('still unwraps a real tag that carries a stray backtick on one side only', () => {
109148
// The case the unpaired strip is actually for: the model backticked the
110149
// opener but not the closer (or vice versa), which would block the chip.
Lines changed: 54 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,66 @@
11
const HIDDEN_INLINE_REFERENCE_PATTERN =
22
/`[^`\n]*(?:internal\/tool-results\/|internal\/blocktips\/|components\/integrations\/[^`\n]*README)[^`\n]*`/g
33

4+
/** JSON strings own their escaped quotes, backticks, and any quoted tag markers. */
5+
const JSON_STRING_SOURCE = String.raw`"(?:[^"\\\r\n]|\\[^\r\n])*"`
6+
47
/**
5-
* A complete inline-chip tag — `<workspace_resource>` or `<source>` — as
6-
* opener, payload, closer. Both are JSON-bodied tags the model places inside a
7-
* sentence, so both attract the same stray backticks.
8-
*
9-
* Two constraints on the payload, both load-bearing:
10-
*
11-
* - **No backtick.** A payload is JSON and carries none, so this is what tells a
12-
* real tag from prose MENTIONING the tag name — a message explaining the
13-
* syntax writes the opener and the closer as two separately backticked spans.
14-
* - **No nested opener**, via the negative lookahead. A cost bound rather than a
15-
* correctness rule: a lazy scan allowed to cross an opener restarts from every
16-
* opener, so a message repeating the tag name is quadratic — on the main
17-
* thread, for every streamed chunk.
18-
*
19-
* Accepted trade: a resource whose title or path itself contains a backtick is
20-
* not matched, so it renders as text rather than a chip. That costs one chip and
21-
* is rare; the failure it replaces corrupts a whole message and is common.
8+
* Complete chip tags consume JSON strings atomically. Outside strings, a new
9+
* opener or backtick ends the candidate, so prose mentions cannot join into a
10+
* tag and repeated unclosed openers cannot repeatedly scan the same suffix.
2211
*/
23-
const COMPLETE_TAG_SOURCE =
24-
'<(?<chipTag>workspace_resource|source)>(?:(?!<\\k<chipTag>>)[^`])*?<\\/\\k<chipTag>>'
12+
const COMPLETE_TAG_SOURCE = `<(?<chipTag>workspace_resource|source)>\\s*\\{(?:${JSON_STRING_SOURCE}|[^"\`<])*?\\}\\s*</\\k<chipTag>>`
2513

26-
/** Non-global so {@link RegExp.test} has no `lastIndex` to carry between calls. */
27-
const COMPLETE_INLINE_CHIP_TAG = new RegExp(COMPLETE_TAG_SOURCE)
14+
const CHIP_OR_CODE_DELIMITER = new RegExp(`${COMPLETE_TAG_SOURCE}|\`|\n`, 'g')
2815

2916
/**
30-
* One left-to-right pass over the two things that can own a backtick: an inline
31-
* code span, and a tag with a stray backtick pressed against it.
32-
*
33-
* ONE pass is the design. Two separate passes each have to guess which backticks
34-
* belong together, and every previous arrangement of this file got a different
35-
* case wrong — a span two words away, a code fence, then a span sitting flush
36-
* against the tag. Here a span consumes its own delimiters as the scan reaches
37-
* them, so `` `config.json`<tag> `` keeps its pair without a special case.
38-
*
39-
* The trailing backtick is only taken when no further backtick follows on the
40-
* line; otherwise it is not a stray at all but the opener of the next span, and
41-
* `` <tag>`config.json` `` would lose that span's delimiter. A LEADING backtick
42-
* needs no such guard, because a backtick that closes a span is consumed as part
43-
* of that span. Of the two, only the trailing lookahead is pinned by a test —
44-
* swapping the alternatives changes behaviour only for a span that both opens
45-
* flush against a tag and closes elsewhere, which no fixture covers.
17+
* Pair Markdown delimiters outside chip payloads in one forward pass. A pair
18+
* containing a chip is unwrapped; a lone delimiter is removed only when flush
19+
* against a chip. Neighbouring code spans and multiline fences keep their pairs.
4620
*/
47-
const CODE_SPAN_OR_FLANKED_TAG = new RegExp(
48-
`\`[^\`\\n]*\`|\`?(${COMPLETE_TAG_SOURCE})(?:\`(?![^\`\\n]*\`))?`,
49-
'g'
50-
)
51-
5221
export function sanitizeChatDisplayContent(content: string): string {
53-
return content
54-
.replace(CODE_SPAN_OR_FLANKED_TAG, (match, tag?: string) => {
55-
// A tag with stray backticks against it: keep the tag, drop the strays.
56-
if (tag !== undefined) return tag
22+
const removedDelimiters: number[] = []
23+
let openingTick = -1
24+
let containsChip = false
25+
let adjacentToChip = false
26+
let lastChipEnd = -1
27+
28+
for (const match of content.matchAll(CHIP_OR_CODE_DELIMITER)) {
29+
const index = match.index
30+
if (match.groups?.chipTag) {
31+
if (openingTick !== -1) {
32+
containsChip = true
33+
adjacentToChip ||= index === openingTick + 1
34+
}
35+
lastChipEnd = index + match[0].length
36+
continue
37+
}
38+
39+
if (match[0] === '\n') {
40+
if (openingTick !== -1 && adjacentToChip) removedDelimiters.push(openingTick)
41+
openingTick = -1
42+
lastChipEnd = -1
43+
continue
44+
}
45+
46+
if (openingTick === -1) {
47+
openingTick = index
48+
containsChip = false
49+
adjacentToChip = lastChipEnd === index
50+
} else {
51+
if (containsChip) removedDelimiters.push(openingTick, index)
52+
openingTick = -1
53+
}
54+
}
55+
56+
if (openingTick !== -1 && adjacentToChip) removedDelimiters.push(openingTick)
5757

58-
// A code span. Unwrap it only when it genuinely holds a tag — the parser
59-
// lifts the tag out either way, so leaving the delimiters would strand a
60-
// pair of backticks around a hole. Anything else is someone else's span.
61-
const inner = match.slice(1, -1)
62-
return COMPLETE_INLINE_CHIP_TAG.test(inner) ? inner : match
63-
})
64-
.replace(HIDDEN_INLINE_REFERENCE_PATTERN, '')
58+
const parts: string[] = []
59+
let start = 0
60+
for (const index of removedDelimiters) {
61+
parts.push(content.slice(start, index))
62+
start = index + 1
63+
}
64+
parts.push(content.slice(start))
65+
return parts.join('').replace(HIDDEN_INLINE_REFERENCE_PATTERN, '')
6566
}

apps/sim/app/workspace/[workspaceId]/home/components/suggested-actions/suggested-actions.test.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
77

88
const { mockCaptureEvent, modeState } = vi.hoisted(() => ({
99
mockCaptureEvent: vi.fn(),
10-
/** The URL `mode` param as the nuqs mock serves it; `set` is the live setter once mounted. */
1110
modeState: { initial: 'build', set: (_next: string) => {} },
1211
}))
1312

14-
vi.mock('nuqs', async () => {
13+
vi.mock('@/app/workspace/[workspaceId]/home/hooks/use-mothership-mode', async () => {
1514
const { useState } = await import('react')
1615
return {
17-
useQueryState: () => {
16+
useMothershipMode: () => {
1817
const [mode, setMode] = useState(modeState.initial)
1918
modeState.set = setMode
2019
return [mode, setMode]

apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/mode-switcher/mode-switcher.test.tsx

Lines changed: 41 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2,34 +2,19 @@
22
* @vitest-environment jsdom
33
*/
44
import { act } from 'react'
5+
import { NuqsTestingAdapter, type UrlUpdateEvent } from 'nuqs/adapters/testing'
56
import { createRoot, type Root } from 'react-dom/client'
67
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
78

8-
const { mockCaptureEvent, mockSetSearchQuery, mockSetSearchFilters, modeState } = vi.hoisted(
9-
() => ({
10-
mockCaptureEvent: vi.fn(),
11-
mockSetSearchQuery: vi.fn(),
12-
mockSetSearchFilters: vi.fn(),
13-
/** The URL `mode` param as the nuqs mock serves it; `set` is the live setter once mounted. */
14-
modeState: { initial: 'build', set: (_next: string) => {} },
15-
})
16-
)
9+
const { mockCaptureEvent, mockLeaveSearch } = vi.hoisted(() => ({
10+
mockCaptureEvent: vi.fn(),
11+
mockLeaveSearch: vi.fn(),
12+
}))
13+
const mockUrlUpdate = vi.fn<(event: UrlUpdateEvent) => void>()
1714

1815
vi.mock('next/navigation', () => ({
1916
useParams: () => ({ workspaceId: 'workspace-1' }),
2017
}))
21-
vi.mock('nuqs', async () => {
22-
const { useState } = await import('react')
23-
return {
24-
useQueryState: (key: string) => {
25-
const [mode, setMode] = useState(modeState.initial)
26-
if (key !== 'mode') return [null, mockSetSearchQuery]
27-
modeState.set = setMode
28-
return [mode, setMode]
29-
},
30-
useQueryStates: () => [{}, mockSetSearchFilters],
31-
}
32-
})
3318
vi.mock('posthog-js/react', () => ({ usePostHog: () => null }))
3419
vi.mock('@/lib/posthog/client', () => ({ captureEvent: mockCaptureEvent }))
3520

@@ -38,12 +23,18 @@ import { ModeSwitcher } from '@/app/workspace/[workspaceId]/home/components/user
3823
let root: Root | null = null
3924
let container: HTMLDivElement | null = null
4025

41-
function mount() {
26+
function mount(searchParams = '') {
4227
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
4328
container = document.createElement('div')
4429
document.body.appendChild(container)
4530
root = createRoot(container)
46-
act(() => root?.render(<ModeSwitcher />))
31+
act(() =>
32+
root?.render(
33+
<NuqsTestingAdapter hasMemory searchParams={searchParams} onUrlUpdate={mockUrlUpdate}>
34+
<ModeSwitcher onLeaveSearch={mockLeaveSearch} />
35+
</NuqsTestingAdapter>
36+
)
37+
)
4738
}
4839

4940
function trigger(): HTMLButtonElement {
@@ -63,24 +54,26 @@ function items(): HTMLElement[] {
6354
return Array.from(document.querySelectorAll<HTMLElement>('[role="menuitem"]'))
6455
}
6556

66-
function select(index: number) {
67-
act(() => {
57+
async function select(index: number) {
58+
await act(async () => {
6859
items()[index].dispatchEvent(new MouseEvent('click', { bubbles: true, button: 0 }))
60+
await vi.advanceTimersByTimeAsync(1)
6961
})
7062
}
7163

7264
beforeEach(() => {
65+
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] })
7366
mockCaptureEvent.mockClear()
74-
mockSetSearchQuery.mockClear()
75-
mockSetSearchFilters.mockClear()
76-
modeState.initial = 'build'
67+
mockLeaveSearch.mockClear()
68+
mockUrlUpdate.mockClear()
7769
})
7870

7971
afterEach(() => {
8072
if (root) act(() => root?.unmount())
8173
container?.remove()
8274
root = null
8375
container = null
76+
vi.useRealTimers()
8477
})
8578

8679
describe('ModeSwitcher', () => {
@@ -108,47 +101,52 @@ describe('ModeSwitcher', () => {
108101
expect(rows[2].querySelector('svg')).toBeNull()
109102
})
110103

111-
it('writes the chosen mode to the URL and reports the change', () => {
104+
it('writes the chosen mode to the URL and reports the change', async () => {
112105
mount()
113106
openMenu()
114-
select(1)
107+
await select(1)
115108

116109
expect(trigger().textContent).toBe('Search')
117110
expect(mockCaptureEvent).toHaveBeenCalledWith(null, 'chat_mode_changed', {
118111
workspace_id: 'workspace-1',
119112
mode: 'search',
120113
})
121-
expect(mockSetSearchQuery).not.toHaveBeenCalled()
114+
expect(mockUrlUpdate.mock.lastCall?.[0].searchParams.get('mode')).toBe('search')
115+
expect(mockLeaveSearch).not.toHaveBeenCalled()
122116
})
123117

124118
it('reads the mode from the URL on mount', () => {
125-
modeState.initial = 'assistant'
126-
mount()
119+
mount('?mode=assistant')
127120

128121
expect(trigger().textContent).toBe('Assistant')
129122
expect(trigger().getAttribute('aria-label')).toBe('Mode: Assistant')
130123
})
131124

132-
it('drops the search query from the URL when leaving Search', () => {
133-
modeState.initial = 'search'
134-
mount()
125+
it('clears the composer and search parameters together when leaving Search', async () => {
126+
mount('?mode=search&q=budget&source=upload&updated=7d&resource=report')
135127
openMenu()
136-
select(0)
128+
await select(0)
137129

138130
expect(trigger().textContent).toBe('Build')
139-
expect(mockSetSearchQuery).toHaveBeenCalledWith(null, { history: 'replace', scroll: false })
140-
expect(mockSetSearchFilters).toHaveBeenCalledWith(
141-
{ source: null, updated: null },
142-
{ history: 'replace', scroll: false }
131+
expect(mockLeaveSearch).toHaveBeenCalledOnce()
132+
expect(mockUrlUpdate).toHaveBeenCalledOnce()
133+
expect(mockUrlUpdate.mock.lastCall?.[0].searchParams.toString()).toBe('resource=report')
134+
expect(mockUrlUpdate.mock.lastCall?.[0].options).toMatchObject({
135+
history: 'replace',
136+
scroll: false,
137+
})
138+
expect(mockLeaveSearch.mock.invocationCallOrder[0]).toBeLessThan(
139+
mockUrlUpdate.mock.invocationCallOrder[0]
143140
)
144141
})
145142

146-
it('does not report re-selecting the active mode', () => {
143+
it('does not report re-selecting the active mode', async () => {
147144
mount()
148145
openMenu()
149-
select(0)
146+
await select(0)
150147

151148
expect(trigger().textContent).toBe('Build')
152149
expect(mockCaptureEvent).not.toHaveBeenCalled()
150+
expect(mockLeaveSearch).not.toHaveBeenCalled()
153151
})
154152
})

0 commit comments

Comments
 (0)