Skip to content

Commit b2b1a83

Browse files
fix(table): address bugbot/greptile review feedback
Two P1 issues + one cleanup from the bot reviewers: 1. **Double-dispatch + completed-output wipe.** Both PATCH row routes (`app/api/table/[tableId]/rows/[rowId]` and `app/api/v1/tables/[tableId]/rows/[rowId]`) were firing a second `runWorkflowColumn({ mode: 'incomplete' })` after `updateRow` returns. `updateRow` already fires `mode: 'new'` internally for user edits, so the second call created a concurrent dispatch. Worse, the `mode: 'incomplete'` path's `bulkClearWorkflowGroupCells` wipes ALL targeted output columns on any row where any one column is empty — meaning sibling-group completed outputs could be erased. Removed both route-level calls; auto-dispatch lives entirely in `updateRow`. 2. **`runWorkflowColumn` log-spamming on plain tables.** `if (targetGroups.length === 0) throw new Error(...)` fired on every row insert/update for tables without any workflow groups (the majority). Every caller wraps with `.catch(logger.error)`, so each PATCH produced an error-level log. Return `{ dispatchId: null }` silently — manual `runWorkflowColumn` callers pass `groupIds` explicitly so they can't reach this branch. 3. **`isManualRun` plumbed through dispatch SSE events.** Late-arriving `kind: 'dispatch'` events for dispatches not in the initial fetch were hardcoding `isManualRun: false`. Added the field to the event shape, emit it from `dispatcherStep` (pending → complete, dispatching transitions) and `markActiveDispatchesCancelled`, and consume it in the SSE handler with a sensible fallback for legacy emits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7279172 commit b2b1a83

6 files changed

Lines changed: 29 additions & 28 deletions

File tree

apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import { generateRequestId } from '@/lib/core/utils/request'
1515
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1616
import type { RowData } from '@/lib/table'
1717
import { deleteRow, updateRow } from '@/lib/table'
18-
import { runWorkflowColumn } from '@/lib/table/workflow-columns'
1918
import { accessError, checkAccess } from '@/app/api/table/utils'
2019

2120
const logger = createLogger('TableRowAPI')
@@ -137,15 +136,11 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
137136
// Only `null` when a `cancellationGuard` is supplied and the SQL guard
138137
// rejects the write — this route doesn't pass one, so reaching null is a bug.
139138
if (!updatedRow) throw new Error('updateRow returned null without a cancellationGuard')
140-
// Auto-fire any newly-eligible workflow groups (deps just became met).
141-
void runWorkflowColumn({
142-
tableId,
143-
workspaceId: validated.workspaceId,
144-
rowIds: [updatedRow.id],
145-
mode: 'incomplete',
146-
isManualRun: false,
147-
requestId,
148-
}).catch((err) => logger.error(`[${requestId}] auto-dispatch (row PATCH) failed:`, err))
139+
// Auto-dispatch for user edits is handled inside `updateRow` (mode: 'new').
140+
// Firing a second mode: 'incomplete' dispatch here would race with the
141+
// `mode: 'new'` one AND bulk-clear sibling-group outputs (the incomplete
142+
// bulk-clear wipes ALL targeted columns when any one column on the row
143+
// is empty).
149144

