Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements a comprehensive debugging system for workflow execution in the Sim platform. The changes introduce a dedicated debug panel with step-by-step execution capabilities, variable inspection, and visual feedback for workflow debugging.
The core architectural changes include:
State Management Extensions: The execution store is extended with new debugging-specific state properties (panelFocusedBlockId, executingBlockIds, breakpointId) to track which blocks are focused in the debug panel, currently executing, and serve as breakpoints. These follow the established Zustand store pattern with proper state isolation.
Debug Panel Integration: A new 'debug' tab is added to the panel system, with a comprehensive 2000+ line debug component that handles complex scenarios including parallel execution (via virtual block IDs), variable scoping, environment variable filtering, and real-time execution tracking. The panel provides input/output visualization, clickable token navigation, and multi-tab interfaces for different aspects of debugging.
Execution Flow Changes: The executor now marks blocks with initial states as executed: false instead of executed: true, enabling proper debug stepping through the entire workflow including pre-existing states. Debug mode disables streaming execution to maintain synchronous, controllable execution flow essential for debugging.
Visual Feedback System: Workflow blocks now display comprehensive visual indicators including error states (highest priority), execution states (pulsing rings), focused states, and breakpoint indicators. The system handles both direct block execution and parallel execution scenarios with special virtual block ID logic.
Control Flow Reorganization: Debug controls are moved from the control bar to the dedicated debug panel, with the debug mode toggle now serving as a setup action that prepares the debugging environment without immediately executing. This provides a cleaner separation between high-level workflow actions and detailed debugging operations.
The implementation integrates deeply with the existing execution system while maintaining backward compatibility, leveraging the established store architecture and UI patterns throughout the codebase.
Confidence score: 3/5
- This PR introduces significant complexity with potential edge cases in parallel execution and state management
- Score reflects the substantial scope of changes across critical execution paths and complex debugging logic
- Pay close attention to the debug panel component and execution state management files
9 files reviewed, 8 comments
| context.blockStates.set(blockId, { | ||
| output: output as NormalizedBlockOutput, | ||
| executed: true, | ||
| executed: false, |
There was a problem hiding this comment.
logic: Changing executed: false enables debug stepping through blocks with initial states. Verify this doesn't break non-debug execution paths where these blocks should remain executed.
| executor: Executor | null | ||
| debugContext: ExecutionContext | null | ||
| autoPanDisabled: boolean | ||
| panelFocusedBlockId?: string | null |
There was a problem hiding this comment.
style: Consider making panelFocusedBlockId non-optional since it's initialized to null - the optional modifier is redundant
| panelFocusedBlockId?: string | null | |
| panelFocusedBlockId: string | null |
| <div className='fixed top-[76px] right-4 z-20 flex h-9 w-[308px] items-center gap-1 rounded-[14px] border bg-card px-[2.5px] py-1 shadow-xs'> | ||
| <div | ||
| className='fixed top-[76px] right-4 z-20 flex h-9 items-center gap-1 rounded-[14px] border bg-card px-[2.5px] py-1 shadow-xs' | ||
| style={{ width: isDebugging ? '380px' : '308px' }} |
There was a problem hiding this comment.
style: dynamic width calculation could use CSS custom properties for maintainability
| // Compute accessible output variables for the focused block with tag-style references | ||
| const outputVariableEntries = useMemo(() => { | ||
| if (!focusedBlockId) return [] as Array<{ ref: string; value: any }> | ||
|
|
||
| const normalizeBlockName = (name: string) => (name || '').replace(/\s+/g, '').toLowerCase() | ||
| const getSubBlockValue = (blockId: string, property: string): any => { | ||
| return useSubBlockStore.getState().getValue(blockId, property) | ||
| } | ||
| const generateOutputPaths = (outputs: Record<string, any>, prefix = ''): string[] => { | ||
| const paths: string[] = [] | ||
| for (const [key, value] of Object.entries(outputs || {})) { | ||
| const current = prefix ? `${prefix}.${key}` : key | ||
| if (typeof value === 'string') { | ||
| paths.push(current) | ||
| } else if (value && typeof value === 'object') { | ||
| if ('type' in value && typeof (value as any).type === 'string') { | ||
| paths.push(current) | ||
| if ((value as any).type === 'object' && (value as any).properties) { | ||
| paths.push(...generateOutputPaths((value as any).properties, current)) | ||
| } else if ((value as any).type === 'array' && (value as any).items?.properties) { | ||
| paths.push(...generateOutputPaths((value as any).items.properties, current)) | ||
| } | ||
| } else { | ||
| paths.push(...generateOutputPaths(value as Record<string, any>, current)) | ||
| } | ||
| } else { | ||
| paths.push(current) | ||
| } | ||
| } | ||
| return paths | ||
| } | ||
|
|
||
| const getAccessiblePathsForBlock = (blockId: string): string[] => { | ||
| const blk = blockById.get(blockId) | ||
| if (!blk) return [] | ||
| const cfg = getBlock(blk.type) | ||
| if (!cfg) return [] | ||
|
|
||
| // Response format overrides | ||
| const responseFormatValue = getSubBlockValue(blockId, 'responseFormat') | ||
| const responseFormat = parseResponseFormatSafely(responseFormatValue, blockId) | ||
| if (responseFormat) { | ||
| const fields = extractFieldsFromSchema(responseFormat) | ||
| if (fields.length > 0) return fields.map((f: any) => f.name) | ||
| } | ||
|
|
||
| if (blk.type === 'evaluator') { | ||
| const metricsValue = getSubBlockValue(blockId, 'metrics') | ||
| if (metricsValue && Array.isArray(metricsValue) && metricsValue.length > 0) { | ||
| const valid = metricsValue.filter((m: { name?: string }) => m?.name) | ||
| return valid.map((m: { name: string }) => m.name.toLowerCase()) | ||
| } | ||
| return generateOutputPaths(cfg.outputs || {}) | ||
| } | ||
|
|
||
| if (blk.type === 'starter') { | ||
| const startWorkflowValue = getSubBlockValue(blockId, 'startWorkflow') | ||
| if (startWorkflowValue === 'chat') { | ||
| return ['input', 'conversationId', 'files'] | ||
| } | ||
| const inputFormatValue = getSubBlockValue(blockId, 'inputFormat') | ||
| if (inputFormatValue && Array.isArray(inputFormatValue)) { | ||
| return inputFormatValue | ||
| .filter((f: { name?: string }) => f.name && f.name.trim() !== '') | ||
| .map((f: { name: string }) => f.name) | ||
| } | ||
| return [] | ||
| } | ||
|
|
||
| if (blk.triggerMode && cfg.triggers?.enabled) { | ||
| const triggerId = cfg?.triggers?.available?.[0] | ||
| const firstTrigger = triggerId ? getTrigger(triggerId) : getTriggersByProvider(blk.type)[0] | ||
| if (firstTrigger?.outputs) { | ||
| return generateOutputPaths(firstTrigger.outputs) | ||
| } | ||
| } | ||
|
|
||
| const operationValue = getSubBlockValue(blockId, 'operation') | ||
| if (operationValue && cfg?.tools?.config?.tool) { | ||
| try { | ||
| const toolId = cfg.tools.config.tool({ operation: operationValue }) | ||
| const toolConfig = toolId ? getTool(toolId) : null | ||
| if (toolConfig?.outputs) return generateOutputPaths(toolConfig.outputs) | ||
| } catch {} | ||
| } | ||
|
|
||
| return generateOutputPaths(cfg.outputs || {}) | ||
| } | ||
|
|
||
| const edges = currentWorkflow.edges || [] | ||
| const accessibleIds = new Set<string>( | ||
| BlockPathCalculator.findAllPathNodes(edges, focusedBlockId) | ||
| ) | ||
|
|
||
| // Always allow referencing the starter block | ||
| if (starterId && starterId !== focusedBlockId) accessibleIds.add(starterId) | ||
|
|
||
| const entries: Array<{ ref: string; value: any }> = [] | ||
|
|
||
| // Helper: collect executed outputs including virtual parallel iterations and loop/parallel context items | ||
| const collectExecutedOutputs = (baseId: string): Record<string, any>[] => { | ||
| const collected: Record<string, any>[] = [] | ||
| const bs = debugContext?.blockStates | ||
| if (bs) { | ||
| const direct = bs.get(baseId)?.output | ||
| if (direct && typeof direct === 'object') collected.push(direct) | ||
| // Include virtual executions for parallels | ||
| try { | ||
| for (const [key, state] of bs.entries()) { | ||
| const mapping = debugContext?.parallelBlockMapping?.get(key as any) | ||
| if (mapping && mapping.originalBlockId === baseId && state?.output) { | ||
| collected.push(state.output as any) | ||
| } | ||
| } | ||
| } catch {} | ||
| } | ||
| return collected | ||
| } | ||
|
|
||
| // Add loop/parallel special variables if block is inside a loop or parallel | ||
| const addLoopParallelVariables = () => { | ||
| if (!debugContext) return | ||
|
|
||
| // Check if focused block is inside a loop | ||
| for (const [loopId, loop] of Object.entries(currentWorkflow.loops || {})) { | ||
| if ((loop as any).nodes?.includes(focusedBlockId)) { | ||
| // Add loop.item and loop.index references | ||
| const loopItem = debugContext.loopItems?.get(loopId) | ||
| const loopIndex = debugContext.loopIterations?.get(loopId) | ||
| const loopItems = debugContext.loopItems?.get(`${loopId}_items`) | ||
|
|
||
| if (loopItem !== undefined) { | ||
| entries.push({ ref: '<loop.item>', value: loopItem }) | ||
| } | ||
| if (loopIndex !== undefined) { | ||
| entries.push({ ref: '<loop.index>', value: loopIndex }) | ||
| } | ||
| if (loopItems !== undefined) { | ||
| entries.push({ ref: '<loop.items>', value: loopItems }) | ||
| } | ||
|
|
||
| // Also add references for the loop block itself if it has executed | ||
| const loopBlock = blockById.get(loopId) | ||
| if (loopBlock) { | ||
| const loopName = normalizeBlockName(getDisplayName(loopBlock)) | ||
| if (loopItem !== undefined) { | ||
| entries.push({ ref: `<${loopName}.item>`, value: loopItem }) | ||
| } | ||
| if (loopIndex !== undefined) { | ||
| entries.push({ ref: `<${loopName}.index>`, value: loopIndex }) | ||
| } | ||
| if (loopItems !== undefined) { | ||
| entries.push({ ref: `<${loopName}.items>`, value: loopItems }) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Check if focused block is inside a parallel | ||
| for (const [parallelId, parallel] of Object.entries(currentWorkflow.parallels || {})) { | ||
| if ((parallel as any).nodes?.includes(focusedBlockId)) { | ||
| // Check for virtual block execution to get iteration info | ||
| const parallelState = debugContext.parallelExecutions?.get(parallelId) | ||
| if (parallelState) { | ||
| // Get current iteration context | ||
| const currentVirtualId = debugContext.currentVirtualBlockId | ||
| if (currentVirtualId) { | ||
| const mapping = debugContext.parallelBlockMapping?.get(currentVirtualId) | ||
| if (mapping) { | ||
| const iterationIndex = mapping.iterationIndex | ||
| const parallelItems = debugContext.loopItems?.get(`${parallelId}_items`) | ||
| const parallelItem = parallelItems | ||
| ? Array.isArray(parallelItems) | ||
| ? parallelItems[iterationIndex] | ||
| : Object.values(parallelItems)[iterationIndex] | ||
| : undefined | ||
|
|
||
| if (parallelItem !== undefined) { | ||
| entries.push({ ref: '<parallel.item>', value: parallelItem }) | ||
| } | ||
| entries.push({ ref: '<parallel.index>', value: iterationIndex }) | ||
| if (parallelItems !== undefined) { | ||
| entries.push({ ref: '<parallel.items>', value: parallelItems }) | ||
| } | ||
|
|
||
| // Also add references for the parallel block itself | ||
| const parallelBlock = blockById.get(parallelId) | ||
| if (parallelBlock) { | ||
| const parallelName = normalizeBlockName(getDisplayName(parallelBlock)) | ||
| if (parallelItem !== undefined) { | ||
| entries.push({ ref: `<${parallelName}.item>`, value: parallelItem }) | ||
| } | ||
| entries.push({ ref: `<${parallelName}.index>`, value: iterationIndex }) | ||
| if (parallelItems !== undefined) { | ||
| entries.push({ ref: `<${parallelName}.items>`, value: parallelItems }) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for (const id of accessibleIds) { | ||
| const blk = blockById.get(id) | ||
| if (!blk) continue | ||
|
|
||
| const allowedPathsSet = new Set<string>(getAccessiblePathsForBlock(id)) | ||
| if (allowedPathsSet.size === 0) continue | ||
|
|
||
| const displayName = getDisplayName(blk) | ||
| const normalizedName = normalizeBlockName(displayName) | ||
|
|
||
| // Gather executed outputs (direct and virtual) | ||
| const executedOutputs = collectExecutedOutputs(id) | ||
|
|
||
| // Flatten helper over multiple outputs with last-wins per path | ||
| const pathToValue = new Map<string, any>() | ||
| const flatten = (obj: any, prefix = ''): Array<{ path: string; value: any }> => { | ||
| if (obj == null || typeof obj !== 'object') return [] | ||
| const items: Array<{ path: string; value: any }> = [] | ||
| for (const [k, v] of Object.entries(obj)) { | ||
| const current = prefix ? `${prefix}.${k}` : k | ||
| if (v && typeof v === 'object' && !Array.isArray(v)) { | ||
| if (allowedPathsSet.has(current)) items.push({ path: current, value: v }) | ||
| items.push(...flatten(v, current)) | ||
| } else { | ||
| if (allowedPathsSet.has(current)) items.push({ path: current, value: v }) | ||
| } | ||
| } | ||
| return items | ||
| } | ||
|
|
||
| for (const out of executedOutputs) { | ||
| const pairs = flatten(out) | ||
| for (const { path, value } of pairs) { | ||
| pathToValue.set(path, value) | ||
| } | ||
| } | ||
|
|
||
| for (const [path, value] of pathToValue.entries()) { | ||
| entries.push({ ref: `<${normalizedName}.${path}>`, value }) | ||
| } | ||
| } | ||
|
|
||
| // Add loop/parallel context variables | ||
| addLoopParallelVariables() | ||
|
|
||
| // Sort for stable UI (by ref) | ||
| entries.sort((a, b) => a.ref.localeCompare(b.ref)) | ||
| return entries | ||
| }, [ | ||
| focusedBlockId, | ||
| currentWorkflow.edges, | ||
| currentWorkflow.loops, | ||
| currentWorkflow.parallels, | ||
| starterId, | ||
| blockById, | ||
| executionVersion, | ||
| debugContext, | ||
| ]) |
There was a problem hiding this comment.
style: outputVariableEntries memo is extremely large (261 lines) with multiple nested helper functions - this should be extracted into separate functions or a custom hook
Context Used: Context - If a switch statement is large and handles multiple cases, extract each case into separate functions for better maintainability. (link)
| const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)) | ||
| let attempts = 0 | ||
| while (attempts < 40) { | ||
| // ~2s max | ||
| const st = useExecutionStore.getState() | ||
| if (st.executor && st.debugContext && Array.isArray(st.pendingBlocks)) break | ||
| await wait(50) | ||
| attempts++ | ||
| } |
There was a problem hiding this comment.
style: polling loop with hardcoded timing (50ms intervals, 40 attempts) could be unreliable - consider using a more robust async pattern or make timing configurable
| } catch (e) { | ||
| // Swallow to avoid double error surfaces in UI | ||
| } |
There was a problem hiding this comment.
logic: silently swallowing all errors with empty catch block could hide important debugging information
| className='rounded bg-blue-50 px-1.5 py-0.5 font-mono text-[11px] text-blue-700 transition-colors hover:bg-blue-100 dark:bg-blue-900/20 dark:text-blue-400 dark:hover:bg-blue-900/30' | ||
| onClick={(e) => { | ||
| e.stopPropagation() | ||
| handleTokenClick(match.type as any, match.value, match.raw) |
There was a problem hiding this comment.
syntax: type assertion 'as any' bypasses TypeScript safety - consider proper typing
Context Used: Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
| openConsolePanel() | ||
| handleRunWorkflow(undefined, true) // Start in debug mode | ||
| // Activate debug session state so the panel is active | ||
| const execStore = useExecutionStore.getState() |
There was a problem hiding this comment.
style: Direct store access bypasses React state management. Consider using the existing execution hook methods instead of useExecutionStore.getState()
* Updates * Updates * Updates * Checkpoint * Checkpoint * Checkpoitn * Var improvements * Fixes * Execution status * UI improvements * Ui updates * Fix * Fix scoping * Fix workflow vars * Fix env vars * Remove number styling * Variable highlighting * Updates * Update * Fix resume * Stuff * Breakpoint ui * Ui * Ui updates * Loops and parallels * HIde env vars * Checkpoint * Stuff * Panel toggle * Lint
1 participant
Summary
Adds a proper debugger
Fixes #(issue)
Type of Change
Testing
Manual testing
Checklist