feat(workflow): connect workflow visualizer with live data#4153
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Code Review: Workflow Visualizer Live Data IntegrationSummaryThis PR successfully connects the workflow visualizer with live data from the inspector protocol. The implementation is well-structured with proper data transformation and React Query integration. ✅ StrengthsArchitecture & Design
Implementation Quality
🔍 Issues & Recommendations1. Potential Memory Leak in ActionsManager (Medium Priority)Location: The timeout in Recommendation: createResolver<T = void>(options?: {
timeoutMs?: number;
name?: string;
}): { id: number; promise: Promise<T> } {
const id = this.nextId++;
const { promise, resolve, reject } = Promise.withResolvers<T>();
this.suspensions.set(id, { promise, resolve, reject });
const timeoutMs = options?.timeoutMs ?? 2_000;
const timeoutId = setTimeout(() => {
if (this.suspensions.has(id)) {
reject(new Error(`Action timed out: ${options?.name ?? "unknown"}`));
this.suspensions.delete(id);
}
}, timeoutMs);
// Clear timeout when resolved
const wrappedResolve = (value: T) => {
clearTimeout(timeoutId);
resolve(value);
};
this.suspensions.set(id, { promise, resolve: wrappedResolve, reject });
return { id, promise };
}2. Silent Error Handling in decodeCborOrNull (Low Priority)Location: The function silently catches all decode errors. Consider logging for debugging. 3. Type Assertion Without Validation (Low Priority)Location: originalType: kind.val.originalType as EntryKindType,This assumes 4. Workflow State Derivation Logic (Low Priority)Location: The condition at line 216 could be clearer - it's checking if the workflow has started but not finished. 5. Fallback Value (Low Priority)Location: workflowId: entries[0]?.id ?? "unknown",Consider if "unknown" is appropriate - it might mask data issues. 🧪 Testing Recommendations
📝 Minor Observations
✨ Overall AssessmentQuality: High This is a solid implementation that successfully integrates live workflow data. Main concerns are around error handling edge cases and potential memory management. Code is production-ready with the minor improvements suggested above. Recommendation: ✅ Approve with minor suggestions 🎯 Action Items
|
Graphite Automations"Test" took an action on this PR • (02/06/26)1 assignee was added to this PR based on Kacper Wojciechowski's automation. |

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: