Skip to content

perf(ui): localize monitor slow-input edge alerts - #680

Merged
streamer45 merged 7 commits into
mainfrom
devin/1787500683-branch-name
Aug 23, 2026
Merged

perf(ui): localize monitor slow-input edge alerts#680
streamer45 merged 7 commits into
mainfrom
devin/1787500683-branch-name

Conversation

@staging-devin-ai-integration

@staging-devin-ai-integration staging-devin-ai-integration Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace Monitor’s all-node atom subscription and controlled edges rewrites with an exact (session, target node, target pin) Jotai selector, so slow_input_timeout transitions rerender only the affected edge.
  • Narrow PinRow’s React Flow subscription to its resolved passthrough packet type, preventing metadata-only graph updates from invalidating every pin row.
  • Scope topology rebuilds and saved node positions to the selected session, so identical graphs rebind state and restore the correct layout after a session switch while same-session topology changes preserve live drag positions.
  • Preserve the existing timer badge and tooltip details, with regression coverage for render containment, alert recovery, and cross-session topology/layout isolation.

Review & Validation

  • Confirm a mixer slow-input transition shows ⏱️ only on the matching target pin and clears on recovery.
  • Switch between identical-topology sessions and verify node state, alerts, and saved positions follow the selected session.
  • Profile a mixing pipeline and verify the transition no longer rerenders MonitorView or unrelated PinRow/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

Status Commit
🟢 Reviewed 7da53d7
Devin Review (Staging)

Signed-off-by: streamkit-devin <devin@streamkit.dev>
Signed-off-by: streamkit-devin <devin@streamkit.dev>
@staging-devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

staging-devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: streamkit-devin <devin@streamkit.dev>

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 2 new potential issues.

Devin Review (Staging)
Debug

Playground

Comment on lines +68 to +76
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]
)
);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 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.

Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Comment on lines +89 to +111
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;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 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.

Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

@codecov

codecov Bot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.27%. Comparing base (717444b) to head (7da53d7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
ui/src/components/TypedEdge.tsx 82.85% 6 Missing ⚠️
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              
Flag Coverage Δ
backend 85.36% <ø> (-0.02%) ⬇️
ui 84.40% <90.00%> (-0.55%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
core 85.79% <ø> (ø)
engine 83.76% <ø> (ø)
api 91.14% <ø> (ø)
nodes 84.96% <ø> (-0.04%) ⬇️
server 85.26% <ø> (ø)
plugin-native 84.79% <ø> (ø)
plugin-wasm 95.41% <ø> (ø)
ui-services 86.29% <ø> (-0.25%) ⬇️
ui-components 68.94% <86.04%> (-1.24%) ⬇️
Files with missing lines Coverage Δ
ui/src/components/node/PinRow.tsx 87.03% <100.00%> (ø)
ui/src/utils/monitorTopology.ts 100.00% <100.00%> (ø)
ui/src/utils/pipelineGraph.ts 100.00% <100.00%> (ø)
ui/src/components/TypedEdge.tsx 41.09% <82.85%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: streamkit-devin <devin@streamkit.dev>

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 1 new potential issue.

Devin Review (Staging)
Debug

Playground

Comment on lines +70 to +76
const resolvedPassthroughType = useStore(
React.useCallback(
(state: ReactFlowState): PacketType | null =>
hasPassthroughOutput ? findUpstreamOutputType(nodeId, state.nodeLookup, state.edges) : null,
[hasPassthroughOutput, nodeId]
)
);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 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).

Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Signed-off-by: streamkit-devin <devin@streamkit.dev>

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 2 new potential issues.

Devin Review (Staging)
Debug

Playground

Comment thread ui/src/views/MonitorView.tsx
Comment on lines +99 to +107
(state) => {
const details = extractSlowTimeoutDetailsFromNodeState(state);
if (!details || !details.slowPins.includes(targetHandle)) return null;
return details;
},
deepEqual
);
}, [edge.target, sessionId, targetHandle]);
const details = useAtomValue(detailsAtom);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 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.

Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Signed-off-by: streamkit-devin <devin@streamkit.dev>
Signed-off-by: streamkit-devin <devin@streamkit.dev>

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 new potential issues.

Devin Review (Staging)
Debug

Playground

Comment on lines +70 to +76
const resolvedPassthroughType = useStore(
React.useCallback(
(state: ReactFlowState): PacketType | null =>
hasPassthroughOutput ? findUpstreamOutputType(nodeId, state.nodeLookup, state.edges) : null,
[hasPassthroughOutput, nodeId]
)
);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 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.

Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Comment on lines +938 to +939
const reusePreviousPositions = topologySessionIdRef.current === selectedSessionId;
topologySessionIdRef.current = selectedSessionId;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 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.

Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Comment on lines +75 to +79
export const describeSlowInputs = (
pipeline: Pipeline,
nodeId: string,
slowPins: string[]
): string[] => describeSlowInputsFromConnections(pipeline.connections, nodeId, slowPins);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 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.

Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@streamer45
streamer45 merged commit a592a15 into main Aug 23, 2026
26 checks passed
@streamer45
streamer45 deleted the devin/1787500683-branch-name branch August 23, 2026 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants