fix: flow-run-graph-attempt-filter#22388
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70d74b7430
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ...data, | ||
| root_node_ids: rootNodeIds, | ||
| start_time: startTime, | ||
| end_time: endTime, |
There was a problem hiding this comment.
Preserve null end_time for active retried runs
When a flow run is still active but the selected attempt currently has only terminal or pending nodes (for example during an awaiting-retry delay after attempt 1 failed and before attempt 2 starts), getRunTimeRange returns a finite endTime and this overwrites the API's null graph end_time. The polling loop in ui-v2/src/graphs/factories/data.ts only refetches when returned data.end_time is null, so the graph stops polling and never shows later retry attempts. Keep the original null end time for active flow runs, or otherwise keep signaling that polling should continue, while still using the filtered range for rendering.
Useful? React with 👍 / 👎.
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d75503823
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const nodes = new Map<string, RunGraphNode>(); | ||
|
|
||
| for (const [nodeId, node] of data.nodes) { | ||
| if (node.flow_run_run_count === runCount) { |
There was a problem hiding this comment.
Keep pending downstream nodes in attempt filters
When a task never reaches RUNNING because an upstream failed, its flow_run_run_count stays at the DB/default value 0 (the task engine and UpdateFlowRunTrackerOnTasks only populate it when entering RUNNING). This strict equality check means those NotReady/PENDING downstream placeholders are classified as a bogus Run 0 and are omitted when the user filters to the actual parent flow attempt, so retried graphs lose pending downstream nodes. Normalize the attempt count for non-running task runs before filtering or avoid treating 0 as a separate selectable attempt.
Useful? React with 👍 / 👎.
| theme: resolvedTheme === "dark" ? "dark" : "light", | ||
| }), | ||
| [flowRunId, resolvedTheme], | ||
| [fetchGraph, flowRunId, resolvedTheme], |
There was a problem hiding this comment.
Avoid restarting the graph on attempt changes
Including fetchGraph here makes config change whenever selectedAttemptRunCount changes, so selecting a different attempt tears the graph down via the [config] start effect. stop() calls emitter.clear(), but the React effect that registers itemSelected/viewportDateRangeUpdated listeners does not rerun for an attempt-only change, leaving the wrapper unsubscribed; after switching attempts, node clicks no longer open the selection panel and viewport callbacks stop firing. Keep the graph instance mounted for filter changes or make the emitter subscriptions re-register after this restart.
Useful? React with 👍 / 👎.
|
I noticed a potential issue with
Consider preserving the original |
|
Two small cleanup items:
|
desertaxle
left a comment
There was a problem hiding this comment.
@hemantmm the review comments posted by Devin and Codex will need to be addressed before this PR can be merged.
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
closes: #22220
Added a flow run attempt dropdown to the flow run graph so that it can be retried, runs can be viewed one attempt at a time, using the task run flow_run_run_count to filter nodes and rescale the graph timeline.