Skip to content

Commit 0ce8bea

Browse files
fix(enrichments): address PR review feedback
- Guard the enrichment cell path on `enrichmentId` so a group typed 'enrichment' without a registry id falls through to the workflow path instead of erroring. - Clear stale output values when skipping a row for missing required inputs, so the auto cascade re-enriches once inputs return (was left completed+filled). - Write a terminal state on abort in the enrichment path (matches the workflow path) so a cancel between run and terminal-write can't leave the cell running. - Edit mode: apply the group update (mappings/deps/auto-run) before column renames so the primary edit lands even if a rename fails. - Disable Save once validation has surfaced a missing required input. - Use the workflowGroupById map instead of O(n) find in the context-menu and action-bar hot paths.
1 parent 60fab03 commit 0ce8bea

3 files changed

Lines changed: 55 additions & 26 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/enrichments-sidebar/enrichment-config.tsx

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ export function EnrichmentConfig({
137137
addWorkflowGroup.isPending ||
138138
updateWorkflowGroup.isPending ||
139139
updateColumn.isPending ||
140+
(showValidation && missingRequired) ||
140141
!depsValid ||
141142
outputsInvalid
142143

@@ -151,22 +152,23 @@ export function EnrichmentConfig({
151152

152153
if (existingGroup) {
153154
try {
154-
// Rename any output columns the user changed first; the rename cascades
155+
// Apply the group edit (mappings / deps / auto-run) first so it lands
156+
// even if a later column rename fails. Renames run after and cascade
155157
// into the group's output refs server-side.
156-
for (const o of enrichment.outputs) {
157-
const original = originalOutputName(o.id)
158-
const next = (outputNames[o.id] ?? '').trim()
159-
if (original && next && next !== original) {
160-
await updateColumn.mutateAsync({ columnName: original, updates: { name: next } })
161-
}
162-
}
163158
await updateWorkflowGroup.mutateAsync({
164159
groupId: existingGroup.id,
165160
name: enrichment.name,
166161
dependencies: { columns: deps },
167162
inputMappings: inputMappingsList,
168163
autoRun,
169164
})
165+
for (const o of enrichment.outputs) {
166+
const original = originalOutputName(o.id)
167+
const next = (outputNames[o.id] ?? '').trim()
168+
if (original && next && next !== original) {
169+
await updateColumn.mutateAsync({ columnName: original, updates: { name: next } })
170+
}
171+
}
170172
toast.success(`Updated "${enrichment.name}"`)
171173
onClose()
172174
} catch (err) {

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -767,8 +767,7 @@ export function TableGrid({
767767
typeof _exec?.jobId === 'string' &&
768768
_exec.jobId.startsWith('paused-')
769769
// Enrichment cells have no workflow execution trace to open.
770-
const _isEnrichmentGroup =
771-
tableWorkflowGroups.find((wg) => wg.id === _gid)?.type === 'enrichment'
770+
const _isEnrichmentGroup = workflowGroupById.get(_gid)?.type === 'enrichment'
772771
contextMenuHasStartedRun =
773772
!_isEnrichmentGroup &&
774773
(_exec?.status === 'completed' ||
@@ -2930,8 +2929,7 @@ export function TableGrid({
29302929
status === 'pending' && typeof exec?.jobId === 'string' && exec.jobId.startsWith('paused-')
29312930
// Enrichment groups have no workflow execution to open — never offer "View
29322931
// execution" for them.
2933-
const isEnrichmentGroup =
2934-
tableWorkflowGroups.find((wg) => wg.id === groupId)?.type === 'enrichment'
2932+
const isEnrichmentGroup = workflowGroupById.get(groupId)?.type === 'enrichment'
29352933
return {
29362934
rowId: row.id,
29372935
groupId,
@@ -2940,7 +2938,7 @@ export function TableGrid({
29402938
!isEnrichmentGroup &&
29412939
(status === 'completed' || status === 'error' || status === 'running' || isPaused),
29422940
}
2943-
}, [normalizedSelection, rows, displayColumns, tableWorkflowGroups])
2941+
}, [normalizedSelection, rows, displayColumns, workflowGroupById])
29442942

29452943
const tableWorkflowGroupIds = useMemo(
29462944
() => tableWorkflowGroups.map((g) => g.id),

apps/sim/background/workflow-column-execution.ts

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,11 @@ async function runWorkflowAndWriteTerminal(
115115
writeWorkflowGroupState(cellCtx, { executionState, dataPatch })
116116

117117
// Enrichment groups call a registry function directly instead of running a
118-
// workflow, reusing the same pickup → run → terminal-write status flow.
119-
if (group.type === 'enrichment') {
118+
// workflow, reusing the same pickup → run → terminal-write status flow. The
119+
// `enrichmentId` guard ensures only true registry enrichments take this path
120+
// — a group typed 'enrichment' without a registry id falls through to the
121+
// workflow path rather than erroring.
122+
if (group.type === 'enrichment' && group.enrichmentId) {
120123
const { getEnrichment } = await import('@/enrichments/registry')
121124
const { runEnrichment } = await import('@/enrichments/run')
122125
const enrichment = getEnrichment(group.enrichmentId)
@@ -155,25 +158,42 @@ async function runWorkflowAndWriteTerminal(
155158
}
156159

157160
// Skip (don't error) rows missing a required input — common when a table
158-
// is partially filled. The cell completes empty and re-runs once the
159-
// input columns fill (if they're dependencies).
161+
// is partially filled. Clear any prior output values so a stale result
162+
// doesn't linger (and doesn't mark the group `completed`-and-filled, which
163+
// would block the auto cascade from re-enriching once inputs return).
160164
const isEmpty = (v: unknown) => v === undefined || v === null || v === ''
161165
const missingRequired = enrichment.inputs.some(
162166
(i) => i.required && isEmpty(enrichInputs[i.id])
163167
)
164168
if (missingRequired) {
165-
await writeState({
166-
status: 'completed',
167-
executionId,
168-
jobId: null,
169-
workflowId: statusId,
170-
error: null,
171-
})
169+
const clearPatch: RowData = {}
170+
for (const out of group.outputs) {
171+
if (!isEmpty(row.data[out.columnName])) clearPatch[out.columnName] = ''
172+
}
173+
await writeState(
174+
{
175+
status: 'completed',
176+
executionId,
177+
jobId: null,
178+
workflowId: statusId,
179+
error: null,
180+
},
181+
clearPatch
182+
)
172183
return 'completed'
173184
}
174185

175186
try {
176-
if (signal?.aborted) return 'error'
187+
if (signal?.aborted) {
188+
await writeState({
189+
status: 'error',
190+
executionId,
191+
jobId: null,
192+
workflowId: statusId,
193+
error: 'Cancelled',
194+
})
195+
return 'error'
196+
}
177197
const { result, cost, error } = await runEnrichment(enrichment, enrichInputs, {
178198
tableId,
179199
rowId,
@@ -182,7 +202,16 @@ async function runWorkflowAndWriteTerminal(
182202
})
183203

184204
// An abort during the cascade must not be recorded as a completed cell.
185-
if (signal?.aborted) return 'error'
205+
if (signal?.aborted) {
206+
await writeState({
207+
status: 'error',
208+
executionId,
209+
jobId: null,
210+
workflowId: statusId,
211+
error: 'Cancelled',
212+
})
213+
return 'error'
214+
}
186215

187216
// Every provider that ran errored (auth / rate-limit / outage) — surface
188217
// it rather than writing a blank cell that looks like "no data found".

0 commit comments

Comments
 (0)