Skip to content

Commit 6421994

Browse files
committed
refactor(chat): clean up highlight-to-chat selections
Correctness: - Stop reporting fabricated line numbers for rich-markdown selections. `doc.textBetween` counts ProseMirror block boundaries, not markdown source lines, so the chip label and the agent prompt both claimed line ranges that don't exist in the file. Line info is now emitted only by Monaco. - Bound a table_selection's rendered markdown by characters, not just row and column counts — 500 wide rows dwarfed the 20k-char file-selection budget. Rows are emitted until the budget is spent, and the content says what was omitted. - Complete `areContextsEqual` for both selection kinds, so re-adding the same selection dedupes while a different passage of an already-referenced file registers as new. Consistency: - Carry the resource display name on the context instead of recovering it by regex from the chip label, deleting fileNameFromSelectionLabel and tableNameFromSelectionLabel. - Replace the user-visible `#k3f9` hash disambiguator with a readable ordinal applied at insert time (`Sales (3 rows) (2)`), via a shared prepareContextForInsert used by both the add-to-chat and paste paths. - Fold MothershipPendingContextStorage into MothershipHandoffStorage as a chip-only handoff (optional message), removing the parallel storage class, the second drain effect, and its StrictMode guard ref. - Replace `window.location.assign` with `router.push`, matching the existing "Troubleshoot in Chat" handoff. - Collapse three near-duplicate synchronous copy branches in table-grid into shared buildTableSelectionContext / writeLoadedRowsWithChip helpers, also reused by the add-to-chat handler. - Trim multi-paragraph inline comments to TSDoc stating each reason once. Tests: new coverage for the character budget, the label ordinal, selection equality, and chip-only handoff accumulation; each verified to fail when its fix is reverted.
1 parent 27dbd17 commit 6421994

24 files changed

