Skip to content

Commit f1ee3e9

Browse files
improvement(mothership): bounded delete above the cap runs async, not inline
An explicit delete limit now mirrors update: ≤1000 runs inline, above the cap it escalates to the background worker honoring the limit via maxRows — instead of always staying inline. The worker stops after maxRows (per-page fetch capped to the remaining budget). Bounded background deletes skip pendingDeleteMask: the filter-based mask hides every match, which would over-hide the rows beyond the cap the job never deletes. Unmasked, a bounded delete is eventually consistent like a bounded update (rows disappear as deleted), and doomedCount is omitted from the payload so the count isn't double-subtracted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d2c93ae commit f1ee3e9

5 files changed

Lines changed: 87 additions & 29 deletions

File tree

apps/sim/lib/copilot/tools/server/table/user-table.test.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -732,20 +732,35 @@ describe('userTableServerTool.delete_rows_by_filter', () => {
732732
})
733733
})
734734

735-
it('runs an explicit large limit inline without escalating (delete loads only ids)', async () => {
735+
it('escalates an explicit limit above the cap to a background delete with maxRows (unmasked)', async () => {
736+
mockQueryRows.mockResolvedValueOnce({
737+
rows: [],
738+
rowCount: 0,
739+
totalCount: 20000,
740+
limit: 1,
741+
offset: 0,
742+
})
743+
736744
const result = await userTableServerTool.execute(
737745
{
738746
operation: 'delete_rows_by_filter',
739747
args: { tableId: 'tbl_1', filter: { name: 'x' }, limit: 5000 },
740748
},
741749
{ userId: 'user-1', workspaceId: 'workspace-1' }
742750
)
751+
await flushDetached()
743752

744753
expect(result.success).toBe(true)
745-
// An explicit limit never counts/escalates — it deletes inline, bounded by the limit.
746-
expect(mockQueryRows).not.toHaveBeenCalled()
747-
expect(mockDeleteRowsByFilter).toHaveBeenCalledTimes(1)
748-
expect(mockDeleteRowsByFilter.mock.calls[0][1]).toMatchObject({ limit: 5000 })
754+
// target = min(limit 5000, matchCount 20000) = 5000, above the inline cap → background.
755+
expect(result.data?.doomedCount).toBe(5000)
756+
expect(mockDeleteRowsByFilter).not.toHaveBeenCalled()
757+
const [, , type, payload] = mockMarkTableJobRunning.mock.calls[0]
758+
expect(type).toBe('delete')
759+
// Bounded delete carries maxRows and omits doomedCount so the mask is skipped and the count
760+
// isn't double-subtracted.
761+
expect(payload).toMatchObject({ maxRows: 5000 })
762+
expect((payload as { doomedCount?: number }).doomedCount).toBeUndefined()
763+
expect(mockRunTableDelete.mock.calls[0][0]).toMatchObject({ maxRows: 5000 })
749764
})
750765

751766
it('deletes inline when the unbounded match count is within the cap', async () => {
@@ -801,6 +816,8 @@ describe('userTableServerTool.delete_rows_by_filter', () => {
801816
expect(tableId).toBe('tbl_1')
802817
expect(type).toBe('delete')
803818
expect(payload).toMatchObject({ doomedCount: 20000, cutoff: expect.any(String) })
819+
// Unbounded delete masks the whole set — no maxRows cap.
820+
expect((payload as { maxRows?: number }).maxRows).toBeUndefined()
804821
expect(mockRunTableDelete).toHaveBeenCalledTimes(1)
805822
expect(mockRunTableDelete.mock.calls[0][0]).toMatchObject({
806823
jobId,

apps/sim/lib/copilot/tools/server/table/user-table.ts

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,9 @@ async function dispatchDeleteJob(params: {
161161
workspaceId: string
162162
filter: Filter
163163
cutoff: Date
164+
maxRows?: number
164165
}): Promise<void> {
165-
const { jobId, tableId, workspaceId, filter, cutoff } = params
166+
const { jobId, tableId, workspaceId, filter, cutoff, maxRows } = params
166167
if (isTriggerDevEnabled) {
167168
try {
168169
const [{ tableDeleteTask }, { tasks }] = await Promise.all([
@@ -171,7 +172,7 @@ async function dispatchDeleteJob(params: {
171172
])
172173
await tasks.trigger<typeof tableDeleteTask>(
173174
'table-delete',
174-
{ jobId, tableId, workspaceId, filter, cutoff: cutoff.toISOString() },
175+
{ jobId, tableId, workspaceId, filter, cutoff: cutoff.toISOString(), maxRows },
175176
{ tags: [`tableId:${tableId}`, `jobId:${jobId}`] }
176177
)
177178
} catch (error) {
@@ -180,10 +181,12 @@ async function dispatchDeleteJob(params: {
180181
}
181182
} else {
182183
runDetached('table-delete', () =>
183-
runTableDelete({ jobId, tableId, workspaceId, filter, cutoff }).catch(async (error) => {
184-
await markTableDeleteFailed(tableId, jobId, error)
185-
throw error
186-
})
184+
runTableDelete({ jobId, tableId, workspaceId, filter, cutoff, maxRows }).catch(
185+
async (error) => {
186+
await markTableDeleteFailed(tableId, jobId, error)
187+
throw error
188+
}
189+
)
187190
)
188191
}
189192
}
@@ -817,27 +820,31 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
817820
const idByName = buildIdByName(table.schema)
818821
const idFilter = filterNamesToIds(args.filter, idByName)
819822

820-
// An explicit limit runs inline (delete loads only row ids, so even a large bounded
821-
// delete is light). Only an unbounded "delete everything matching" measures the blast
822-
// radius and hands off to the background delete worker (same path as the UI's select-all
823-
// delete) — the read-path mask hides exactly the all-matching set, which a bounded delete
824-
// would over-hide.
825-
if (args.limit === undefined) {
823+
// Inline handles up to MAX_BULK_OPERATION_SIZE rows; a larger delete (an explicit limit
824+
// above the cap, or unbounded "delete everything matching") hands off to the background
825+
// delete worker so a broad delete on a huge table doesn't load every matching id into this
826+
// request. A small explicit limit is the fast path.
827+
const deleteInlineEligible =
828+
args.limit !== undefined && args.limit <= TABLE_LIMITS.MAX_BULK_OPERATION_SIZE
829+
if (!deleteInlineEligible) {
826830
const { totalCount } = await queryRows(
827831
table,
828832
{ filter: idFilter, limit: 1, withExecutions: false },
829833
requestId
830834
)
831835
const matchCount = totalCount ?? 0
832-
if (matchCount > TABLE_LIMITS.MAX_BULK_OPERATION_SIZE) {
833-
const doomedCount = Math.min(matchCount, table.rowCount)
836+
const target = args.limit !== undefined ? Math.min(args.limit, matchCount) : matchCount
837+
if (target > TABLE_LIMITS.MAX_BULK_OPERATION_SIZE) {
838+
const doomedCount = Math.min(target, table.rowCount)
834839
const cutoff = new Date()
835840
const jobId = generateId()
836-
const payload: TableDeleteJobPayload = {
837-
filter: idFilter,
838-
cutoff: cutoff.toISOString(),
839-
doomedCount,
840-
}
841+
// Unbounded: mask the whole matching set (instant post-delete view), so `doomedCount`
842+
// drives the count adjustment. Bounded (maxRows): no mask — `doomedCount` is omitted so
843+
// the count isn't double-subtracted; rows disappear progressively as they're deleted.
844+
const bounded = args.limit !== undefined
845+
const payload: TableDeleteJobPayload = bounded
846+
? { filter: idFilter, cutoff: cutoff.toISOString(), maxRows: args.limit }
847+
: { filter: idFilter, cutoff: cutoff.toISOString(), doomedCount }
841848
assertNotAborted()
842849
const claimed = await markTableJobRunning(table.id, jobId, 'delete', payload)
843850
if (!claimed) {
@@ -849,10 +856,13 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
849856
workspaceId,
850857
filter: idFilter,
851858
cutoff,
859+
maxRows: args.limit,
852860
})
853861
return {
854862
success: true,
855-
message: `Started background delete of ${doomedCount} matching rows (job ${jobId}). The rows are hidden from reads immediately — query_rows already reflects the post-delete view.`,
863+
message: bounded
864+
? `Started background delete of up to ${doomedCount} matching rows (job ${jobId}). Rows delete in the background — query_rows to check progress.`
865+
: `Started background delete of ${doomedCount} matching rows (job ${jobId}). The rows are hidden from reads immediately — query_rows already reflects the post-delete view.`,
856866
data: { jobId, doomedCount },
857867
}
858868
}

apps/sim/lib/table/delete-runner.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,22 @@ describe('runTableDelete', () => {
8282
)
8383
})
8484

85+
it('stops once maxRows is reached and caps the final page fetch to the remaining budget', async () => {
86+
// budget 3 with page size 2: first page fills 2, the second is capped to the remaining 1.
87+
mockSelectRowIdPage.mockResolvedValueOnce(['a', 'b']).mockResolvedValueOnce(['c'])
88+
89+
await runTableDelete(basePayload({ filter: { status: 'old' }, maxRows: 3 }))
90+
91+
expect(mockSelectRowIdPage).toHaveBeenCalledTimes(2)
92+
expect(mockSelectRowIdPage.mock.calls[0][0]).toMatchObject({ limit: 2 })
93+
expect(mockSelectRowIdPage.mock.calls[1][0]).toMatchObject({ limit: 1 })
94+
expect(mockDeletePageByIds).toHaveBeenCalledTimes(2)
95+
expect(mockMarkJobReady).toHaveBeenCalledWith('tbl_1', 'job_1')
96+
expect(mockAppendTableEvent).toHaveBeenCalledWith(
97+
expect.objectContaining({ status: 'ready', progress: 3 })
98+
)
99+
})
100+
85101
it('skips excluded rows but still advances the keyset cursor past them', async () => {
86102
mockSelectRowIdPage.mockResolvedValueOnce(['keep', 'x']).mockResolvedValueOnce([])
87103

apps/sim/lib/table/delete-runner.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ export interface TableDeletePayload {
3636
excludeRowIds?: string[]
3737
/** Only rows created at/before this instant are deleted, so mid-job inserts survive. */
3838
cutoff: Date
39+
/**
40+
* Stop after deleting this many rows (an explicit caller-supplied limit). Omitted = every match.
41+
* Not combined with `excludeRowIds` (the UI's select-all path uses excludes and no cap; the
42+
* copilot tool uses a cap and no excludes), so the per-page fetch can be bounded directly.
43+
*/
44+
maxRows?: number
3945
}
4046

4147
/**
@@ -52,8 +58,9 @@ export interface TableDeletePayload {
5258
* newer job took the table) returns quietly.
5359
*/
5460
export async function runTableDelete(payload: TableDeletePayload): Promise<void> {
55-
const { jobId, tableId, workspaceId, filter, excludeRowIds, cutoff } = payload
61+
const { jobId, tableId, workspaceId, filter, excludeRowIds, cutoff, maxRows } = payload
5662
const requestId = generateId().slice(0, 8)
63+
const budget = maxRows ?? Number.POSITIVE_INFINITY
5764

5865
try {
5966
const table = await getTableById(tableId, { includeArchived: true })
@@ -74,7 +81,7 @@ export async function runTableDelete(payload: TableDeletePayload): Promise<void>
7481
let lastReported = resumed
7582
let afterId: string | undefined
7683

77-
while (true) {
84+
while (processed < budget) {
7885
// Ownership gate before every page: once this run loses the table (cancel/supersede),
7986
// updateJobProgress returns false and we stop before deleting further.
8087
const owns = await updateJobProgress(tableId, processed, jobId)
@@ -86,7 +93,7 @@ export async function runTableDelete(payload: TableDeletePayload): Promise<void>
8693
cutoff,
8794
filterClause,
8895
afterId,
89-
limit: TABLE_LIMITS.DELETE_PAGE_SIZE,
96+
limit: Math.min(TABLE_LIMITS.DELETE_PAGE_SIZE, budget - processed),
9097
})
9198
if (page.length === 0) break
9299
// Advance the keyset cursor past the whole page — excluded ids are skipped (not deleted),

apps/sim/lib/table/types.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,16 @@ export interface TableDeleteJobPayload {
213213
/** ISO timestamp; rows created after it are spared. */
214214
cutoff: string
215215
/** Doomed-row estimate captured at kickoff — display-only: list/detail counts subtract the
216-
* not-yet-deleted remainder (doomedCount - rows_processed) while the job runs. */
216+
* not-yet-deleted remainder (doomedCount - rows_processed) while the job runs. Set only for an
217+
* unbounded delete (the masked "delete everything matching" path); omitted when `maxRows` is set. */
217218
doomedCount?: number
219+
/**
220+
* Stop after deleting this many rows (an explicit caller-supplied limit above the inline cap).
221+
* Omitted = delete every match. When set, reads are NOT masked: the delete is eventually
222+
* consistent (rows disappear as they're deleted) like a bounded update, because the filter-based
223+
* mask would over-hide the rows beyond the cap that this job never deletes.
224+
*/
225+
maxRows?: number
218226
}
219227

220228
/**

0 commit comments

Comments
 (0)