150145
return NextResponse.json({
151146
success: true,

apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { generateRequestId } from '@/lib/core/utils/request'
1414
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1515
import type { RowData } from '@/lib/table'
1616
import { updateRow } from '@/lib/table'
17-
import { runWorkflowColumn } from '@/lib/table/workflow-columns'
1817
import { accessError, checkAccess } from '@/app/api/table/utils'
1918
import {
2019
checkRateLimit,
@@ -145,14 +144,9 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
145144
if (!updatedRow) {
146145
return NextResponse.json({ error: 'Row not found' }, { status: 404 })
147146
}
148-
void runWorkflowColumn({
149-
tableId,
150-
workspaceId: validated.workspaceId,
151-
rowIds: [updatedRow.id],
152-
mode: 'incomplete',
153-
isManualRun: false,
154-
requestId,
155-
}).catch((err) => logger.error(`[${requestId}] auto-dispatch (v1 row update) failed:`, err))
147+
// Auto-dispatch for user edits is handled inside `updateRow` (mode: 'new').
148+
// Firing a second mode: 'incomplete' dispatch here would race with it AND
149+
// bulk-clear sibling-group outputs.
156150

157151
return NextResponse.json({
158152
success: true,

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-event-stream.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ export function useTableEventStream({
145145
}
146146

147147
const applyDispatch = (event: Extract<TableEvent, { kind: 'dispatch' }>): void => {
148-
const { dispatchId, status, scope, cursor, mode } = event
148+
const { dispatchId, status, scope, cursor, mode, isManualRun } = event
149149
queryClient.setQueryData<TableRunState>(tableKeys.activeDispatches(tableId), (prev) => {
150150
if (!prev) return prev
151151
const list = prev.dispatches
@@ -160,19 +160,23 @@ export function useTableEventStream({
160160
// overlay. Leave existing cache alone.
161161
return prev
162162
}
163+
const idx = list.findIndex((d) => d.id === dispatchId)
164+
const existing = idx === -1 ? undefined : list[idx]
165+
// Prefer the event payload (current truth from server); fall back to
166+
// the cached entry's value if this is a legacy emit without the
167+
// field, and finally to `false` if we have nothing.
168+
const resolvedManualRun = isManualRun ?? existing?.isManualRun ?? false
163169
const next: ActiveDispatch = {
164170
id: dispatchId,
165171
status,
166172
mode,
167-
isManualRun: false,
173+
isManualRun: resolvedManualRun,
168174
cursor,
169175
scope,
170176
}
171-
const idx = list.findIndex((d) => d.id === dispatchId)
172177
if (idx === -1) return { ...prev, dispatches: [...list, next] }
173178
const merged = list.slice()
174-
// Preserve isManualRun from the initial fetch — SSE doesn't carry it.
175-
merged[idx] = { ...next, isManualRun: list[idx].isManualRun }
179+
merged[idx] = next
176180
return { ...prev, dispatches: merged }
177181
})
178182
}

apps/sim/lib/table/dispatcher.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ export async function dispatcherStep(dispatchId: string): Promise<DispatcherStep
298298
scope: dispatch.scope,
299299
cursor: dispatch.cursor,
300300
mode: dispatch.mode,
301+
isManualRun: dispatch.isManualRun,
301302
})
302303
return 'done'
303304
}
@@ -358,6 +359,7 @@ export async function dispatcherStep(dispatchId: string): Promise<DispatcherStep
358359
scope: dispatch.scope,
359360
cursor: lastPosition,
360361
mode: dispatch.mode,
362+
isManualRun: dispatch.isManualRun,
361363
}),
362364
])
363365

@@ -450,6 +452,7 @@ export async function markActiveDispatchesCancelled(tableId: string): Promise<Di
450452
scope: d.scope,
451453
cursor: d.cursor,
452454
mode: d.mode,
455+
isManualRun: d.isManualRun,
453456
})
454457
)
455458
)

apps/sim/lib/table/events.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,16 +98,17 @@ export type TableEvent =
9898
| {
9999
/** Dispatcher status signal emitted by `dispatcherStep` and the cancel
100100
* path. Drives the client-side "about to run" overlay for rows the
101-
* dispatcher hasn't reached yet. `scope` + `cursor` + `mode` are
102-
* carried on every transition so the client can upsert without
103-
* refetching the dispatches list. */
101+
* dispatcher hasn't reached yet. `scope` + `cursor` + `mode` +
102+
* `isManualRun` are carried on every transition so the client can
103+
* upsert without refetching the dispatches list. */
104104
kind: 'dispatch'
105105
tableId: string
106106
dispatchId: string
107107
status: TableDispatchStatus
108108
scope?: { groupIds: string[]; rowIds?: string[] }
109109
cursor?: number
110110
mode?: 'all' | 'incomplete' | 'new'
111+
isManualRun?: boolean
111112
}
112113

113114
export interface TableEventEntry {

apps/sim/lib/table/workflow-columns.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,11 @@ export async function runWorkflowColumn(opts: {
462462

463463
const allGroups = table.schema.workflowGroups ?? []
464464
const targetGroups = groupIds ? allGroups.filter((g) => groupIds.includes(g.id)) : allGroups
465-
if (targetGroups.length === 0) throw new Error('No matching workflow groups for run')
465+
// Tables with no workflow groups are the majority. Auto-fire callers from
466+
// every row write would otherwise produce error-level log spam on every
467+
// PATCH/insert. Manual run-column callers always pass `groupIds` so they
468+
// can't reach here with an empty target.
469+
if (targetGroups.length === 0) return { dispatchId: null }
466470
const targetGroupIds = targetGroups.map((g) => g.id)
467471

468472
// Wipe targeted output cols + executions[gid] before any cells fire so the

0 commit comments

Comments
 (0)