perf(ui): localize monitor slow-input edge alerts - #680
Conversation
Signed-off-by: streamkit-devin <devin@streamkit.dev>
Signed-off-by: streamkit-devin <devin@streamkit.dev>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Signed-off-by: streamkit-devin <devin@streamkit.dev>
| const hasPassthroughOutput = | ||
| !isInput && pins.some((pin) => (pin as OutputPin).produces_type === 'Passthrough'); | ||
| const resolvedPassthroughType = useStore( | ||
| React.useCallback( | ||
| (state: ReactFlowState): PacketType | null => | ||
| hasPassthroughOutput ? findUpstreamOutputType(nodeId, state.nodeLookup, state.edges) : null, | ||
| [hasPassthroughOutput, nodeId] | ||
| ) | ||
| ); |
There was a problem hiding this comment.
📝 Info: PinRow subscription narrowing stays behavior-equivalent
The rewrite replaces the full edges/nodeLookup subscription and per-pin map with one useStore selector returning the resolved passthrough type. findUpstreamOutputType always resolves from the first input pin regardless of output pin, so the old per-pin loop produced the same value for every passthrough pin; collapsing to one value is equivalent. The primitive return lets Object.is skip re-renders on unrelated store updates.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| export function useSlowInputAlert( | ||
| edge: AlertEdge, | ||
| monitorAlertContext: MonitorEdgeAlertContext | undefined | ||
| ): TypedEdgeAlert | null { | ||
| const sessionId = monitorAlertContext?.sessionId; | ||
| const targetHandle = edge.targetHandle ?? ''; | ||
| const detailsAtom = React.useMemo<SlowInputDetailsAtom>(() => { | ||
| if (!sessionId) return nullSlowInputDetailsAtom; | ||
| return selectAtom( | ||
| nodeStateAtom(nodeKey(sessionId, edge.target)), | ||
| (state) => { | ||
| const details = extractSlowTimeoutDetailsFromNodeState(state); | ||
| if (!details || !details.slowPins.includes(targetHandle)) return null; | ||
| return details; | ||
| }, | ||
| deepEqual | ||
| ); | ||
| }, [edge.target, sessionId, targetHandle]); | ||
| const details = useAtomValue(detailsAtom); | ||
| return monitorAlertContext | ||
| ? buildSlowInputAlert(edge, details, monitorAlertContext.connections) | ||
| : null; | ||
| } |
There was a problem hiding this comment.
📝 Info: Edge alert computed per-edge via Jotai selector
useSlowInputAlert subscribes each edge to a selectAtom over the target node's state, gated to the edge's target pin with deepEqual equality. It reads the default store, matching how node components read state, and no Jotai Provider shadows it. The gate and tooltip lines match the removed subscription hook.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #680 +/- ##
==========================================
- Coverage 85.33% 85.27% -0.07%
==========================================
Files 249 251 +2
Lines 76190 76246 +56
Branches 2329 2485 +156
==========================================
- Hits 65019 65016 -3
- Misses 11165 11224 +59
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Signed-off-by: streamkit-devin <devin@streamkit.dev>
| const resolvedPassthroughType = useStore( | ||
| React.useCallback( | ||
| (state: ReactFlowState): PacketType | null => | ||
| hasPassthroughOutput ? findUpstreamOutputType(nodeId, state.nodeLookup, state.edges) : null, | ||
| [hasPassthroughOutput, nodeId] | ||
| ) | ||
| ); |
There was a problem hiding this comment.
📝 Info: PinRow render suppression depends on stable PacketType refs
The useStore selector reruns on every React Flow store change and returns produces_type from the upstream node. For object-valued PacketType, re-render suppression relies on Object.is seeing a stable nested reference, which holds only while node data objects are preserved between topology rebuilds (as pickStableCanvasNodes ensures).
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Signed-off-by: streamkit-devin <devin@streamkit.dev>
| (state) => { | ||
| const details = extractSlowTimeoutDetailsFromNodeState(state); | ||
| if (!details || !details.slowPins.includes(targetHandle)) return null; | ||
| return details; | ||
| }, | ||
| deepEqual | ||
| ); | ||
| }, [edge.target, sessionId, targetHandle]); | ||
| const details = useAtomValue(detailsAtom); |
There was a problem hiding this comment.
📝 Info: Dropped apiNode.state fallback is safe
The removed fallback (old code read the atom then apiNode.state) is preserved because seedPipelineAtoms runs alongside every setPipeline, seeding node state into the atoms that useSlowInputAlert reads. setPipeline has no other production caller, so no seeding window is missed.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Signed-off-by: streamkit-devin <devin@streamkit.dev>
Signed-off-by: streamkit-devin <devin@streamkit.dev>
| const resolvedPassthroughType = useStore( | ||
| React.useCallback( | ||
| (state: ReactFlowState): PacketType | null => | ||
| hasPassthroughOutput ? findUpstreamOutputType(nodeId, state.nodeLookup, state.edges) : null, | ||
| [hasPassthroughOutput, nodeId] | ||
| ) | ||
| ); |
There was a problem hiding this comment.
📝 Info: Passthrough selector recomputes on every store event
For passthrough-output rows, the useStore selector runs findUpstreamOutputType on every store change including pan/zoom and drags, versus the old useMemo that recomputed only on edge/node reference changes. It returns a primitive, so no extra render happens and correctness holds.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| const reusePreviousPositions = topologySessionIdRef.current === selectedSessionId; | ||
| topologySessionIdRef.current = selectedSessionId; |
There was a problem hiding this comment.
📝 Info: Cross-session position reuse holds even with a transient null pipeline
reusePreviousPositions compares topologySessionIdRef to the selected session. On a switch where the new pipeline is momentarily undefined, the clear pass empties the node list, so the later rebuild reads empty previous positions and falls back to the target session's saved positions. Live positions from the prior session are never reused.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| export const describeSlowInputs = ( | ||
| pipeline: Pipeline, | ||
| nodeId: string, | ||
| slowPins: string[] | ||
| ): string[] => describeSlowInputsFromConnections(pipeline.connections, nodeId, slowPins); |
There was a problem hiding this comment.
🔍 describeSlowInputs now referenced only by tests
After the subscription hook was deleted, describeSlowInputs is used only in test code; production now calls describeSlowInputsFromConnections. Confirm the unused-code gate treats test-only usage as used so the export is not flagged.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Confirmed the repository’s unused-code gate counts this test usage: bun run knip passes at the current head with describeSlowInputs referenced by pipelineGraph.test.ts. No production dead-code failure is present, so I’m leaving the compatibility wrapper unchanged.
Summary
edgesrewrites with an exact(session, target node, target pin)Jotai selector, soslow_input_timeouttransitions rerender only the affected edge.PinRow’s React Flow subscription to its resolved passthrough packet type, preventing metadata-only graph updates from invalidating every pin row.Review & Validation
⏱️only on the matching target pin and clears on recovery.MonitorViewor unrelatedPinRow/edge components.Notes
Focused Monitor tests, UI lint/typecheck, Knip, formatting, REUSE, and render-performance assertions pass locally. Browser profiling was not rerun because it was not requested; CI is the aggregate verification gate.
Link to Devin session: https://staging.itsdev.in/sessions/738c0703efe441cb80769d070c318be6
Open in Devin Desktop: https://staging.itsdev.in/desktop/session/738c0703efe441cb80769d070c318be6?variant=devin-insiders
Requested by: @streamer45
Devin Review
7da53d7