Skip to content

Commit 8455da9

Browse files
committed
fix(chat): apply chip handoffs as one batch; widen table copy chip path
Cursor Bugbot: a multi-context chip handoff dispatched one event per context, and insertContextChip resolved label collisions against selectedContexts read through a ref that only refreshes on render. Each dispatch therefore saw the same stale list, so a second same-label selection was never ordinalized and addContext dropped it while its @token still landed in the text. The event now carries the whole batch and insertContextChips threads each resolved context forward as it goes. Greptile: an explicit multi-row ('some') selection no longer requires every selected row to be loaded before taking the chip-carrying sync path. That gate assumed the paged fall-through would copy more, but its loadRows returns rowsRef.current unchanged for 'some' — the same rows, minus the chip. Renamed the parameter to to say what it actually gates on. Remaining chip-less cases are inherent to the async Clipboard API, which replaces the whole clipboard and cannot hold a custom MIME: a filtered select-all (must page in more rows) and selections past the 500-row chip cap. 'Add to chat' covers both — it is not gesture-bound and drains to the cap.
1 parent a80f2e2 commit 8455da9

7 files changed

Lines changed: 75 additions & 37 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/prompt-editor/use-prompt-editor.ts

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -468,35 +468,44 @@ export function usePromptEditor({
468468
)
469469

470470
/**
471-
* Inserts a context as an `@label` chip at the caret and registers it. Unlike
471+
* Inserts contexts as `@label` chips at the caret and registers them. Unlike
472472
* the menu-driven inserts, this is triggered programmatically (the
473473
* highlight-to-chat action in the file/table viewers) rather than by a typed
474474
* `@`/`/` trigger, so it always inserts at the current cursor position.
475+
*
476+
* Takes the whole batch so label collisions resolve against the chips added
477+
* earlier in the same call: `selectedContexts` is React state read through a
478+
* ref, so it does not reflect an add until the next render.
475479
*/
476-
const insertContextChip = useCallback(
477-
(context: ChatContext) => {
478-
const prepared = prepareContextForInsert(
479-
context,
480-
contextManagementRef.current.selectedContexts
481-
)
482-
if (!prepared) {
480+
const insertContextChips = useCallback(
481+
(contexts: ChatContext[]) => {
482+
let attached = contextManagementRef.current.selectedContexts
483+
const prepared: ChatContext[] = []
484+
for (const context of contexts) {
485+
const next = prepareContextForInsert(context, attached)
486+
if (!next) continue
487+
prepared.push(next)
488+
attached = [...attached, next]
489+
}
490+
if (prepared.length === 0) {
483491
textareaRef.current?.focus()
484492
return
485493
}
494+
486495
const textarea = textareaRef.current
487496
if (textarea) {
488497
const currentValue = valueRef.current
489498
const insertAt = textarea.selectionStart ?? currentValue.length
490499
const needsSpaceBefore = insertAt > 0 && !/\s/.test(currentValue.charAt(insertAt - 1))
491-
const insertText = `${needsSpaceBefore ? ' ' : ''}@${prepared.label} `
500+
const insertText = `${needsSpaceBefore ? ' ' : ''}${prepared.map((c) => `@${c.label} `).join('')}`
492501
const newValue = `${currentValue.slice(0, insertAt)}${insertText}${currentValue.slice(insertAt)}`
493502

494503
pendingCursorRef.current = insertAt + insertText.length
495504
valueRef.current = newValue
496505
setValueState(newValue)
497506
}
498507

499-
addContextNotified(prepared)
508+
for (const context of prepared) addContextNotified(context)
500509
},
501510
[textareaRef, addContextNotified]
502511
)
@@ -1101,8 +1110,8 @@ export function usePromptEditor({
11011110
clear,
11021111
focusAtEnd,
11031112
insertResources,
1104-
/** Inserts a context as an `@label` chip at the caret (highlight-to-chat). */
1105-
insertContextChip,
1113+
/** Inserts contexts as `@label` chips at the caret (highlight-to-chat). */
1114+
insertContextChips,
11061115
insertSlashTrigger,
11071116
openResourceMenu,
11081117
/** The editor's textarea element — focus management, caret restore. */

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,9 @@ const UserInputImpl = forwardRef<UserInputHandle, UserInputProps>(function UserI
185185
useEffect(() => {
186186
const handler = (e: Event) => {
187187
const detail = (e as CustomEvent<MothershipAddContextDetail>).detail
188-
if (!detail?.context) return
188+
if (!detail?.contexts?.length) return
189189
e.preventDefault()
190-
editorRef.current.insertContextChip(detail.context)
190+
editorRef.current.insertContextChips(detail.contexts)
191191
textareaRef.current?.focus()
192192
}
193193
window.addEventListener(MOTHERSHIP_ADD_CONTEXT_EVENT, handler)

apps/sim/app/workspace/[workspaceId]/home/home.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727
MothershipHandoffStorage,
2828
} from '@/lib/core/utils/browser-storage'
2929
import {
30-
addMothershipContext,
30+
addMothershipContexts,
3131
MOTHERSHIP_SEND_MESSAGE_EVENT,
3232
type MothershipSendMessageDetail,
3333
} from '@/lib/mothership/events'
@@ -355,10 +355,9 @@ export function Home({ chatId, userName, userId, tableViewsEnabled }: HomeProps)
355355
sendMessage(handoff.message, undefined, handoff.contexts)
356356
return
357357
}
358-
for (const context of handoff.contexts ?? []) {
359-
handleContextAdd(context)
360-
addMothershipContext(context)
361-
}
358+
const contexts = handoff.contexts ?? []
359+
for (const context of contexts) handleContextAdd(context)
360+
addMothershipContexts(contexts)
362361
// eslint-disable-next-line react-hooks/exhaustive-deps -- one-shot drain; handleContextAdd is a stable body function
363362
}, [chatId, workspaceId, sendMessage])
364363

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -364,21 +364,24 @@ function buildTableSelectionContext(opts: {
364364
* cannot do this: its async Clipboard API write replaces the whole clipboard and
365365
* so cannot carry a custom MIME type.
366366
*
367-
* Taking this path requires a chip to carry AND every selected row already
368-
* loaded and within the chip cap; otherwise the canonical paged path handles the
367+
* Taking this path requires a chip to carry, `rows` being the complete copy, and
368+
* a set within the chip cap; otherwise the canonical paged path handles the
369369
* copy, including its row loading and truncation notice.
370370
*
371+
* @param complete - Whether `rows` is everything this copy should contain. False
372+
* when the paged path would load rows this caller cannot see yet, so deferring
373+
* to it copies strictly more.
371374
* @returns True when it handled the copy, false to fall through to the paged path.
372375
*/
373376
function writeLoadedRowsWithChip(opts: {
374377
clipboardData: DataTransfer | null
375378
rows: TableRowType[]
376-
allLoaded: boolean
379+
complete: boolean
377380
buildCells: (row: TableRowType) => string[]
378381
context: ChatContext | null
379382
}): boolean {
380383
const { rows, context } = opts
381-
if (!context || !opts.allLoaded || rows.length === 0 || rows.length > MAX_TABLE_SELECTION_ROWS) {
384+
if (!context || !opts.complete || rows.length === 0 || rows.length > MAX_TABLE_SELECTION_ROWS) {
382385
return false
383386
}
384387
opts.clipboardData?.setData(
@@ -3012,14 +3015,17 @@ export function TableGrid({
30123015

30133016
if (!rowSelectionIsEmpty(rowSel)) {
30143017
e.preventDefault()
3015-
// A filtered select-all ('all') covers rows beyond the loaded page, so
3016-
// only an explicit multi-row selection can take the chip-carrying path.
3018+
// Only an explicit multi-row selection can carry the chip: a filtered
3019+
// select-all ('all') pages in rows beyond those loaded, which the async
3020+
// path must fetch. For 'some' the fall-through re-reads the same loaded
3021+
// rows (see its `loadRows`), so it never copies more than this does —
3022+
// the selection is complete here even when some ids aren't loaded yet.
30173023
if (rowSel.kind === 'some') {
30183024
const selectedRows = currentRows.filter((row) => rowSelectionIncludes(rowSel, row.id))
30193025
const handled = writeLoadedRowsWithChip({
30203026
clipboardData: e.clipboardData,
30213027
rows: selectedRows,
3022-
allLoaded: selectedRows.length === rowSel.ids.size,
3028+
complete: true,
30233029
buildCells: (row) => cols.map((col) => cellToText(row.data[col.key], col)),
30243030
context: buildTableSelectionContext({
30253031
tableId,
@@ -3059,12 +3065,12 @@ export function TableGrid({
30593065
}
30603066
const colByKey = new Map(cols.map((c) => [c.key, c]))
30613067

3062-
// A column-header selection spans every row, so the chip-carrying path
3063-
// only applies once the whole table is loaded.
3068+
// A column-header selection spans every row, and its fall-through pages
3069+
// in the rest — so the chip path applies only once all of them are here.
30643070
const handled = writeLoadedRowsWithChip({
30653071
clipboardData: e.clipboardData,
30663072
rows: currentRows,
3067-
allLoaded: currentRows.length >= selectAllTotalRef.current,
3073+
complete: currentRows.length >= selectAllTotalRef.current,
30683074
buildCells: (row) =>
30693075
colNames.map((name) => cellToText(row.data[name], colByKey.get(name))),
30703076
context: buildTableSelectionContext({

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/user-input/utils.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,22 @@ describe('prepareContextForInsert', () => {
7878

7979
expect(prepareContextForInsert(context, [])).toEqual(context)
8080
})
81+
82+
it('ordinalizes within a batch when the caller threads each result forward', () => {
83+
// How insertContextChips applies a multi-context handoff: `selectedContexts`
84+
// is React state read through a ref and does not reflect an add until the
85+
// next render, so the batch must accumulate locally or the second colliding
86+
// chip is silently dropped.
87+
const batch = [tableSelection(), tableSelection({ rowIds: ['r7', 'r8', 'r9'] })]
88+
let attached: ChatContext[] = []
89+
const prepared: ChatContext[] = []
90+
for (const context of batch) {
91+
const next = prepareContextForInsert(context, attached)
92+
if (!next) continue
93+
prepared.push(next)
94+
attached = [...attached, next]
95+
}
96+
97+
expect(prepared.map((c) => c.label)).toEqual(['Sales (3 rows)', 'Sales (3 rows) (2)'])
98+
})
8199
})

apps/sim/hooks/use-add-to-chat.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import { useCallback } from 'react'
44
import { useParams, useRouter } from 'next/navigation'
55
import { MothershipHandoffStorage } from '@/lib/core/utils/browser-storage'
6-
import { addMothershipContext } from '@/lib/mothership/events'
6+
import { addMothershipContexts } from '@/lib/mothership/events'
77
import type { ChatContext } from '@/stores/panel'
88

99
/**
@@ -23,7 +23,7 @@ export function useAddToChat(): (context: ChatContext) => void {
2323

2424
return useCallback(
2525
(context: ChatContext) => {
26-
if (addMothershipContext(context)) return
26+
if (addMothershipContexts([context])) return
2727
if (!workspaceId) return
2828
if (MothershipHandoffStorage.store({ contexts: [context] }, workspaceId)) {
2929
router.push(`/workspace/${workspaceId}/home`)

apps/sim/lib/mothership/events.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,22 +61,28 @@ export function sendMothershipMessage(message: string, contexts?: ChatContext[])
6161
export const MOTHERSHIP_ADD_CONTEXT_EVENT = 'mothership-add-context'
6262

6363
export interface MothershipAddContextDetail {
64-
/** The context to attach as a chip in the input. */
65-
context: ChatContext
64+
/** The contexts to attach as chips, in insertion order. */
65+
contexts: ChatContext[]
6666
}
6767

6868
/**
69-
* Dispatches a passive "add this context chip" request to a mounted Mothership
69+
* Dispatches a passive "add these context chips" request to a mounted Mothership
7070
* chat input — the highlight-to-chat action in the file and table viewers.
7171
*
72+
* Carries the whole batch in one event rather than one event per context: the
73+
* input resolves label collisions against its current chips, and that list only
74+
* refreshes on re-render, so consecutive synchronous dispatches would each see
75+
* the same stale list and drop colliding chips.
76+
*
7277
* @returns `true` when a mounted input consumed it, `false` when none was
7378
* listening — callers fall back to persisting a chip-only
7479
* {@link MothershipHandoff} for the next chat mount.
7580
*/
76-
export function addMothershipContext(context: ChatContext): boolean {
81+
export function addMothershipContexts(contexts: ChatContext[]): boolean {
82+
if (contexts.length === 0) return false
7783
const consumed = dispatchClaimable<MothershipAddContextDetail>(MOTHERSHIP_ADD_CONTEXT_EVENT, {
78-
context,
84+
contexts,
7985
})
80-
logger.info('Dispatched mothership add-context event', { kind: context.kind, consumed })
86+
logger.info('Dispatched mothership add-context event', { count: contexts.length, consumed })
8187
return consumed
8288
}

0 commit comments

Comments
 (0)