Lines changed: 663 additions & 447 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { useRouter } from 'next/navigation'
1414
import { useSession } from '@/lib/auth/auth-client'
1515
import {
1616
buildFileSelectionLabel,
17-
selectionKey,
1817
truncateSelectionText,
1918
} from '@/lib/copilot/chat/selection-context'
2019
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
@@ -1133,23 +1132,25 @@ export function LoadedRichMarkdownEditor({
11331132
)
11341133

11351134
const addToChat = useAddToChat()
1135+
/**
1136+
* No line range: this editor renders a ProseMirror document, whose block
1137+
* boundaries do not correspond to markdown source lines (blank lines between
1138+
* paragraphs, list markers, heading prefixes and fenced blocks all shift the
1139+
* real line). Reporting a derived count would label the chip — and prompt the
1140+
* agent — with line numbers that don't exist in the file.
1141+
*/
11361142
const buildSelectionContext = useCallback((): ChatContext | null => {
11371143
if (!editor) return null
11381144
const { from, to } = editor.state.selection
11391145
if (from === to) return null
11401146
const text = editor.state.doc.textBetween(from, to, '\n')
11411147
if (!text.trim()) return null
1142-
// Markdown has no native line numbers; approximate from newlines before the
1143-
// selection so repeated selections in the same file get distinct labels.
1144-
const startLine = editor.state.doc.textBetween(0, from, '\n').split('\n').length
1145-
const endLine = startLine + text.split('\n').length - 1
11461148
return {
11471149
kind: 'file_selection',
11481150
fileId: file.id,
1149-
label: buildFileSelectionLabel(file.name, startLine, endLine, selectionKey([text])),
1151+
fileName: file.name,
1152+
label: buildFileSelectionLabel(file.name),
11501153
text: truncateSelectionText(text),
1151-
startLine,
1152-
endLine,
11531154
}
11541155
}, [editor, file.id, file.name])
11551156

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/text-editor.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import type { editor as MonacoEditorTypes } from 'monaco-editor'
77
import dynamic from 'next/dynamic'
88
import {
99
buildFileSelectionLabel,
10-
selectionKey,
1110
truncateSelectionText,
1211
} from '@/lib/copilot/chat/selection-context'
1312
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
@@ -395,7 +394,8 @@ export const TextEditor = memo(function TextEditor({
395394
return {
396395
kind: 'file_selection',
397396
fileId: file.id,
398-
label: buildFileSelectionLabel(file.name, startLine, endLine, selectionKey([text])),
397+
fileName: file.name,
398+
label: buildFileSelectionLabel(file.name, startLine, endLine),
399399
text: truncateSelectionText(text),
400400
startLine,
401401
endLine,

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/use-selection-copy-bridge.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,14 @@ import type { ChatContext } from '@/stores/panel'
88
* Rides a selection {@link ChatContext} onto the editor's native copy so a
99
* highlighted passage copied with Cmd+C pastes into Chat as a reference chip.
1010
*
11-
* The listener is attached to `containerRef` in the BUBBLE phase so it runs
12-
* AFTER the inner editor's own copy handler (Monaco and ProseMirror both call
13-
* `clearData()` then write `text/plain`/`text/html`) — the custom
14-
* `text/x-sim-selection` type is added last and survives, leaving normal copy
15-
* untouched. `buildContext` returns `null` when there is no non-empty selection.
11+
* Attached in the BUBBLE phase so it runs after the inner editor's own copy
12+
* handler — Monaco and ProseMirror both `clearData()` before writing
13+
* `text/plain`, so the custom type must be added last to survive.
1614
*
17-
* `enabled` lets a caller whose container mounts late (e.g. behind a loading
18-
* gate) re-run the effect once the node exists — a ref object isn't reactive, so
19-
* without it the effect would bail on the first render and never re-attach.
15+
* @param buildContext - Returns null when there is no non-empty selection.
16+
* @param enabled - Re-runs the effect for a container that mounts late (behind a
17+
* loading gate); a ref isn't reactive, so the effect would otherwise bail on the
18+
* first render and never re-attach.
2019
*/
2120
export function useSelectionCopyBridge(
2221
containerRef: RefObject<HTMLElement | null>,

apps/sim/app/workspace/[workspaceId]/home/components/chat-context-kind-registry/chat-context-kind-registry.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
} from '@sim/emcn/icons'
1313
import { AgentSkillsIcon, McpIcon } from '@/components/icons'
1414
import { getDocumentIcon } from '@/components/icons/document-icons'
15-
import { fileNameFromSelectionLabel } from '@/lib/copilot/chat/selection-context'
1615
import type { ChatContextKind, ChatMessageContext } from '@/app/workspace/[workspaceId]/home/types'
1716
import { getBareIconStyle } from '@/blocks/brand-icon-style'
1817
import { getBlockRegistry } from '@/blocks/registry'
@@ -90,9 +89,9 @@ export const CHAT_CONTEXT_KIND_REGISTRY: Record<ChatContextKind, ChatContextKind
9089
file_selection: {
9190
label: 'File selection',
9291
renderIcon: ({ context, className }) => {
93-
// Strip the `:line` suffix so `getDocumentIcon` reads the real extension
94-
// (e.g. `md`, not `md:12-40`) and shows the correct file glyph.
95-
const FileDocIcon = getDocumentIcon('', fileNameFromSelectionLabel(context.label))
92+
// The label carries a `:line` suffix, so read the extension off the file
93+
// name the context carries — `getDocumentIcon` needs `md`, not `md:12-40`.
94+
const FileDocIcon = getDocumentIcon('', context.fileName ?? context.label)
9695
return <FileDocIcon className={className} />
9796
},
9897
},

apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/chip-clipboard-codec.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,11 @@ export function serializeSelectionForClipboard(
162162

163163
/**
164164
* Finds the selection-scoped chips (`file_selection` / `table_selection`) whose
165-
* highlighted token falls inside `selectedText`. These kinds carry an inline
166-
* text blob / row-id array that can't fit a portable `sim:kind/id` link, so the
167-
* chat input's copy/cut path round-trips them through the custom
168-
* `text/x-sim-selection` clipboard MIME instead. Uses the overlay's exact
169-
* tokenization so a label that is a substring of another never false-matches.
165+
* highlighted token falls inside `selectedText` — the chips the copy/cut path
166+
* must route through the custom clipboard MIME rather than a portable link.
167+
*
168+
* Uses the overlay's exact tokenization so a label that is a substring of
169+
* another never false-matches.
170170
*/
171171
export function selectionContextsInText(
172172
selectedText: string,

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

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
useMentionTokens,
2626
} from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/user-input/hooks'
2727
import {
28-
isContextAlreadySelected,
28+
prepareContextForInsert,
2929
restoreSkillTriggerText,
3030
SKILL_CHIP_TRIGGER,
3131
} from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/user-input/utils'
@@ -475,10 +475,11 @@ export function usePromptEditor({
475475
*/
476476
const insertContextChip = useCallback(
477477
(context: ChatContext) => {
478-
// A chip's `@label` token must be unique — `addContext` dedupes a matching
479-
// label, so inserting a token for an already-present context would orphan
480-
// it (a second token with no backing context). Skip and just focus.
481-
if (isContextAlreadySelected(context, contextManagementRef.current.selectedContexts)) {
478+
const prepared = prepareContextForInsert(
479+
context,
480+
contextManagementRef.current.selectedContexts
481+
)
482+
if (!prepared) {
482483
textareaRef.current?.focus()
483484
return
484485
}
@@ -487,16 +488,15 @@ export function usePromptEditor({
487488
const currentValue = valueRef.current
488489
const insertAt = textarea.selectionStart ?? currentValue.length
489490
const needsSpaceBefore = insertAt > 0 && !/\s/.test(currentValue.charAt(insertAt - 1))
490-
const insertText = `${needsSpaceBefore ? ' ' : ''}@${context.label} `
491+
const insertText = `${needsSpaceBefore ? ' ' : ''}@${prepared.label} `
491492
const newValue = `${currentValue.slice(0, insertAt)}${insertText}${currentValue.slice(insertAt)}`
492-
const newPos = insertAt + insertText.length
493493

494-
pendingCursorRef.current = newPos
494+
pendingCursorRef.current = insertAt + insertText.length
495495
valueRef.current = newValue
496496
setValueState(newValue)
497497
}
498498

499-
addContextNotified(context)
499+
addContextNotified(prepared)
500500
},
501501
[textareaRef, addContextNotified]
502502
)
@@ -923,25 +923,20 @@ export function usePromptEditor({
923923
const selectionContext = readSelectionContextFromClipboard(e.clipboardData)
924924
if (selectionContext) {
925925
e.preventDefault()
926-
// A chip's `@label` token must be unique — `addContext` dedupes a matching
927-
// label, so inserting a token for an already-present selection would orphan
928-
// it (a second token with no backing context). Skip and keep focus, mirroring
929-
// insertContextChip.
930-
if (
931-
isContextAlreadySelected(selectionContext, contextManagementRef.current.selectedContexts)
932-
) {
933-
return
934-
}
926+
const prepared = prepareContextForInsert(
927+
selectionContext,
928+
contextManagementRef.current.selectedContexts
929+
)
930+
if (!prepared) return
935931
const selStart = textarea.selectionStart ?? valueRef.current.length
936932
const selEnd = textarea.selectionEnd ?? selStart
937933
const needsSpaceBefore = selStart > 0 && !/\s/.test(valueRef.current.charAt(selStart - 1))
938-
const insert = `${needsSpaceBefore ? ' ' : ''}@${selectionContext.label} `
934+
const insert = `${needsSpaceBefore ? ' ' : ''}@${prepared.label} `
939935
textarea.setRangeText(insert, selStart, selEnd, 'end')
940-
const newValue = textarea.value
941936
const caret = selStart + insert.length
942-
contextManagementRef.current.addContext(selectionContext)
943-
valueRef.current = newValue
944-
setValueState(newValue)
937+
contextManagementRef.current.addContext(prepared)
938+
valueRef.current = textarea.value
939+
setValueState(textarea.value)
945940
requestAnimationFrame(() => textarea.setSelectionRange(caret, caret))
946941
return
947942
}
@@ -1038,14 +1033,10 @@ export function usePromptEditor({
10381033
* (the caller must then perform the cut deletion itself, since the default
10391034
* was prevented).
10401035
*
1041-
* Selection chips (`file_selection` / `table_selection`) can't fit a portable
1042-
* link — their inline text / row-id payload lives only in the context. When
1043-
* the selection is exactly one such chip (the common copy/cut of a
1044-
* highlight-to-chat chip), ride its full context on the custom
1045-
* `text/x-sim-selection` MIME so paste restores it; otherwise it would leave a
1046-
* bare `@label` with no backing data. Mixed selections keep the portable/plain
1047-
* path (the single-slot MIME can't carry more than one), so the chip degrades
1048-
* to its label text there rather than dropping the rest of the selection.
1036+
* Selection chips carry an inline text / row-id payload that no portable link
1037+
* can hold, so a lone selection chip rides the custom `text/x-sim-selection`
1038+
* MIME instead. That slot fits only one, so a mixed selection keeps the
1039+
* portable path and its selection chip degrades to bare label text.
10491040
*/
10501041
const writeSanitizedClipboard = useCallback(
10511042
(e: React.ClipboardEvent<HTMLTextAreaElement>): boolean => {

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

Lines changed: 26 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,12 @@ import { useQueryState } from 'nuqs'
1919
import { usePostHog } from 'posthog-js/react'
2020
import { requestJson } from '@/lib/api/client/request'
2121
import { createWorkflowContract } from '@/lib/api/contracts'
22-
import {
23-
fileNameFromSelectionLabel,
24-
tableNameFromSelectionLabel,
25-
} from '@/lib/copilot/chat/selection-context'
2622
import { canonicalWorkspaceFilePath } from '@/lib/copilot/vfs/path-utils'
2723
import {
2824
LandingPromptStorage,
2925
type LandingWorkflowSeed,
3026
LandingWorkflowSeedStorage,
3127
MothershipHandoffStorage,
32-
MothershipPendingContextStorage,
3328
} from '@/lib/core/utils/browser-storage'
3429
import {
3530
addMothershipContext,
@@ -335,43 +330,37 @@ export function Home({ chatId, userName, userId, tableViewsEnabled }: HomeProps)
335330
}, [sendMessage])
336331

337332
/**
338-
* Consumes a one-shot handoff left by another surface (e.g. "Troubleshoot in
339-
* Chat" on an errored log viewed from a different route) and auto-sends it
340-
* into this fresh chat, tagging the run so Sim can inspect the failure. Only
341-
* the cross-route path lands here — when a chat is already mounted the event
342-
* above delivers directly. Gated to the new-chat surface (`!chatId`): a
333+
* Consumes a one-shot handoff left by another surface and applies it to this
334+
* fresh chat. Two shapes arrive here: a message handoff (e.g. "Troubleshoot in
335+
* Chat" on an errored log) is auto-sent with its contexts attached; a
336+
* chip-only handoff (highlight-to-chat from the standalone Files/Tables pages)
337+
* seeds reference chips and sends nothing.
338+
*
339+
* Only the cross-route path lands here — when a chat is already mounted the
340+
* events deliver directly. Gated to the new-chat surface (`!chatId`): a
343341
* handoff always targets a fresh chat, so an existing `/chat/[chatId]` mount
344342
* must never claim it if navigation races. `consume` clears the entry
345343
* atomically, so it fires at most once even across a StrictMode remount.
344+
*
345+
* Chip-only handoffs open each resource directly rather than relying on the
346+
* input's listener being mounted, then dispatch so the input inserts the chip.
347+
* This effect is declared after `useChat`, so its chat-init `setResources([])`
348+
* has already flushed and cannot wipe the just-opened resource.
346349
*/
347350
useEffect(() => {
348351
if (chatId) return
349352
const handoff = MothershipHandoffStorage.consume(workspaceId)
350-
if (handoff) sendMessage(handoff.message, undefined, handoff.contexts)
351-
}, [chatId, workspaceId, sendMessage])
352-
353-
/**
354-
* Drains contexts persisted by the highlight-to-chat action (standalone
355-
* Files/Tables page). Runs after this component's mount effects — including
356-
* `useChat`'s chat-init `setResources([])` — so re-dispatching each context
357-
* inserts its chip in the (already mounted) conversation input AND opens its
358-
* resource in the slideover without the reset wiping it. A ref guards against
359-
* the StrictMode double-invoke draining twice.
360-
*/
361-
const hasDrainedPendingContextRef = useRef(false)
362-
useEffect(() => {
363-
if (hasDrainedPendingContextRef.current || !workspaceId) return
364-
hasDrainedPendingContextRef.current = true
365-
const pending = MothershipPendingContextStorage.consume(workspaceId)
366-
for (const context of pending) {
367-
// Open the resource in the slideover directly (deterministic — not
368-
// dependent on the input's event listener being mounted yet), then
369-
// dispatch the event so the mounted input inserts the chip.
353+
if (!handoff) return
354+
if (handoff.message) {
355+
sendMessage(handoff.message, undefined, handoff.contexts)
356+
return
357+
}
358+
for (const context of handoff.contexts ?? []) {
370359
handleContextAdd(context)
371360
addMothershipContext(context)
372361
}
373-
// eslint-disable-next-line react-hooks/exhaustive-deps -- mount-only drain; handleContextAdd is stable enough for a one-shot
374-
}, [workspaceId])
362+
// eslint-disable-next-line react-hooks/exhaustive-deps -- one-shot drain; handleContextAdd is a stable body function
363+
}, [chatId, workspaceId, sendMessage])
375364

376365
function resolveResourceFromContext(
377366
context: ChatContext
@@ -396,13 +385,13 @@ export function Home({ chatId, userName, userId, tableViewsEnabled }: HomeProps)
396385
}
397386

398387
/**
399-
* Tab title for the resource a chip opens. Selection chips carry a
400-
* location suffix in their label (`notes.md:12-40`, `Sales (3 rows)`); the
401-
* underlying resource is the whole file/table, so strip the suffix.
388+
* Tab title for the resource a chip opens. A selection chip's label describes
389+
* the selection (`notes.md:12-40`, `Sales (3 rows)`) but the tab shows the
390+
* whole file/table, so title it from the resource name the context carries.
402391
*/
403392
function resourceTitleForContext(context: ChatContext): string {
404-
if (context.kind === 'file_selection') return fileNameFromSelectionLabel(context.label)
405-
if (context.kind === 'table_selection') return tableNameFromSelectionLabel(context.label)
393+
if (context.kind === 'file_selection') return context.fileName
394+
if (context.kind === 'table_selection') return context.tableName
406395
return context.label
407396
}
408397

apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,13 +318,18 @@ function isChatContext(value: unknown): value is ChatContext {
318318
case 'table_selection':
319319
return (
320320
typeof value.tableId === 'string' &&
321+
typeof value.tableName === 'string' &&
321322
Array.isArray(value.rowIds) &&
322323
value.rowIds.every((id) => typeof id === 'string')
323324
)
324325
case 'file':
325326
return typeof value.fileId === 'string'
326327
case 'file_selection':
327-
return typeof value.fileId === 'string' && typeof value.text === 'string'
328+
return (
329+
typeof value.fileId === 'string' &&
330+
typeof value.fileName === 'string' &&
331+
typeof value.text === 'string'
332+
)
328333
case 'folder':
329334
return typeof value.folderId === 'string'
330335
case 'filefolder':
@@ -3231,13 +3236,15 @@ export function useChat(
32313236
...(c.kind === 'mcp' && 'serverId' in c ? { serverId: c.serverId } : {}),
32323237
...(c.kind === 'file_selection'
32333238
? {
3239+
fileName: c.fileName,
32343240
text: c.text,
32353241
...(c.startLine ? { startLine: c.startLine } : {}),
32363242
...(c.endLine ? { endLine: c.endLine } : {}),
32373243
}
32383244
: {}),
32393245
...(c.kind === 'table_selection'
32403246
? {
3247+
tableName: c.tableName,
32413248
rowIds: c.rowIds,
32423249
...(c.columnIds ? { columnIds: c.columnIds } : {}),
32433250
}

apps/sim/app/workspace/[workspaceId]/home/types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,13 @@ export interface ChatMessageContext {
150150
serverId?: string
151151
/** Selected passage for a `file_selection` context. */
152152
text?: string
153+
/** Source file name for a `file_selection` context. */
154+
fileName?: string
153155
/** 1-based inclusive line range for a `file_selection` context. */
154156
startLine?: number
155157
endLine?: number
158+
/** Source table name for a `table_selection` context. */
159+
tableName?: string
156160
/** Selected row ids for a `table_selection` context. */
157161
rowIds?: string[]
158162
/** Selected column ids for a `table_selection` cell range. */

0 commit comments

Comments
 (0)