Skip to content

Commit b7e7220

Browse files
fix(enrichment): address review — persist detail on cancel/skip, exclude not_run from ran count, refetch on panel open
1 parent 18264bd commit b7e7220

5 files changed

Lines changed: 59 additions & 10 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/enrichment-details/enrichment-details.tsx

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { MAX_LOG_DETAILS_WIDTH_RATIO, MIN_LOG_DETAILS_WIDTH } from '@/stores/log
1919

2020
type EnrichmentDetailsTab = 'result' | 'cascade'
2121

22-
type ResultStatus = 'matched' | 'no_match' | 'error'
22+
type ResultStatus = 'matched' | 'no_match' | 'error' | 'not_run'
2323

2424
const RESULT_STATUS_CONFIG: Record<
2525
ResultStatus,
@@ -28,6 +28,7 @@ const RESULT_STATUS_CONFIG: Record<
2828
matched: { variant: 'green', label: 'Matched' },
2929
no_match: { variant: 'gray', label: 'No match' },
3030
error: { variant: 'red', label: 'Error' },
31+
not_run: { variant: 'gray', label: 'Not run' },
3132
}
3233

3334
/** Minimum bar width so a sub-millisecond provider still shows on the timeline. */
@@ -69,15 +70,21 @@ function buildCascadeRows(providers: EnrichmentProviderOutcome[]): CascadeRow[]
6970
})
7071
}
7172

73+
/** A provider that actually executed its tool (not skipped / never reached). */
74+
function didRun(p: EnrichmentProviderOutcome): boolean {
75+
return p.status !== 'skipped' && p.status !== 'not_run'
76+
}
77+
7278
/**
7379
* Derives the cell-level outcome from the cascade — mirrors the executor: a run
74-
* is `error` only when every provider that actually ran errored; a clean miss is
75-
* `no_match`.
80+
* is `error` only when every provider that ran errored; `not_run` when nothing
81+
* executed (missing inputs / cancelled early); otherwise a clean `no_match`.
7682
*/
7783
function deriveResultStatus(detail: EnrichmentRunDetail): ResultStatus {
7884
if (detail.matchedProvider) return 'matched'
79-
const ran = detail.providers.filter((p) => p.status !== 'skipped')
80-
if (ran.length > 0 && ran.every((p) => p.status === 'error')) return 'error'
85+
const ran = detail.providers.filter(didRun)
86+
if (ran.length === 0) return 'not_run'
87+
if (ran.every((p) => p.status === 'error')) return 'error'
8188
return 'no_match'
8289
}
8390

