Skip to content

Commit c31aaf1

Browse files
kerrylukerrylu
committed
fix(tables): address memory table review findings
1 parent a506b28 commit c31aaf1

8 files changed

Lines changed: 143 additions & 33 deletions

File tree

‎apps/sim/app/api/table/[tableId]/export/route.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,20 @@ interface RouteParams {
2424
params: Promise<{ tableId: string }>
2525
}
2626

27+
/** HEAD /api/table/[tableId]/export - Validates download access before browser navigation. */
28+
export const HEAD = withRouteHandler(async (request: NextRequest, { params }: RouteParams) => {
29+
const requestId = generateRequestId()
30+
const { tableId } = tableIdParamsSchema.parse(await params)
31+
const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
32+
if (!auth.success || !auth.userId) {
33+
return NextResponse.json({ error: 'Authentication required' }, { status: 401 })
34+
}
35+
36+
const access = await checkAccess(tableId, auth.userId, 'read')
37+
if (!access.ok) return accessError(access, requestId, tableId)
38+
return new NextResponse(null, { status: 204, headers: { 'Cache-Control': 'no-store' } })
39+
})
40+
2741
/** GET /api/table/[tableId]/export - Streams the full table contents as CSV or JSON. */
2842
export const GET = withRouteHandler(async (request: NextRequest, { params }: RouteParams) => {
2943
const requestId = generateRequestId()

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/column-header-menu.test.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44
import { act } from 'react'
55
import { createRoot } from 'react-dom/client'
6-
import { afterEach, describe, expect, it, vi } from 'vitest'
6+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
77

88
vi.mock('@sim/emcn', () => ({ cn: (...values: unknown[]) => values.filter(Boolean).join(' ') }))
99
vi.mock('@sim/emcn/icons', () => ({ ChevronDown: () => null }))
@@ -48,6 +48,10 @@ const HANDLERS = {
4848
onOpenConfig: vi.fn(),
4949
}
5050

51+
beforeEach(() => {
52+
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
53+
})
54+
5155
afterEach(() => {
5256
vi.clearAllMocks()
5357
document.body.replaceChildren()

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/column-header-menu.tsx‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -344,17 +344,19 @@ export const ColumnHeaderMenu = React.memo(function ColumnHeaderMenu({
344344
/>
345345
</div>
346346
)}
347-
<div
348-
className='-right-[3px] absolute top-0 z-[1] h-full w-[6px] cursor-col-resize'
349-
draggable={false}
350-
onDragStart={(e) => e.stopPropagation()}
351-
onPointerDown={handleResizePointerDown}
352-
onDoubleClick={(e) => {
353-
e.preventDefault()
354-
e.stopPropagation()
355-
onAutoResize(column.key)
356-
}}
357-
/>
347+
{!readOnly && (
348+
<div
349+
className='-right-[3px] absolute top-0 z-[1] h-full w-[6px] cursor-col-resize'
350+
draggable={false}
351+
onDragStart={(e) => e.stopPropagation()}
352+
onPointerDown={handleResizePointerDown}
353+
onDoubleClick={(e) => {
354+
e.preventDefault()
355+
e.stopPropagation()
356+
onAutoResize(column.key)
357+
}}
358+
/>
359+
)}
358360
</th>
359361
)
360362
})

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ describe('TableGrid virtual cells', () => {
153153
<TableGrid
154154
workspaceId='workspace-1'
155155
tableId='virtual-table'
156+
remoteSelections={[]}
157+
emitCellSelection={vi.fn()}
156158
locks={{
157159
schemaLocked: true,
158160
insertLocked: true,
@@ -214,6 +216,8 @@ describe('TableGrid virtual cells', () => {
214216
<TableGrid
215217
workspaceId='workspace-1'
216218
tableId='virtual-table'
219+
remoteSelections={[]}
220+
emitCellSelection={vi.fn()}
217221
locks={{
218222
schemaLocked: true,
219223
insertLocked: true,

‎apps/sim/hooks/queries/tables.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,6 +2104,12 @@ export async function downloadTableExport(
21042104
format: 'csv' | 'json' = 'csv'
21052105
): Promise<void> {
21062106
const url = `/api/table/${tableId}/export?format=${format}&t=${Date.now()}`
2107+
// boundary-raw-fetch: HEAD preflights a streaming download before browser navigation owns it
2108+
const response = await fetch(url, { method: 'HEAD' })
2109+
if (!response.ok) {
2110+
const status = [response.status, response.statusText].filter(Boolean).join(' ')
2111+
throw new Error(`Unable to export table (${status})`)
2112+
}
21072113
const safeName = fileName.replace(/[^a-zA-Z0-9_-]+/g, '_').replace(/^_+|_+$/g, '') || 'table'
21082114
const a = document.createElement('a')
21092115
a.href = url

‎apps/sim/lib/virtual-tables/memory-virtual-table.server.test.ts‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,11 @@ describe('Memory virtual table', () => {
113113
Array.from(strings)
114114
.join('')
115115
.match(/::text/g)
116-
).toHaveLength(5)
117-
expect([values[0], values[2], values[4], values[6], values[8]]).toEqual([
116+
).toHaveLength(6)
117+
expect([values[0], values[2], values[4], values[6], values[8], values[10]]).toEqual([
118118
'id',
119119
'conversation_id',
120+
'transcript',
120121
'message_count',
121122
'created_at',
122123
'updated_at',
@@ -187,6 +188,7 @@ describe('Memory virtual table', () => {
187188
updatedAt: UPDATED_AT,
188189
data: [{ role: 'user', content: 'Hello' }],
189190
messageCount: 2,
191+
rowBytes: 100,
190192
},
191193
{
192194
id: 'memory-2',
@@ -195,8 +197,13 @@ describe('Memory virtual table', () => {
195197
updatedAt: UPDATED_AT,
196198
data: [{ role: 'user', content: 'Second' }],
197199
messageCount: 1,
200+
rowBytes: 100,
198201
},
199202
])
203+
queueTableRows(schemaMock.memory, [
204+
{ id: 'memory-1', data: [{ role: 'user', content: 'Hello' }] },
205+
{ id: 'memory-2', data: [{ role: 'user', content: 'Second' }] },
206+
])
200207
const result = await queryMemoryTableRows({
201208
workspaceId: 'workspace-1',
202209
sort: { message_count: 'asc' },
@@ -228,6 +235,7 @@ describe('Memory virtual table', () => {
228235
{ role: 'assistant', content: 'Hi' },
229236
],
230237
messageCount: 2,
238+
rowBytes: 200,
231239
},
232240
{
233241
id: 'memory-1',
@@ -236,9 +244,20 @@ describe('Memory virtual table', () => {
236244
updatedAt: UPDATED_AT,
237245
data: [{ role: 'user', content: 'First' }],
238246
messageCount: 1,
247+
rowBytes: 100,
239248
},
240249
])
241250
queueTableRows(schemaMock.memory, [{ value: 3 }])
251+
queueTableRows(schemaMock.memory, [
252+
{
253+
id: 'memory-2',
254+
data: [
255+
{ role: 'user', content: 'Hello' },
256+
{ role: 'assistant', content: 'Hi' },
257+
],
258+
},
259+
{ id: 'memory-1', data: [{ role: 'user', content: 'First' }] },
260+
])
242261
const result = await queryMemoryTableRows({
243262
workspaceId: 'workspace-1',
244263
limit: 2,
@@ -400,6 +419,7 @@ describe('Memory virtual table', () => {
400419
rows: [],
401420
totalCount: null,
402421
keysetValid: true,
422+
hasMore: false,
403423
})
404424

405425
expect(dbChainMockFns.select).toHaveBeenCalledTimes(2)
@@ -413,8 +433,10 @@ describe('Memory virtual table', () => {
413433
updatedAt: UPDATED_AT,
414434
data: [{ role: 'user', content: 'Hello' }],
415435
messageCount: 1,
436+
rowBytes: 100,
416437
}
417438
queueTableRows(schemaMock.memory, [candidate])
439+
queueTableRows(schemaMock.memory, [{ id: candidate.id, data: candidate.data }])
418440

419441
const offsetPage = await queryMemoryTableRows({
420442
workspaceId: 'workspace-1',
@@ -427,6 +449,7 @@ describe('Memory virtual table', () => {
427449
expect(dbChainMockFns.offset).toHaveBeenCalledWith(5)
428450

429451
queueTableRows(schemaMock.memory, [candidate])
452+
queueTableRows(schemaMock.memory, [{ id: candidate.id, data: candidate.data }])
430453

431454
const keysetWhereCall = dbChainMockFns.where.mock.calls.length
432455
await expect(

‎apps/sim/lib/virtual-tables/memory-virtual-table.server.ts‎

Lines changed: 74 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { db } from '@sim/db'
22
import { memory, workspace } from '@sim/db/schema'
3-
import { and, count, desc, eq, isNull, lt, max, or, sql } from 'drizzle-orm'
4-
import { TABLE_LIMITS } from '@/lib/table/constants'
3+
import { and, count, desc, eq, inArray, isNull, lt, max, or, sql } from 'drizzle-orm'
4+
import { getMaxPageBytes, TABLE_LIMITS } from '@/lib/table/constants'
55
import { TableQueryValidationError } from '@/lib/table/errors'
66
import {
77
buildFilterClause,
@@ -38,6 +38,16 @@ function referencesMemoryTranscript(value: unknown): boolean {
3838

3939
function createMemoryRowsQuery() {
4040
const messageCount = sql<number>`CASE WHEN jsonb_typeof(${memory.data}) = 'array' THEN jsonb_array_length(${memory.data}) ELSE 0 END`
41+
const createdAtIso = sql<string>`to_char(${memory.createdAt}, 'YYYY-MM-DD"T"HH24:MI:SS.MS"Z"')`
42+
const updatedAtIso = sql<string>`to_char(${memory.updatedAt}, 'YYYY-MM-DD"T"HH24:MI:SS.MS"Z"')`
43+
const rowData = sql<JsonValue>`jsonb_build_object(
44+
${MEMORY_TABLE_COLUMNS.id}::text, ${memory.id},
45+
${MEMORY_TABLE_COLUMNS.conversationId}::text, ${memory.key},
46+
${MEMORY_TABLE_COLUMNS.transcript}::text, ${memory.data},
47+
${MEMORY_TABLE_COLUMNS.messageCount}::text, ${messageCount},
48+
${MEMORY_TABLE_COLUMNS.createdAt}::text, ${createdAtIso},
49+
${MEMORY_TABLE_COLUMNS.updatedAt}::text, ${updatedAtIso}
50+
)`
4151
return db
4252
.select({
4353
id: memory.id,
@@ -48,12 +58,13 @@ function createMemoryRowsQuery() {
4858
deletedAt: memory.deletedAt,
4959
transcript: sql<JsonValue>`${memory.data}`.as('transcript'),
5060
messageCount: messageCount.mapWith(Number).as('message_count'),
61+
rowBytes: sql<number>`octet_length((${rowData})::text)`.mapWith(Number).as('row_bytes'),
5162
data: sql<JsonValue>`jsonb_build_object(
5263
${MEMORY_TABLE_COLUMNS.id}::text, ${memory.id},
5364
${MEMORY_TABLE_COLUMNS.conversationId}::text, ${memory.key},
5465
${MEMORY_TABLE_COLUMNS.messageCount}::text, ${messageCount},
55-
${MEMORY_TABLE_COLUMNS.createdAt}::text, ${memory.createdAt},
56-
${MEMORY_TABLE_COLUMNS.updatedAt}::text, ${memory.updatedAt}
66+
${MEMORY_TABLE_COLUMNS.createdAt}::text, ${createdAtIso},
67+
${MEMORY_TABLE_COLUMNS.updatedAt}::text, ${updatedAtIso}
5768
)`.as('data'),
5869
})
5970
.from(memory)
@@ -176,8 +187,8 @@ export async function queryMemoryTableRows({
176187
key: memoryRows.key,
177188
createdAt: memoryRows.createdAt,
178189
updatedAt: memoryRows.updatedAt,
179-
data: memoryRows.transcript,
180190
messageCount: memoryRows.messageCount,
191+
rowBytes: memoryRows.rowBytes,
181192
})
182193
.from(memoryRows)
183194
.where(pageWhere)
@@ -189,24 +200,69 @@ export async function queryMemoryTableRows({
189200
? db.select({ value: count() }).from(memoryRows).where(baseWhere)
190201
: Promise.resolve(null)
191202
const [candidates, totalRows] = await Promise.all([candidatePromise, totalPromise])
192-
const rows = candidates.map((candidate, index) =>
193-
mapMemoryRecordToTableRow(
194-
{
195-
id: candidate.id,
196-
key: candidate.key,
197-
data: candidate.data,
198-
messageCount: candidate.messageCount,
199-
createdAt: candidate.createdAt,
200-
updatedAt: candidate.updatedAt,
201-
},
202-
offset + index
203-
)
204-
)
203+
const pageByteBudget = getMaxPageBytes() ?? TABLE_LIMITS.MAX_QUERY_RESULT_BYTES
204+
const selectedCandidates: typeof candidates = []
205+
let selectedBytes = 0
206+
let hasMore = false
207+
208+
for (const candidate of candidates) {
209+
const rowBytes = Number(candidate.rowBytes)
210+
if (!Number.isFinite(rowBytes) || rowBytes < 0) {
211+
throw new TableQueryValidationError('Memory table returned an invalid row size')
212+
}
213+
if (selectedCandidates.length === 0 && rowBytes > pageByteBudget) {
214+
throw new TableQueryValidationError(
215+
`Memory transcript exceeds the ${Math.floor(pageByteBudget / (1024 * 1024))}MB query response limit`,
216+
'TABLE_QUERY_RESULT_TOO_LARGE'
217+
)
218+
}
219+
if (selectedCandidates.length > 0 && selectedBytes + rowBytes > pageByteBudget) {
220+
hasMore = true
221+
break
222+
}
223+
selectedCandidates.push(candidate)
224+
selectedBytes += rowBytes
225+
}
226+
227+
const selectedIds = selectedCandidates.map((candidate) => candidate.id)
228+
const transcripts =
229+
selectedIds.length > 0
230+
? await db
231+
.select({ id: memory.id, data: sql<JsonValue>`${memory.data}` })
232+
.from(memory)
233+
.where(
234+
and(
235+
eq(memory.workspaceId, workspaceId),
236+
isNull(memory.deletedAt),
237+
inArray(memory.id, selectedIds)
238+
)
239+
)
240+
.limit(selectedIds.length)
241+
: []
242+
const transcriptById = new Map(transcripts.map((record) => [record.id, record.data]))
243+
const rows = selectedCandidates.flatMap((candidate, index) => {
244+
const transcript = transcriptById.get(candidate.id)
245+
if (transcript === undefined) return []
246+
return [
247+
mapMemoryRecordToTableRow(
248+
{
249+
id: candidate.id,
250+
key: candidate.key,
251+
data: transcript,
252+
messageCount: candidate.messageCount,
253+
createdAt: candidate.createdAt,
254+
updatedAt: candidate.updatedAt,
255+
},
256+
offset + index
257+
),
258+
]
259+
})
205260

206261
return {
207262
rows,
208263
totalCount: totalRows ? Number(totalRows[0].value) : null,
209264
keysetValid: !sort,
265+
hasMore,
210266
}
211267
}
212268
/** Searches Memory cells in PostgreSQL while preserving the active view's row ordinals. */

‎apps/sim/lib/virtual-tables/service.server.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ interface VirtualTablePage {
1515
rows: TableRow[]
1616
totalCount: number | null
1717
keysetValid: boolean
18+
hasMore?: boolean
1819
}
1920

2021
interface VirtualTable {
@@ -78,7 +79,7 @@ export async function queryVirtualTableRows(
7879
limit: limit + 1,
7980
offset,
8081
})
81-
const hasMore = page.rows.length > limit
82+
const hasMore = page.hasMore === true || page.rows.length > limit
8283
const rows = page.rows.slice(0, limit)
8384
const lastRow = rows[rows.length - 1]
8485

0 commit comments

Comments
 (0)