Skip to content

Commit dac8313

Browse files
committed
fix(chat): scope selections to the columns actually picked, and stop under-counting rows
Three findings from one Bugbot round. Hidden columns widened cell ranges (reported twice). buildTableSelectionContext and contextMenuColumnIds collapsed a range to an open scope when it covered 'every column', comparing against displayColumns.length — which drops hidden columns AND expands workflow groups, so it never meant 'the whole schema'. Selecting every visible column therefore cleared columnIds and the server re-fetched columns the user had hidden. The collapse is removed rather than re-based on a schema count: no count available to a caller describes the schema, and an explicit column list is what the user actually selected. totalColumnCount is gone from the signature. Add-to-chat label undercounted rows. The menu derived its count from selectedRowCount (loaded rows only) while the chip was built from the full rowSel.ids set, so the label could promise fewer rows than were sent. Both now read one addToChatRowIds memo, with the count passed through explicitly since it legitimately differs from the count the delete/run labels use. Monaco line range was off by one. A full-line highlight ends at column 1 of the FOLLOWING line, so endLineNumber named a line that contributed no text — the chip label and the agent prompt both claimed an extra line. The collapse test I added last round asserted the buggy behavior as correct; it now pins the opposite, and fails if the collapse returns.
1 parent 65ddf42 commit dac8313

5 files changed

Lines changed: 69 additions & 37 deletions

