-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat(executor): run from/until block #3029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
icecrasher321
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested all edge cases
Greptile OverviewGreptile SummaryImplemented run-from-block and run-until-block execution features that allow users to execute workflows starting from a specific block or stopping after a target block. The implementation includes comprehensive validation, snapshot management, and UI integration. Key Changes:
Implementation Approach: Validation:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant UI as Block Menu
participant Hook as useWorkflowExecution
participant Stream as useExecutionStream
participant API as execute-from-block API
participant Core as execution-core
participant Executor as DAGExecutor
participant Engine as ExecutionEngine
participant Store as ExecutionStore
User->>UI: Right-click block, select "Run from block"
UI->>Hook: handleRunFromBlock(blockId)
Hook->>Store: getLastExecutionSnapshot(workflowId)
Store-->>Hook: Return snapshot
alt No snapshot or dependencies not satisfied
Hook-->>User: Show error notification
else Valid snapshot
Hook->>Stream: executeFromBlock(workflowId, startBlockId, snapshot)
Stream->>API: POST /api/workflows/{id}/execute-from-block
API->>API: Validate auth & request body
API->>Core: executeWorkflowCore(runFromBlock options)
Core->>Executor: executeFromBlock(workflowId, startBlockId, snapshot)
Executor->>Executor: validateRunFromBlock()
Executor->>Executor: computeExecutionSets()
Note over Executor: Compute dirty set (blocks to re-execute)<br/>and upstream set (cached blocks)
Executor->>Executor: Filter snapshot to reachable upstream
Executor->>Executor: Remove incoming edges from non-dirty sources
Executor->>Engine: run()
Engine->>Engine: initializeQueue(startBlockId)
loop Until no work remaining
Engine->>Engine: executeNodeAsync(nodeId)
alt Block in dirty set
Engine->>Engine: Re-execute block
else Block not in dirty set
Engine->>Engine: Use cached output
end
Engine->>API: Send SSE events (block:started, block:completed)
API->>Stream: Stream SSE events
Stream->>Hook: Trigger callbacks
Hook->>Store: Update execution state
Hook->>UI: Update active blocks visualization
end
Engine-->>Executor: Return ExecutionResult
Executor-->>Core: Return result
Core->>API: Send execution:completed event
API->>Stream: Stream final result
Stream->>Hook: onExecutionCompleted
Hook->>Store: setLastExecutionSnapshot() (merge with existing)
Hook->>UI: Update UI state
UI-->>User: Show execution complete
end
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 files reviewed, 3 comments
|
|
||
| // Filter snapshot to include all blocks reachable from dirty blocks | ||
| // This preserves sibling branch outputs that dirty blocks may reference | ||
| const filteredBlockStates: Record<string, any> = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type mismatch - filteredBlockStates is Record<string, any> but should match the type expected by blockStates field
| const filteredBlockStates: Record<string, any> = {} | |
| const filteredBlockStates: Record<string, BlockState> = {} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/executor/execution/executor.ts
Line: 144:144
Comment:
type mismatch - `filteredBlockStates` is `Record<string, any>` but should match the type expected by `blockStates` field
```suggestion
const filteredBlockStates: Record<string, BlockState> = {}
```
How can I resolve this? If you propose a fix, please make it concise.| isDeployedContext: this.contextExtensions.isDeployedContext, | ||
| blockStates: state.getBlockStates(), | ||
| blockLogs: snapshotState?.blockLogs ?? [], | ||
| blockLogs: overrides?.runFromBlockContext ? [] : (snapshotState?.blockLogs ?? []), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when runFromBlockContext exists, blockLogs are reset to empty array, but this discards upstream execution logs that may be needed for debugging and tracing
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/executor/execution/executor.ts
Line: 268:268
Comment:
when `runFromBlockContext` exists, `blockLogs` are reset to empty array, but this discards upstream execution logs that may be needed for debugging and tracing
How can I resolve this? If you propose a fix, please make it concise.| const mergedBlockStates: Record<string, BlockState> = { | ||
| ...effectiveSnapshot.blockStates, | ||
| } | ||
| for (const [bId, state] of accumulatedBlockStates) { | ||
| mergedBlockStates[bId] = state | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merging logic doesn't preserve loop/parallel execution state from the snapshot - only blockStates are merged but loopExecutions and parallelExecutions from effectiveSnapshot should also be included in updatedSnapshot
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts
Line: 1656:1661
Comment:
merging logic doesn't preserve loop/parallel execution state from the snapshot - only `blockStates` are merged but `loopExecutions` and `parallelExecutions` from `effectiveSnapshot` should also be included in `updatedSnapshot`
How can I resolve this? If you propose a fix, please make it concise.
Summary
Adds run from block and run until block functionality to the executor
Type of Change
Testing
Tested with @icecrasher321
Checklist