-
Notifications
You must be signed in to change notification settings - Fork 3.3k
improvement(tag-dropdown): removed custom styling on tag dropdown popover, fixed execution ordering in terminal and loops entries #3126
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
…pover, fixed execution ordering in terminal and loops entries
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile OverviewGreptile SummaryThis PR introduces a monotonically increasing Key changes:
Technical implementation:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Client/Frontend
participant API as API Route
participant Executor as Block Executor
participant Orchestrator as Loop/Parallel Orchestrator
participant Store as Console Store
participant Terminal as Terminal UI
Note over Executor: Initialize executionOrderCounter = 0
Executor->>Executor: createBlockLog and getNextExecutionOrder returns 1
Executor->>API: onBlockStart with executionOrder=1
API->>Client: SSE BlockStartedEvent executionOrder 1
Client->>Store: addConsole with executionOrder 1
Store->>Terminal: Display block with order 1
Executor->>Executor: Execute block
Executor->>API: onBlockComplete with executionOrder 1
API->>Client: SSE BlockCompletedEvent executionOrder 1
Client->>Store: updateConsole with executionOrder 1
Note over Orchestrator: Loop iteration 1 starts
Orchestrator->>Executor: Execute block in iteration 1
Executor->>Executor: getNextExecutionOrder returns 2
Executor->>API: onBlockStart executionOrder 2 iterationCurrent 1
API->>Client: SSE with executionOrder 2 and iterationCurrent 1
Client->>Store: addConsole executionOrder 2 iterationCurrent 1
Orchestrator->>Executor: Execute block in iteration 2
Executor->>Executor: getNextExecutionOrder returns 3
Executor->>API: onBlockStart executionOrder 3 iterationCurrent 2
API->>Client: SSE with executionOrder 3 and iterationCurrent 2
Client->>Store: addConsole executionOrder 3 iterationCurrent 2
Note over Store: Match by blockId AND iterationCurrent
Client->>Store: updateConsole for iterationCurrent 1
Store->>Store: Find entry matching blockId and iterationCurrent 1
Store->>Terminal: Update only iteration 1 entry
Terminal->>Terminal: buildEntryTree sorts by executionOrder
Terminal->>Terminal: Create synthetic parents using min executionOrder
Terminal->>Terminal: Display in correct execution order
|
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.
15 files reviewed, 2 comments
Additional Comments (2)
If the intent is to sort by Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/terminal/hooks/use-terminal-filters.ts
Line: 20:23
Comment:
[P1] Sort config field no longer affects sorting
`sortConfig.field` is initialized to `'timestamp'`, but `filterEntries()` now always sorts by `executionOrder` (ignoring the selected field). This makes the UI sort selector misleading and can break expectations when users switch sort fields.
If the intent is to sort by `executionOrder` only, consider removing `field` from `SortConfig`/UI; otherwise branch on `sortConfig.field` and only use `executionOrder` when appropriate.
How can I resolve this? If you propose a fix, please make it concise.
In This seems especially likely given Also appears in the error path nearby ( Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts
Line: 987:993
Comment:
[P2] `startedAt` is overwritten with completion-time value in console updates
In `onBlockCompleted`/`onBlockError` the `startedAt` variable is assigned from `data.startedAt`, and then passed into `updateConsole`. If `data.startedAt` is not the original start timestamp (or is omitted/incorrect), this will overwrite the entry’s start time in the store and skew duration/ordering display.
This seems especially likely given `addConsole` initially sets `startedAt = new Date().toISOString()` on block start, and later you’re updating it again on completion. If the server always provides the correct `startedAt`, it’s fine—otherwise consider only setting `startedAt` on the initial add, or only updating it when it was previously missing.
Also appears in the error path nearby (`onBlockError`).
How can I resolve this? If you propose a fix, please make it concise. |
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts
Outdated
Show resolved
Hide resolved
...app/workspace/[workspaceId]/w/[workflowId]/components/terminal/hooks/use-terminal-filters.ts
Outdated
Show resolved
Hide resolved
|
@greptile |
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.
5 files reviewed, no comments
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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Summary
Type of Change
Testing
Tested manually
Checklist