File tree

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,13 @@ export const TextEditor = memo(function TextEditor({
390390
const text = model.getValueInRange(sel)
391391
if (!text.trim()) return null
392392
const startLine = sel.startLineNumber
393-
const endLine = sel.endLineNumber
393+
// A full-line highlight ends at column 1 of the FOLLOWING line, so that line
394+
// contributed no text — reporting it would claim a range one line longer
395+
// than what was selected, in both the chip label and the agent's prompt.
396+
const endLine =
397+
sel.endColumn === 1 && sel.endLineNumber > startLine
398+
? sel.endLineNumber - 1
399+
: sel.endLineNumber
394400
return {
395401
kind: 'file_selection',
396402
fileId: file.id,

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/context-menu/context-menu.tsx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,13 @@ interface ContextMenuProps {
6464
* {@link ContextMenuProps.workflowCellScoped}.
6565
*/
6666
addToChatCellScoped?: boolean
67+
/**
68+
* Rows the chip will reference. Differs from {@link ContextMenuProps.selectedRowCount}
69+
* because a gutter selection can extend past the loaded page and the chip
70+
* carries ids the server re-fetches, so the label must not promise fewer rows
71+
* than are actually sent. Defaults to `selectedRowCount`.
72+
*/
73+
addToChatRowCount?: number
6774
}
6875

6976
export function ContextMenu({
@@ -90,6 +97,7 @@ export function ContextMenu({
9097
disableDelete = false,
9198
onAddToChat,
9299
addToChatCellScoped = false,
100+
addToChatRowCount,
93101
}: ContextMenuProps) {
94102
const count = selectedRowCount.toLocaleString()
95103
const deleteLabel = selectedRowCount > 1 ? `Delete ${count} rows` : 'Delete row'
@@ -111,10 +119,11 @@ export function ContextMenu({
111119
runningInSelectionCount === 1
112120
? 'Stop running workflow'
113121
: `Stop ${runningInSelectionCount} running workflows`
122+
const addToChatRows = addToChatRowCount ?? selectedRowCount
114123
const addToChatLabel = addToChatCellScoped
115124
? 'Add cell range to Chat'
116-
: selectedRowCount > 1
117-
? `Add ${count} rows to Chat`
125+
: addToChatRows > 1
126+
? `Add ${addToChatRows.toLocaleString()} rows to Chat`
118127
: 'Add row to Chat'
119128

120129
return (

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

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2985,7 +2985,6 @@ export function TableGrid({
29852985
context: buildTableSelectionContext({
29862986
tableId,
29872987
tableName: tableNameRef.current,
2988-
totalColumnCount: cols.length,
29892988
// Every selected id, not just the loaded page: the chip carries
29902989
// ids and the server re-fetches them, so an unloaded row still
29912990
// reaches the agent. Only the pasted text is limited to `rows`.
@@ -3034,7 +3033,6 @@ export function TableGrid({
30343033
context: buildTableSelectionContext({
30353034
tableId,
30363035
tableName: tableNameRef.current,
3037-
totalColumnCount: cols.length,
30383036
rowIds: currentRows.map((row) => row.id),
30393037
columnIds: selectedColumnIds(cols, sel),
30403038
}),
@@ -3062,7 +3060,6 @@ export function TableGrid({
30623060
const rangeContext = buildTableSelectionContext({
30633061
tableId,
30643062
tableName: tableNameRef.current,
3065-
totalColumnCount: cols.length,
30663063
rowIds: rangeRowIds,
30673064
columnIds: selectedColumnIds(cols, sel),
30683065
})
@@ -3786,12 +3783,30 @@ export function TableGrid({
37863783
)
37873784
: contextMenuRowIds.length || 1
37883785

3786+
/**
3787+
* Rows the Add to Chat chip will reference, before the async drain that
3788+
* select-all and column selections perform. A gutter `some` selection can
3789+
* extend past the loaded page, and the chip carries ids the server re-fetches,
3790+
* so it uses the whole set rather than the loaded intersection
3791+
* `contextMenuRowIds` holds. Shared with the menu label so the count shown and
3792+
* the count sent can't disagree.
3793+
*/
3794+
const addToChatRowIds = useMemo<string[]>(() => {
3795+
if (
3796+
rowSelection.kind === 'some' &&
3797+
contextMenu.row &&
3798+
rowSelectionIncludes(rowSelection, contextMenu.row.id)
3799+
) {
3800+
return [...rowSelection.ids]
3801+
}
3802+
return contextMenuRowIds
3803+
}, [rowSelection, contextMenu.row, contextMenuRowIds])
3804+
37893805
/**
37903806
* Column ids for an "Add to chat" table selection. A spreadsheet-style cell
37913807
* range AND a column-header selection (which spans every row of the chosen
37923808
* columns) narrow the columns; whole-row (gutter) selections and single rows
3793-
* send every column (undefined). A range spanning all columns is equivalent to
3794-
* whole rows, so it also collapses to undefined.
3809+
* send every column (undefined).
37953810
*/
37963811
const contextMenuColumnIds = useMemo<string[] | undefined>(() => {
37973812
if (!contextMenu.isOpen || !contextMenu.row) return undefined
@@ -3807,8 +3822,11 @@ export function TableGrid({
38073822
if (contextRowArrayIndex < sel.startRow || contextRowArrayIndex > sel.endRow) return undefined
38083823
// Collapsed here too (not only in buildTableSelectionContext) because this
38093824
// also decides whether the menu item reads "cell range" or "rows".
3825+
// Not collapsed to `undefined` when it spans every visible column: hidden
3826+
// columns mean "all visible" is not "all", and widening would send the agent
3827+
// columns the user hid. See buildTableSelectionContext.
38103828
const ids = selectedColumnIds(displayColumns, sel)
3811-
return ids.length > 0 && ids.length < displayColumns.length ? ids : undefined
3829+
return ids.length > 0 ? ids : undefined
38123830
}, [contextMenu.isOpen, contextMenu.row, rowSelection, normalizedSelection, rows, displayColumns])
38133831

38143832
const addToChat = useAddToChat()
@@ -3818,14 +3836,7 @@ export function TableGrid({
38183836
// `contextMenuRowIds` reflects; drain up to the cap so the chip references as
38193837
// many rows as it can carry (bounded by MAX_TABLE_SELECTION_ROWS) instead of a
38203838
// silent loaded-only subset — mirroring how the copy path loads before writing.
3821-
const gutterSelection = rowSelectionRef.current
3822-
// Prefer the whole gutter selection over the loaded intersection.
3823-
let sourceRowIds =
3824-
gutterSelection.kind === 'some' &&
3825-
contextMenu.row &&
3826-
rowSelectionIncludes(gutterSelection, contextMenu.row.id)
3827-
? [...gutterSelection.ids]
3828-
: contextMenuRowIds
3839+
let sourceRowIds = addToChatRowIds
38293840
if (contextMenuIsSelectAll || isColumnSelectionRef.current) {
38303841
try {
38313842
const { rows: loaded } = await ensureRowsLoadedUpToRef.current(MAX_TABLE_SELECTION_ROWS)
@@ -3844,20 +3855,17 @@ export function TableGrid({
38443855
const context = buildTableSelectionContext({
38453856
tableId,
38463857
tableName: tableData?.name,
3847-
totalColumnCount: displayColumns.length,
38483858
rowIds: sourceRowIds,
38493859
columnIds: contextMenuColumnIds,
38503860
})
38513861
if (context) addToChat(context)
38523862
}, [
38533863
addToChat,
3854-
contextMenuRowIds,
3864+
addToChatRowIds,
38553865
contextMenuColumnIds,
38563866
contextMenuIsSelectAll,
3857-
contextMenu.row,
38583867
tableId,
38593868
tableData?.name,
3860-
displayColumns.length,
38613869
])
38623870

38633871
const pendingUpdate = updateRowMutation.isPending ? updateRowMutation.variables : null
@@ -4582,8 +4590,9 @@ export function TableGrid({
45824590
disableInsert={!canManualAddRow}
45834591
disableDuplicate={!canInsertFullRow}
45844592
disableDelete={!canDeleteRow}
4585-
onAddToChat={contextMenuRowIds.length > 0 ? handleAddSelectionToChat : undefined}
4593+
onAddToChat={addToChatRowIds.length > 0 ? handleAddSelectionToChat : undefined}
45864594
addToChatCellScoped={Boolean(contextMenuColumnIds)}
4595+
addToChatRowCount={contextMenuIsSelectAll ? selectedRowCount : addToChatRowIds.length}
45874596
/>
45884597

45894598
<ExpandedCellPopover

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.test.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ describe('selectedColumnIds', () => {
3030
})
3131

3232
describe('buildTableSelectionContext', () => {
33-
const base = { tableId: 't1', tableName: 'Sales', totalColumnCount: 3 }
33+
const base = { tableId: 't1', tableName: 'Sales' }
3434

3535
it('returns null before the table name has loaded, or with nothing selected', () => {
3636
expect(buildTableSelectionContext({ ...base, tableName: undefined, rowIds: ['r1'] })).toBeNull()
@@ -49,23 +49,30 @@ describe('buildTableSelectionContext', () => {
4949
expect(context.label).toContain(`${MAX_TABLE_SELECTION_ROWS} rows`)
5050
})
5151

52-
it('collapses a range covering every column to an open scope', () => {
53-
// Equivalent to whole rows — leaving it open keeps the server correct if the
54-
// schema changes, instead of pinning a now-stale column list.
52+
it('keeps a full-width range scoped rather than widening it to every column', () => {
53+
// Callers can only count rendered columns, which drop hidden ones and expand
54+
// workflow groups — so "covers everything visible" is not "covers the
55+
// schema". Widening here would re-fetch columns the user had hidden.
5556
const context = buildTableSelectionContext({
5657
...base,
5758
rowIds: ['r1'],
5859
columnIds: ['c0', 'c1', 'c2'],
5960
})
6061

62+
if (context?.kind !== 'table_selection') throw new Error('expected a table_selection')
63+
expect(context.columnIds).toEqual(['c0', 'c1', 'c2'])
64+
})
65+
66+
it('leaves the scope open only when no columns are given (whole rows)', () => {
67+
const context = buildTableSelectionContext({ ...base, rowIds: ['r1'] })
68+
6169
if (context?.kind !== 'table_selection') throw new Error('expected a table_selection')
6270
expect(context.columnIds).toBeUndefined()
6371
})
6472

6573
it('keeps a narrower range scoped, capped at the column limit', () => {
6674
const context = buildTableSelectionContext({
6775
...base,
68-
totalColumnCount: MAX_TABLE_SELECTION_COLUMNS + 50,
6976
rowIds: ['r1'],
7077
columnIds: Array.from({ length: MAX_TABLE_SELECTION_COLUMNS + 10 }, (_, i) => `c${i}`),
7178
})

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -373,26 +373,27 @@ export function selectedColumnIds(
373373

374374
/**
375375
* Materializes a `table_selection` chat context from a grid selection, applying
376-
* the shared row/column caps. `columnIds` narrows the context to a cell range; a
377-
* range covering every column is equivalent to whole rows, so it collapses to an
378-
* open scope (the server then includes all columns, and stays correct if the
379-
* schema changes). Returns null before the table name has loaded or when nothing
380-
* is selected.
376+
* the shared row/column caps. `columnIds` narrows the context to a cell range;
377+
* omit it for a whole-row selection, where the agent should see every column.
378+
* Returns null before the table name has loaded or when nothing is selected.
379+
*
380+
* A range is never widened back to an open scope for "covering everything":
381+
* the only counts available to callers come from the rendered grid, which both
382+
* drops hidden columns and expands workflow groups, so "all of them" cannot be
383+
* compared to the schema. Treating a full-width range as whole rows would let
384+
* the server re-fetch columns the user had hidden.
381385
*/
382386
export function buildTableSelectionContext(opts: {
383387
tableId: string
384388
tableName: string | undefined
385-
totalColumnCount: number
386389
rowIds: string[]
387390
columnIds?: string[]
388391
}): ChatContext | null {
389-
const { tableId, tableName, totalColumnCount, columnIds } = opts
392+
const { tableId, tableName, columnIds } = opts
390393
if (!tableName || opts.rowIds.length === 0) return null
391394
const rowIds = opts.rowIds.slice(0, MAX_TABLE_SELECTION_ROWS)
392395
const scopedColumnIds =
393-
columnIds && columnIds.length > 0 && columnIds.length < totalColumnCount
394-
? columnIds.slice(0, MAX_TABLE_SELECTION_COLUMNS)
395-
: undefined
396+
columnIds && columnIds.length > 0 ? columnIds.slice(0, MAX_TABLE_SELECTION_COLUMNS) : undefined
396397
return {
397398
kind: 'table_selection',
398399
tableId,

0 commit comments

Comments
 (0)