Skip to content

Commit eff18e6

Browse files
committed
fix(chat): derive the budget reserve from the same clause it reserves for
Cursor Bugbot: worstCaseSizeClause built its own string that omitted the row/rows word the real clause always carries, so the reserve ran ~5 characters short and a tightly packed selection could still exceed MAX_TABLE_SELECTION_CONTENT_LENGTH. A bug in the previous fix, from duplicating the format instead of sharing it. Replaced with one sizeClause(shown, omitted) used for both the up-front reserve and the final prose, so the two cannot describe the count differently. The reserve passes (rows.length, rows.length) — max digits on both counts and the plural forced — which is an upper bound on any real clause. The earlier tight-packing test could not see this: a single cell width leaves whatever remainder it leaves, and 100 left more than 5 characters. Added a sweep over widths 60-75 that collects overflows so a failure names the width; it catches the reported bug at width 74 (20002 vs 20000).
1 parent 2e5f8f5 commit eff18e6

2 files changed

Lines changed: 59 additions & 15 deletions

File tree

apps/sim/lib/copilot/chat/process-contents.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,47 @@ describe('processContextsServer - table_selection contexts', () => {
531531
expect(ctx.content).toContain('omitted for length')
532532
})
533533

534+
it('holds the cap across cell widths, including ones that pack flush to it', async () => {
535+
// A single width can leave slack that hides an under-reserved prefix by a
536+
// few characters. Sweeping widths lands at least one run with almost no
537+
// remainder, which is where an off-by-N in the reserve actually shows up.
538+
getTableById.mockResolvedValue({
539+
name: 'Sales',
540+
workspaceId: 'ws-1',
541+
schema: { columns: [{ id: 'c_notes', name: 'Notes' }] },
542+
})
543+
544+
const overflows: Array<{ width: number; length: number }> = []
545+
for (let width = 60; width <= 75; width++) {
546+
const rows = Array.from({ length: MAX_TABLE_SELECTION_ROWS }, (_, i) => ({
547+
id: `r${i}`,
548+
data: { c_notes: 'x'.repeat(width) },
549+
}))
550+
getRowsByIds.mockResolvedValue(rows)
551+
552+
const result = await processContextsServer(
553+
[
554+
{
555+
kind: 'table_selection',
556+
tableId: 'tbl-1',
557+
tableName: 'Sales',
558+
label: 'Sales (500 rows)',
559+
rowIds: rows.map((r) => r.id),
560+
} as ChatContext,
561+
],
562+
'user-1',
563+
'summarize',
564+
'ws-1'
565+
)
566+
567+
const { length } = result[0].content
568+
if (length > MAX_TABLE_SELECTION_CONTENT_LENGTH) overflows.push({ width, length })
569+
}
570+
571+
// Collected rather than asserted per-iteration so a failure names the widths.
572+
expect(overflows).toEqual([])
573+
})
574+
534575
it('spends a character budget across rows and reports what it omitted', async () => {
535576
// Row/column caps alone don't bound prompt cost: wide cells blow past the
536577
// budget long before MAX_TABLE_SELECTION_ROWS.

apps/sim/lib/copilot/chat/process-contents.ts

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -931,14 +931,8 @@ async function resolveFileSelectionResource(
931931
}
932932

933933
/**
934-
* Longest form the size clause can take for `total` rows — every row omitted.
935-
* Used to reserve prefix space before the real counts are known.
934+
* Renders one cell for a markdown table row, escaping the delimiters.
936935
*/
937-
function worstCaseSizeClause(total: number): string {
938-
return `${total} of ${total}, ${total} omitted for length`
939-
}
940-
941-
/** Renders one cell for a markdown table row, escaping the delimiters. */
942936
function renderTableCell(value: unknown): string {
943937
if (value === null || value === undefined) return ''
944938
const cell = typeof value === 'string' ? value : JSON.stringify(value)
@@ -983,15 +977,27 @@ async function resolveTableSelectionResource(
983977
const describe = (size: string) =>
984978
`Selected ${scope} from table "${table.name}" (${size}):\n\n${header}\n${divider}\n`
985979

980+
/**
981+
* The size clause, e.g. `5 rows` or `189 rows of 500, 311 omitted for length`.
982+
* Used for both the up-front reserve and the final prose, so the two can never
983+
* describe the row count differently.
984+
*/
985+
const sizeClause = (shownCount: number, omittedCount: number) => {
986+
const shown = `${shownCount} ${shownCount === 1 ? 'row' : 'rows'}`
987+
return omittedCount > 0
988+
? `${shown} of ${rows.length}, ${omittedCount} omitted for length`
989+
: shown
990+
}
991+
986992
// Spend the character budget row by row. Everything that is not a row — the
987993
// prose, the table head, and every newline — is reserved up front, or the cap
988994
// is silently overrun whenever the last row leaves less slack than the prefix
989-
// needs. The reserve uses the longest form the size clause can take (every row
990-
// omitted), since its real value isn't known until packing finishes; a few
991-
// characters of unused slack beats overshooting the documented bound.
995+
// needs. The real clause isn't known until packing finishes, so reserve its
996+
// longest form: every row shown AND every row omitted maximizes both counts
997+
// and forces the plural. A few characters of unused slack beats overshooting.
992998
const lines: string[] = []
993999
let remaining =
994-
MAX_TABLE_SELECTION_CONTENT_LENGTH - describe(worstCaseSizeClause(rows.length)).length
1000+
MAX_TABLE_SELECTION_CONTENT_LENGTH - describe(sizeClause(rows.length, rows.length)).length
9951001
for (const row of rows) {
9961002
const line = `| ${columns.map((col) => renderTableCell(row.data[getColumnId(col)])).join(' | ')} |`
9971003
// The first row always goes in, so a single oversized row still yields a
@@ -1001,10 +1007,7 @@ async function resolveTableSelectionResource(
10011007
remaining -= line.length + 1
10021008
}
10031009

1004-
const omitted = rows.length - lines.length
1005-
const shown = `${lines.length} ${lines.length === 1 ? 'row' : 'rows'}`
1006-
const size = omitted > 0 ? `${shown} of ${rows.length}, ${omitted} omitted for length` : shown
1007-
const content = `${describe(size)}${lines.join('\n')}`
1010+
const content = `${describe(sizeClause(lines.length, rows.length - lines.length))}${lines.join('\n')}`
10081011
return {
10091012
type: 'table_selection',
10101013
tag: label ? `@${label}` : '@',

0 commit comments

Comments
 (0)