@@ -131,7 +138,7 @@ function EnrichmentDetailsContent({
131138
? (detail.providers.find((p) => p.id === detail.matchedProvider)?.label ??
132139
detail.matchedProvider)
133140
: null
134-
const ranCount = detail ? detail.providers.filter((p) => p.status !== 'skipped').length : 0
141+
const ranCount = detail ? detail.providers.filter(didRun).length : 0
135142
const lastError = detail
136143
? [...detail.providers].reverse().find((p) => p.status === 'error')?.error
137144
: null
@@ -229,7 +236,7 @@ function EnrichmentDetailsContent({
229236
{/* Provider waterfall — each row is one cascade attempt on a shared timeline */}
230237
<div className='flex flex-col pb-4'>
231238
{buildCascadeRows(detail.providers).map(({ outcome, offsetPct, widthPct }) => {
232-
const ran = outcome.status !== 'skipped' && outcome.status !== 'not_run'
239+
const ran = didRun(outcome)
233240
const { icon: ProviderIcon, bgColor: rawBgColor } = getBlockIconAndColor(
234241
'tool',
235242
outcome.toolId

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ async function runWorkflowAndWriteTerminal(
208208
// workflow path rather than erroring.
209209
if (group.type === 'enrichment' && group.enrichmentId) {
210210
const { getEnrichment } = await import('@/enrichments/registry')
211-
const { runEnrichment } = await import('@/enrichments/run')
211+
const { runEnrichment, skippedEnrichmentDetail } = await import('@/enrichments/run')
212212
const enrichment = getEnrichment(group.enrichmentId)
213213
// `tableRowExecutions.workflowId` is an opaque id for status; use the
214214
// enrichment id for enrichment cells.
@@ -320,6 +320,7 @@ async function runWorkflowAndWriteTerminal(
320320
jobId: null,
321321
workflowId: statusId,
322322
error: null,
323+
enrichmentDetails: skippedEnrichmentDetail(enrichment),
323324
},
324325
clearPatch
325326
)
@@ -334,6 +335,7 @@ async function runWorkflowAndWriteTerminal(
334335
jobId: null,
335336
workflowId: statusId,
336337
error: 'Cancelled',
338+
enrichmentDetails: skippedEnrichmentDetail(enrichment),
337339
})
338340
return 'error'
339341
}
@@ -352,6 +354,7 @@ async function runWorkflowAndWriteTerminal(
352354
jobId: null,
353355
workflowId: statusId,
354356
error: 'Cancelled',
357+
enrichmentDetails: detail,
355358
})
356359
return 'error'
357360
}

apps/sim/enrichments/run.test.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
66
const { mockExecuteTool } = vi.hoisted(() => ({ mockExecuteTool: vi.fn() }))
77
vi.mock('@/tools', () => ({ executeTool: mockExecuteTool }))
88

9-
import { runEnrichment } from '@/enrichments/run'
9+
import { runEnrichment, skippedEnrichmentDetail } from '@/enrichments/run'
1010
import type { EnrichmentConfig, EnrichmentProvider } from '@/enrichments/types'
1111

1212
const ICON = (() => null) as unknown as EnrichmentConfig['icon']
@@ -117,6 +117,14 @@ describe('runEnrichment cascade detail', () => {
117117
expect(outcome.detail.providers.map((p) => p.status)).toEqual(['no_match'])
118118
})
119119

120+
it('skippedEnrichmentDetail marks every provider skipped without running', () => {
121+
const detail = skippedEnrichmentDetail(config([prov('a'), prov('b')]))
122+
expect(detail.matchedProvider).toBeNull()
123+
expect(detail.totalCost).toBe(0)
124+
expect(detail.providers.map((p) => p.status)).toEqual(['skipped', 'skipped'])
125+
expect(mockExecuteTool).not.toHaveBeenCalled()
126+
})
127+
120128
it('does not error when some providers no-match and only some error', async () => {
121129
mockExecuteTool.mockImplementation((toolId: string) => {
122130
if (toolId === 'tool_a') return { success: false, output: { status: 500 } }

apps/sim/enrichments/run.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,32 @@ export interface EnrichmentRunOutcome {
2424
detail: EnrichmentRunDetail
2525
}
2626

27+
/**
28+
* Detail for a terminal cell that recorded no provider attempt — missing
29+
* required inputs, or cancelled before any provider ran. Every provider is
30+
* marked `skipped` so the details panel stays informative (shows the configured
31+
* cascade) instead of empty.
32+
*/
33+
export function skippedEnrichmentDetail(enrichment: EnrichmentConfig): EnrichmentRunDetail {
34+
const now = new Date().toISOString()
35+
return {
36+
startedAt: now,
37+
completedAt: now,
38+
durationMs: 0,
39+
totalCost: 0,
40+
matchedProvider: null,
41+
providers: enrichment.providers.map((provider) => ({
42+
id: provider.id,
43+
label: provider.label,
44+
toolId: provider.toolId,
45+
status: 'skipped' as const,
46+
cost: 0,
47+
durationMs: 0,
48+
error: null,
49+
})),
50+
}
51+
}
52+
2753
/** True when at least one output value in the result is non-empty. */
2854
function hasResult(result: Record<string, unknown>): boolean {
2955
return Object.values(result).some((v) => v !== undefined && v !== null && v !== '')

apps/sim/hooks/queries/tables.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,11 @@ async function fetchEnrichmentDetail(
307307
* Enrichment cascade breakdown for one cell, fetched on demand when the
308308
* enrichment details panel opens. Kept off the hot grid read — only queried
309309
* while `enabled` (panel open with a selected row + group).
310+
*
311+
* `staleTime: 0` so reopening the panel always refetches: a cell can be re-run
312+
* between opens (the run writes new `enrichmentDetails` in the background with no
313+
* client invalidation), and the panel is opened on demand, so a fresh fetch per
314+
* open keeps the cascade in sync without a cached stale run.
310315
*/
311316
export function useEnrichmentDetail(
312317
tableId: string,
@@ -319,7 +324,7 @@ export function useEnrichmentDetail(
319324
queryFn: ({ signal }) =>
320325
fetchEnrichmentDetail(tableId, rowId as string, groupId as string, signal),
321326
enabled: Boolean(tableId && rowId && groupId) && (options?.enabled ?? true),
322-
staleTime: 30 * 1000,
327+
staleTime: 0,
323328
})
324329
}
325330

0 commit comments

Comments
 (0)