Draft
Conversation
leszko
force-pushed
the
rafal/graph-mode
branch
from
057bcd7 to
d415162
Compare
gioelecerati
force-pushed
the
rafal/graph-mode
branch
3 times, most recently
from
dbe40cc to
afc94d0
Compare
Contributor
🚀 fal.ai Preview Deployment
TestingConnect to this preview deployment by running this on your branch: 🧪 E2E tests will run automatically against this deployment. |
Contributor
❌ E2E Tests failed
Test ArtifactsCheck the workflow run for screenshots, traces, and failure details. |
Signed-off-by: Rafał Leszko <rafal@livepeer.org> Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: Rafał Leszko <rafal@livepeer.org> Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: Rafał Leszko <rafal@livepeer.org> Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
Signed-off-by: gioelecerati <gioele@cerati.org>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (6)
frontend/src/components/graph/ui/NodePillListInput.tsx (1)
23-49:⚠️ Potential issue | 🟠 MajorDon't commit partial comma-separated input.
1,currently propagates[1], and the sync effect then rewrites the field back to1, so multi-value entry by typing doesn't work. Only callonChangeonce the whole token list is complete and every token parses successfully.Possible fix
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { const text = e.target.value; setInputValue(text); - const parts = text - .split(",") - .map(s => s.trim()) - .filter(s => s); - const numbers = parts - .map(s => { - const num = Number(s); - return Number.isNaN(num) ? null : num; - }) - .filter((n): n is number => n !== null); - - if (numbers.length > 0) { - onChange(numbers); - } else if (text === "") { + const rawParts = text.split(","); + const trimmedParts = rawParts.map(part => part.trim()); + + if (text.trim() === "") { onChange([]); + return; + } + + if (trimmedParts.some(part => part === "")) { + return; + } + + const numbers = trimmedParts.map(Number); + if (numbers.every(num => !Number.isNaN(num))) { + onChange(numbers); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/ui/NodePillListInput.tsx` around lines 23 - 49, handleChange currently calls onChange as soon as any token parses (so typing "1," propagates [1]) which breaks incremental multi-value entry; change the logic in handleChange (keep setInputValue and the useEffect as-is) so that you only call onChange([]) when text === "" and otherwise only call onChange(numbers) when parts.length > 0 AND numbers.length === parts.length (i.e. every token parsed successfully); if some tokens are incomplete/invalid, do not call onChange until they all parse.frontend/src/pages/StreamPage.tsx (2)
1884-1904:⚠️ Potential issue | 🟠 MajorClear
settings.inputSourcewhen the graph switches back tovideoorcamera.The local-source branch only calls
switchMode(sourceMode), so a previous Spout/NDI/Syphon selection remains enabled in settings and gets sent on the next perform-mode start.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/StreamPage.tsx` around lines 1884 - 1904, When switching away from server-side sources, clear the persisted inputSource so leftover Spout/NDI/Syphon settings aren't reused: in the branch where sourceMode is not "spout"/"ndi"/"syphon" (the else that currently only calls switchMode(sourceMode)), call updateSettings to reset settings.inputSource (e.g., set enabled: false and clear source_type/source_name) before calling switchMode; use the same symbols shown (sourceMode, updateSettings, settings.inputSource, switchMode, sourceNode) so reviewers can locate and apply the change.
1615-1634:⚠️ Potential issue | 🔴 CriticalGraph-mode start still falls back to perform-mode source state.
isServerSideInputand the fallbacksettings.inputSourcebranch are still derived frommode/settings.inputSource. A persisted NDI/Spout/Syphon graph can therefore wrongly require a local stream, and a local graph can inherit a stale server-side source. Once the graph source is resolved, use that normalized source info as the only truth while graph execution is active.Also applies to: 1738-1743
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/StreamPage.tsx` around lines 1615 - 1634, The code currently derives isServerSideInput and stream requirement from mode/settings.inputSource which can be stale for running graphs; change the logic to use the graph's resolved/normalized source object (e.g., a resolvedGraphSource or graph.inputSource) as the single source of truth while the graph is active: compute isServerSideInput, needsVideoInput (instead of reading mode/currentMode/settings.inputSource) from that normalized source, then set streamToSend and the localStream existence check based on that resolved source; update the same pattern used later (around the streamToSend/validation at the other block around 1738-1743) to reference the normalized graph source rather than mode/settings.inputSource so persisted NDI/Spout/Syphon graphs don't incorrectly require or inherit local streams.frontend/src/components/graph/hooks/useGraphState.ts (2)
575-583:⚠️ Potential issue | 🟠 MajorPersist
nodeParamsthroughui_state, and reset or hydrate it with graph loads.All save/export paths serialize only
flowToGraphConfig(nodes, edges), while load/import/clear never read or clear the separatenodeParamsstate. Prompt text and per-node params disappear after reload and can leak into a different graph that reuses the same node IDs.GraphConfig.ui_statealready exists for opaque frontend state and fits this data.Also applies to: 599-607, 646-654, 678-684, 709-710, 722-729, 752-758, 774-775
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/hooks/useGraphState.ts` around lines 575 - 583, The graph load/reset paths (where graphConfig is converted via graphConfigToFlow and nodes/edges set using enrichNodes, setNodes, setEdges, colorEdges, setGraphSource) do not persist or hydrate per-node nodeParams; move nodeParams into GraphConfig.ui_state during saves (use flowToGraphConfig to include nodeParams into ui_state) and on loads/import/clear read ui_state to set or clear the local nodeParams state before calling enrichNodes and setNodes so prompt text and per-node params are restored or reset and cannot leak across graphs; update all corresponding load spots (the other instances you flagged) to extract nodeParams from graphConfig.ui_state and apply/clear them consistently.
634-639:⚠️ Potential issue | 🟠 MajorDon't treat loads, refreshes, or enrichment-only updates as user edits.
This effect fires right after
loadGraphFromBackend()andrefreshGraph(), and it also fires after the enrichmentsetNodes()passes when stream or availability props change. InStreamPage, that flipsnonLinearGraphtotrueeven when the user never mutated the graph.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/hooks/useGraphState.ts` around lines 634 - 639, The effect currently treats any nodes/edges change as a user edit and calls onGraphChangeRef, causing non-user actions (loadGraphFromBackend, refreshGraph, enrichment via setNodes) to flip StreamPage state; add a suppression/ref flag to distinguish programmatic updates: introduce a ref like suppressGraphChangeRef (or lastUpdateSource enum) checked in the useEffect that returns early if set, and set that ref to true around programmatic mutations inside loadGraphFromBackend, refreshGraph, and enrichment code paths (clear it after the update completes) so only genuine user-initiated edits trigger onGraphChangeRef.current().src/scope/server/graph_executor.py (1)
156-163:⚠️ Potential issue | 🟠 MajorWire throttling for every video edge.
The
breakstill stops after the first video consumer, so multi-stage or branched graphs lose backpressure on the remaining paths and can still overfill downstream queues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scope/server/graph_executor.py` around lines 156 - 163, The loop over graph.edges currently breaks after wiring backpressure for the first video consumer; remove the `break` so every stream edge with from_port == "video" wires backpressure. Specifically, in the block iterating `for e in graph.edges` (checking `e.kind != "stream"` and `e.from_port != "video"`), call `producer.throttler.set_next_processor(consumer)` for each edge where `producer = node_processors.get(e.from_node)` and `consumer = node_processors.get(e.to_node)` are not None, and do not exit the loop early so multi-stage/branched graphs get throttling on all video paths. Ensure null-checks for `producer`/`consumer` remain as-is.
🧹 Nitpick comments (18)
frontend/src/components/graph/ContextMenu.tsx (1)
54-67: Consider using a stable key instead of array index.Using
indexas the key works for static menus but could cause issues if items are dynamically reordered. If the menu items are always static per render, this is acceptable.♻️ Optional: Use item.label as key if labels are unique
- {items.map((item, index) => ( - <button - key={index} + {items.map((item) => ( + <button + key={item.label}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/ContextMenu.tsx` around lines 54 - 67, The ContextMenu's items.map currently uses the array index as the React key (key={index}) which can cause reconciliation bugs if items change order; update the mapping in ContextMenu.tsx to use a stable unique identifier from each item instead (e.g., key={item.id} or key={item.label} if labels are guaranteed unique) and fall back to index only if no stable id/label exists—ensure you reference the items.map callback and the button element when making this change.frontend/src/components/graph/nodes/MathNode.tsx (2)
227-242: Consider adding a dependency array touseLayoutEffect.The
useLayoutEffectruns on every render due to the missing dependency array. While this is sometimes intentional for DOM measurements, it may cause unnecessary recalculations. If the row positions only need updating when specific data changes, consider adding appropriate dependencies.♻️ Add dependencies if measurements only need updating on specific changes
useLayoutEffect(() => { const positions: Record<string, number> = {}; for (const [key, el] of rowRefs.current.entries()) { positions[key] = el.offsetTop + el.offsetHeight / 2; } setRowPositions(prev => { const keys = Object.keys(positions); if ( keys.length === Object.keys(prev).length && keys.every(k => Math.abs((prev[k] ?? 0) - positions[k]) < 1) ) { return prev; } return positions; }); - }); + }, [operation]); // Re-measure when operation changes (affects B row visibility)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/nodes/MathNode.tsx` around lines 227 - 242, The useLayoutEffect that measures DOM via rowRefs and updates state via setRowPositions currently has no dependency array and therefore runs on every render; restrict it by adding a dependency array to the effect (for example including rowRefs.current, any props/state that affect row elements like the data/rows prop, and the setter setRowPositions) so the measurement only re-runs when those inputs change—update the useLayoutEffect invocation in MathNode.tsx (the effect that reads rowRefs.current, computes positions, and calls setRowPositions) to include the appropriate dependencies.
185-198: Potential render loop fromsetNodesinuseEffect.Calling
setNodesinsideuseEffectwithresultas a dependency can cause cascading updates if multiple MathNodes are connected in sequence, as each update propagates through the graph. The current implementation should work but may benefit from batching or debouncing in complex graphs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/nodes/MathNode.tsx` around lines 185 - 198, The useEffect in MathNode.tsx that calls setNodes (using id, setNodes, result) can trigger cascading renders; change it so you only call setNodes when the node's currentValue would actually change: read the node by id (or compare against a lastResult ref) and return early if result === existing currentValue to avoid no-op updates, or debounce/batch updates (e.g., via a useRef timer) when results change rapidly; ensure the update still uses the functional updater (nds => nds.map(...)) and only maps/returns a new node object when the value differs to prevent unnecessary re-renders.frontend/src/components/graph/nodes/TupleNode.tsx (1)
189-209: Pointer event listeners may leak if component unmounts during drag.If the TupleNode unmounts while a slider drag is in progress, the
pointermoveandpointeruplisteners remain attached to the detached DOM element. Consider using a ref-based cleanup or moving to React pointer event props.♻️ Possible fix using cleanup on unmount
+import { useCallback, useEffect, useRef } from "react"; + export function TupleNode({ id, data, selected }: NodeProps<TupleNodeType>) { const { setNodes } = useReactFlow(); + const cleanupRef = useRef<(() => void) | null>(null); + + useEffect(() => { + return () => { + cleanupRef.current?.(); + }; + }, []); // ... in the onPointerDown handler: onPointerDown={e => { e.preventDefault(); e.stopPropagation(); const target = e.currentTarget as HTMLElement; target.setPointerCapture(e.pointerId); const setFromMouse = (clientX: number) => { const rect = target.getBoundingClientRect(); let ratio = (clientX - rect.left) / rect.width; ratio = Math.min(Math.max(ratio, 0), 1); setValueAt(i, tMin + ratio * rangeSize); }; setFromMouse(e.clientX); const onMove = (ev: PointerEvent) => setFromMouse(ev.clientX); const onUp = () => { target.removeEventListener("pointermove", onMove); target.removeEventListener("pointerup", onUp); + cleanupRef.current = null; }; + cleanupRef.current = onUp; target.addEventListener("pointermove", onMove); target.addEventListener("pointerup", onUp); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/nodes/TupleNode.tsx` around lines 189 - 209, The onPointerDown handler in TupleNode.tsx attaches pointermove/pointerup listeners to the DOM target (variable target) but doesn't remove them if the component unmounts during a drag; update the implementation to track the active pointer target (e.g., a ref like activePointerTarget) and the bound handlers (onMove/onUp), register them as now but also remove them and call releasePointerCapture in a useEffect cleanup that runs on unmount (or attach listeners to window and remove on unmount), ensuring target.removeEventListener("pointermove", onMove) and target.removeEventListener("pointerup", onUp) are invoked for the same handler references used when adding; keep setFromMouse and setValueAt usage the same but ensure handlers are stable or stored so cleanup can remove them reliably.frontend/src/components/graph/ui/NodePillInput.tsx (2)
32-41: Manual input bypasses min/max constraints.When the user types a value directly,
handleChangedoesn't clamp it within themin/maxbounds. The browser's native validation only prevents form submission but doesn't enforce bounds on the state. Consider clamping or validating on blur.♻️ Proposed fix to clamp on change or blur
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { if (type === "number") { const numValue = Number(e.target.value); if (!Number.isNaN(numValue)) { - onChange(numValue); + onChange(clampValue(numValue)); } } else { onChange(e.target.value); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/ui/NodePillInput.tsx` around lines 32 - 41, handleChange currently converts and forwards typed numbers without enforcing min/max, so manual input can violate bounds; update handleChange (and/or add a complementary onBlur handler) to clamp numeric values to the provided min and max props before calling onChange (use Number(e.target.value) and ensure Number.isFinite), referencing the existing handleChange, the type prop, and onChange callback; if you add an onBlur, name it handleBlur and ensure it also clamps and calls onChange so pasted or partially-entered values are normalized when the input loses focus.
53-98: Consider cleanup on unmount to prevent memory leaks.If the component unmounts while a drag is in progress (e.g., node deletion mid-drag), the
mousemoveandmouseuplisteners remain attached todocument. Add cleanup viauseEffectreturn.♻️ Proposed fix for cleanup
+ const cleanupRef = useRef<(() => void) | null>(null); + + useEffect(() => { + return () => { + cleanupRef.current?.(); + }; + }, []); + const handleMouseDown = useCallback( (e: React.MouseEvent) => { // ... existing code ... const onUp = () => { document.removeEventListener("mousemove", onMove); document.removeEventListener("mouseup", onUp); + cleanupRef.current = null; if (!dragRef.current?.hasDragged) { inputRef.current?.focus(); inputRef.current?.select(); } dragRef.current = null; }; + cleanupRef.current = onUp; document.addEventListener("mousemove", onMove); document.addEventListener("mouseup", onUp); }, [type, disabled, value, min, max, clampValue, onChange] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/ui/NodePillInput.tsx` around lines 53 - 98, The document-level mousemove/mouseup listeners added inside handleMouseDown can leak if the component unmounts during a drag; add a useEffect that returns a cleanup function which removes those listeners and clears dragRef and that cleanup should reference the same onMove/onUp handlers (or guard via stable refs) so they can be removed correctly; specifically update NodePillInput to add a useEffect that removes document.removeEventListener("mousemove", onMove) and document.removeEventListener("mouseup", onUp) on unmount and resets dragRef.current = null, ensuring onMove/onUp are stable or stored in refs so removal works.frontend/src/components/graph/nodes/NoteNode.tsx (1)
27-29: Minor redundancy:onWheelhandler withnowheelclass.The
nowheelclass already tells React Flow to ignore wheel events on this element. ThehandleWheelwithstopPropagationis redundant but harmless. Consider removing one for clarity.Also applies to: 50-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/nodes/NoteNode.tsx` around lines 27 - 29, The onWheel handler handleWheel (which only calls e.stopPropagation()) is redundant because the element already uses the "nowheel" class to suppress wheel events; remove the handleWheel function and any onWheel={handleWheel} usages in NoteNode.tsx (also remove the duplicate handler instance used later in the same file) so the component relies on the existing "nowheel" class instead of an extra stopPropagation call.frontend/src/components/graph/hooks/useOutputSinkSync.ts (1)
14-23: JSON.stringify comparison on every render may be inefficient.The effect runs on every
nodeschange and stringifies the filtered output configs each time. For large graphs, consider a more targeted dependency or shallow comparison.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/hooks/useOutputSinkSync.ts` around lines 14 - 23, The current effect in useOutputSinkSync stringifies full configs on every nodes change which is expensive; instead memoize the relevant output subset and compute a cheap stable key from only the fields you care about (e.g. map output nodes to strings like `${node.id}|${outputSinkType}|${outputSinkEnabled}|${outputSinkName}` and join) or use useMemo to derive outputNodes/configs so the effect depends on that memoized value (or a list of node ids+fields) rather than the entire nodes array; update references to prevOutputConfigsRef, configKey, and the filtering code in useOutputSinkSync to use the new memoized key/comparison.frontend/src/components/graph/ui/NodeCard.tsx (1)
22-41: Consider addingchildrento the dependency array for re-measurement.The effect currently only depends on
autoMinHeight. If children change without triggering a resize event (e.g., text content updates), theminHstate may become stale. The ResizeObserver should catch most cases, but addingchildrenwould make the intent clearer.That said, since ResizeObserver is watching the element and will fire on size changes, this is likely fine in practice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/ui/NodeCard.tsx` around lines 22 - 41, The effect in NodeCard's useEffect only lists autoMinHeight in its dependency array so changes to children can leave minH stale; update the dependency array to include children (alongside autoMinHeight) so the measure function (which reads measureRef.current, uses el.scrollHeight and calls setMinH) re-runs when children change, while keeping the ResizeObserver setup/teardown (ro.observe and ro.disconnect) intact to avoid loops.frontend/src/components/graph/NodeParametersPanel.tsx (2)
85-98: Consider hoistingisArrayOfNumbershelper outside the render function.The
isArrayOfNumbershelper is defined insiderenderPrimitiveField, causing it to be recreated on every call. While not a performance concern for this use case, hoisting it would be cleaner:♻️ Hoist helper
+const isArrayOfNumbers = (obj: Record<string, unknown>): boolean => { + if (obj.type === "array" && obj.items) { + const items = obj.items as { type?: string }; + return items.type === "integer" || items.type === "number"; + } + return false; +}; + export function NodeParametersPanel({ ... }) { ... const renderPrimitiveField = (field: ParsedFieldConfig, disabled: boolean) => { let resolvedType = field.fieldType; - const isArrayOfNumbers = (obj: Record<string, unknown>): boolean => { - if (obj.type === "array" && obj.items) { - const items = obj.items as { type?: string }; - return items.type === "integer" || items.type === "number"; - } - return false; - }; const propAsRecord = field.prop as unknown as Record<string, unknown>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/NodeParametersPanel.tsx` around lines 85 - 98, The helper isArrayOfNumbers is declared inside renderPrimitiveField causing it to be recreated on each render; move the isArrayOfNumbers function to module scope (outside renderPrimitiveField) and update renderPrimitiveField to call the hoisted helper. Locate the isArrayOfNumbers definition within renderPrimitiveField in NodeParametersPanel.tsx, extract it to a top-level function (keeping the same signature and behavior), and ensure any type references (e.g., ParsedFieldConfig usage in renderPrimitiveField) still compile after the move.
30-31: HardcodedRUNTIME_EDITABLE_LOAD_PARAMSmay need to scale.The set currently contains only
"vace_context_scale". As more pipelines are added, this list may grow. Consider whether this should be derived from schema metadata (e.g., aruntime_editableflag on load params) rather than hardcoded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/NodeParametersPanel.tsx` around lines 30 - 31, RUNTIME_EDITABLE_LOAD_PARAMS is hardcoded which won't scale; update NodeParametersPanel to derive runtime-editability from the load param schema (e.g., a runtime_editable flag on each load param) instead of relying solely on the RUNTIME_EDITABLE_LOAD_PARAMS Set: remove or keep the Set only as a backward-compatible fallback, add logic where parameters are built/rendered to check paramSchema.runtime_editable (or param.metadata.runtime_editable) to mark a param editable during streaming, and ensure any existing usages of RUNTIME_EDITABLE_LOAD_PARAMS are updated to consult the schema-derived flag first.frontend/src/components/graph/nodes/OutputNode.tsx (2)
52-59: Consider reusingupdateDatafor title changes to reduce duplication.The
onTitleChangehandler on lines 87-95 duplicates thesetNodespattern already encapsulated inupdateData. Consider reusing the helper for consistency:♻️ Proposed refactor
<NodeHeader title={data.customTitle || `${typeLabel} Output`} dotColor={dotColor} - onTitleChange={newTitle => - setNodes(nds => - nds.map(n => - n.id === id - ? { ...n, data: { ...n.data, customTitle: newTitle } } - : n - ) - ) - } + onTitleChange={newTitle => updateData({ customTitle: newTitle })} />Also applies to: 87-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/nodes/OutputNode.tsx` around lines 52 - 59, The onTitleChange handler duplicates the setNodes mapping logic already implemented in updateData; refactor onTitleChange to call updateData({ title: newTitle }) (or updateData with the appropriate FlowNodeData field) instead of directly calling setNodes, keeping id and current node merge semantics intact and ensuring the same immutable update pattern used by updateData is preserved.
45-50: Unreachable fallback in filter logic.Line 49's
return trueis unreachable since lines 46-48 exhaustively handle all three possibleopt.valuecases ("spout","ndi","syphon"). If new output types are added later, this fallback would silently include them regardless of availability. Consider either removing the fallback or making it explicit:♻️ Option: Make explicit or remove
const filteredOptions = OUTPUT_TYPE_OPTIONS.filter(opt => { if (opt.value === "spout") return spoutAvailable; if (opt.value === "ndi") return ndiAvailable; if (opt.value === "syphon") return syphonAvailable; - return true; + return false; // Unknown types excluded by default });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/nodes/OutputNode.tsx` around lines 45 - 50, The filter for filteredOptions currently uses a final "return true" fallback that is unreachable for the three known values and would silently allow any future unknown output types; update the filter in OutputNode.tsx so unknown opt.value are explicitly excluded—either remove the fallback and return false for any unhandled opt.value, or replace the switch with a lookup against an availability map (e.g., const availability = { spout: spoutAvailable, ndi: ndiAvailable, syphon: syphonAvailable } and return !!availability[opt.value]); ensure OUTPUT_TYPE_OPTIONS is filtered only when availability for opt.value is true.frontend/src/components/graph/AddNodeModal.tsx (1)
241-263: Tooltip portal positioning may clip at viewport edges.The tooltip uses
fixedpositioning withtranslate(-50%, -100%)which can cause clipping when tiles are near the top or sides of the viewport. For a modal centered on screen, this is likely fine, but consider edge detection if issues arise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/AddNodeModal.tsx` around lines 241 - 263, The tooltip in AddNodeModal uses a fixed container with transform: translate(-50%, -100%) (the conditional block rendering tooltip, the outer div with className "fixed z-[9999] pointer-events-none" and the style using tooltip.x/tooltip.y) which can cause clipping at viewport edges; update the positioning logic to perform edge detection before rendering: compute adjusted x/y coordinates (or flip vertical placement) when tooltip.x/tooltip.y would overflow the viewport, and apply those computed coordinates and an optional flipped transform/class to the same tooltip container so it stays fully visible near top or side edges.src/scope/server/graph_state.py (1)
80-89: Potential race condition inset_api_graphpersistence.The file write happens outside the lock (lines 85-86), which means concurrent
set_api_graphcalls could interleave their writes, potentially persisting an older graph. While the comment explains this is to avoid holding the lock during I/O, consider whether this race is acceptable for your use case.For a single-user desktop application, this is likely fine. For multi-threaded server scenarios with concurrent API calls, the last write wins regardless of order.
If strict ordering is needed:
♻️ Alternative: Write inside lock
def set_api_graph(graph: GraphConfig) -> None: """Store a graph config set via the API (memory + file).""" with _lock: global _graph_config _graph_config = graph - # Persist outside the lock to avoid holding it during I/O - _write_to_file(graph) + # Persist inside lock for strict ordering (blocks other operations during I/O) + _write_to_file(graph) logger.info( f"API graph set with {len(graph.nodes)} nodes and {len(graph.edges)} edges" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scope/server/graph_state.py` around lines 80 - 89, The current set_api_graph stores _graph_config under _lock but persists to disk outside the lock, allowing concurrent set_api_graph calls to interleave file writes and persist an older graph; to fix, either perform the file write while holding _lock (move the _write_to_file(graph) call inside the with _lock block) so the in-memory update and disk write are atomic, or implement a version/timestamp check: increment or attach a monotonic sequence/timestamp to _graph_config before releasing the lock and have _write_to_file verify it matches the latest _graph_config (or sequence) so only the most recent graph is persisted; update references to _graph_config, _lock, set_api_graph, and _write_to_file accordingly.frontend/src/components/graph/nodes/KnobsNode.tsx (1)
267-277: Using array index as key for dynamic list items.While using index as a key is generally discouraged for dynamic lists, it's acceptable here because knobs don't have stable unique identifiers. However, if users frequently reorder or remove knobs, React may incorrectly preserve input focus or animation state.
If knob ordering/removal becomes a UX issue, consider assigning stable IDs when knobs are created.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/nodes/KnobsNode.tsx` around lines 267 - 277, The list is using the array index as the React key in the knobs.map rendering which can cause incorrect focus/animation preservation when items are reordered or removed; modify the knobs data structure to assign a stable unique id when each knob is created (e.g., add an id property), update any creation logic that returns/dispatches knobs to include that id, and then change the map to pass key={knob.id} to SingleKnobRow (while keeping handlers like handleValueChange, handleFieldChange, handleRemove and the canRemove prop intact).frontend/src/components/graph/nodes/XYPadNode.tsx (1)
21-26: No validation whenminX >= maxXorminY >= maxY.If a user sets min equal to or greater than max (e.g.,
padMinX=1, padMaxX=0), the calculations on lines 41-42 will produceNaNor unexpected values. Consider adding defensive clamping or validation:🛡️ Defensive handling
- const pctX = maxX > minX ? (clampedX - minX) / (maxX - minX) : 0.5; - const pctY = maxY > minY ? (clampedY - minY) / (maxY - minY) : 0.5; + const rangeX = maxX - minX; + const rangeY = maxY - minY; + const pctX = rangeX > 0 ? (clampedX - minX) / rangeX : 0.5; + const pctY = rangeY > 0 ? (clampedY - minY) / rangeY : 0.5;This already exists on lines 82-83 but could be consolidated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/nodes/XYPadNode.tsx` around lines 21 - 26, In XYPadNode, validate and normalize pad bounds (minX, maxX, minY, maxY) before computing padX/padY: ensure min < max (swap values or set max = min + 1e-6/default range when min >= max), then clamp padX and padY into [min, max] and compute defaults using the safe midpoint; update the logic around variables minX, maxX, minY, maxY and padX/padY (which currently compute midpoint) so it uses the normalized bounds and avoids NaN; you can consolidate with the existing defensive handling referenced in the component (lines that already clamp) to keep validation centralized.src/scope/server/app.py (1)
580-595: Graph pipeline IDs override requested pipeline IDs unconditionally.When an API graph exists, the requested
pipeline_idsfrom thePipelineLoadRequestare completely replaced with the graph's pipeline IDs (line 594). This could be surprising if a caller explicitly requests specific pipelines but a stale graph is persisted.Consider documenting this behavior or logging when the override occurs:
📝 Add logging for visibility
if graph_pipeline_ids: # Use graph pipeline IDs, ensuring all are loaded + if set(graph_pipeline_ids) != set(pipeline_ids): + logger.info( + f"Overriding requested pipeline_ids {pipeline_ids} with " + f"graph pipeline_ids {graph_pipeline_ids}" + ) pipeline_ids = list(dict.fromkeys(graph_pipeline_ids))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scope/server/app.py` around lines 580 - 595, The code unconditionally replaces the requested pipeline_ids with IDs extracted from get_api_graph() (api_graph -> graph_pipeline_ids), which can unexpectedly discard an explicit PipelineLoadRequest; update the logic to either merge the two sets or at minimum log when an override happens: compute graph_pipeline_ids from api_graph, take the union with the existing pipeline_ids (preserving uniqueness), assign that back to pipeline_ids, and add a clear log entry indicating api_graph was present and showing both original requested pipeline_ids and the graph_pipeline_ids (reference symbols: get_api_graph, api_graph, graph_pipeline_ids, pipeline_ids, PipelineLoadRequest).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/graph/hooks/useOutputSinkSync.ts`:
- Around line 27-31: The sinkConfigs map built in useOutputSinkSync.ts
(variables allSinkTypes, sinkConfigs and loop over configs) keys by c.type, so
multiple output nodes sharing the same outputSinkType will overwrite each other;
update the logic to support multiple configs per type (e.g., make sinkConfigs
type Record<string, Array<{enabled:boolean;name:string}>> and push each config
into the array when iterating configs) or alternatively add UI/validation that
prevents creating duplicate types, and ensure downstream sync logic that reads
sinkConfigs (the consumer of this map) is updated to handle arrays instead of
single objects or enforce the single-instance invariant and document it.
In `@frontend/src/components/graph/hooks/useValueForwarding.ts`:
- Around line 74-75: The effect that checks isStreamingRef.current and
onNodeParamChangeRef.current (inside useEffect) won't run when those .current
values change, because refs aren't tracked in the dependency array; update the
hook to accept the boolean streaming state as a prop (e.g., streaming) or
otherwise include a streaming Boolean in the dependencies and call the same
handler when streaming transitions to true so initial connected values are
pushed; modify useValueForwarding to read the passed streaming prop (or add
streaming to the deps) and trigger the existing onNodeParamChangeRef.current
logic when streaming becomes true to ensure values are forwarded on stream
start.
In `@frontend/src/components/graph/nodes/ControlNode.tsx`:
- Around line 283-301: handleItemsChange currently normalizes the comma list on
every keystroke (splitting/trimming/filtering) which removes empty tokens and
prevents typing a trailing comma; change this by keeping the raw text input
separate from the parsed items: have handleItemsChange accept and set a local
raw string state (e.g., rawItemsInput) without splitting/filtering, and add a
new commit handler (e.g., handleItemsBlur or handleItemsCommit) that splits,
trims and filters into controlItems and updates the node data (the existing
setNodes map logic can be reused inside that commit handler); ensure the input
value binds to rawItemsInput and only rebuild rawItemsInput from
node.data.controlItems on mount or when external node data changes.
- Around line 94-124: The component must treat node.data.currentValue as the
source of truth: stop creating a fresh fallback array with data.controlItems ||
["item1", ...] each render and seed local state from data.currentValue (or
fallback only when data.currentValue is undefined) so we don't overwrite
existing node values on mount; update the initialization useEffect (the effect
that calls setNodes and the useState for currentValue) to first check
n.data.currentValue and only set it when undefined, and initialize
useState/currentValue from data.currentValue (or items[0] / min only if
data.currentValue is undefined); also avoid recreating controlItems by
referencing data.controlItems directly (or memoizing a fallback) and add
data.controlItems/data.currentValue/min to the effect deps so paused nodes
reflect Min/Max/Items changes.
In `@frontend/src/components/graph/nodes/KnobsNode.tsx`:
- Around line 205-206: The knobs array derivation should be wrapped in
React.useMemo so its reference is stable when data.knobs is falsy; change the
existing const knobs = data.knobs && data.knobs.length > 0 ? data.knobs :
[defaultKnob(0)] to a useMemo that depends on data.knobs (and defaultKnob if
necessary), and ensure React.useMemo is imported — this will prevent recreating
knobs on every render and stabilize the dependencies for handleValueChange,
handleFieldChange, handleRemove, and handleAdd callbacks.
In `@frontend/src/components/graph/nodes/PipelineNode.tsx`:
- Around line 105-126: When you remeasure and set rowPositions in PipelineNode's
useLayoutEffect, call the React Flow updater so handle anchors/hitboxes refresh:
import useUpdateNodeInternals from `@xyflow/react`, get updateNodeInternals =
useUpdateNodeInternals(), and after the setRowPositions (when keysChanged leads
to an updated positions object) call updateNodeInternals(id) (use the node's id
prop used by the component). Keep this call inside the same useLayoutEffect so
it runs whenever streamInputs/streamOutputs/parameterInputs/supportsPrompts or
row size changes trigger a remeasure; optionally add a ResizeObserver on rowRefs
to retrigger the same effect when individual rows resize.
In `@frontend/src/components/graph/nodes/SinkNode.tsx`:
- Around line 18-22: The useEffect in SinkNode that sets
videoRef.current.srcObject = remoteStream lacks cleanup, leaving previous
MediaStreams attached and causing leaks; update the effect (useEffect watching
remoteStream) to return a cleanup function that checks videoRef.current and
detaches the stream (e.g., set videoRef.current.srcObject = null and optionally
pause the element) and, if needed, stop or remove tracks from the old
MediaStream reference to free resources when remoteStream changes or the
component unmounts.
In `@frontend/src/components/graph/nodes/SliderNode.tsx`:
- Around line 27-28: The node currently computes a rendered clampedValue but
leaves data.value unchanged when min/max/step are edited, causing downstream
consumers to see stale/out-of-range values; update the logic in the
initialization and in the handlers that persist Min/Max/Step (e.g., onMinChange,
onMaxChange, onStepChange or wherever you call setNodeData/saveConfig) to
normalize data.value: compute the clampedValue (bounded by min/max) and quantize
it to the nearest valid step, then persist that normalized value back into
data.value (use the same clamping/quantization used to compute clampedValue) so
both the UI and node data stay in sync.
In `@frontend/src/components/graph/nodes/TupleNode.tsx`:
- Around line 274-287: The per-row handle IDs generated in the values.map loop
(id built via buildHandleId("param", `row_${i}`) in TupleNode.tsx) are
index-based and shift when rows are deleted, causing orphaned edges; change to
stable IDs by attaching a unique identifier to each row (e.g., use the row's
unique key or an explicit rowId on the tupleValues items) and use that when
calling buildHandleId (e.g., `row_${rowId}`), or alternatively add cleanup logic
in the component that runs when tupleValues change to find and remove edges
whose handle id no longer exists; update references in Handle creation, any edge
linking code, and anywhere buildHandleId is consumed to rely on the stable row
identifier.
In `@src/scope/server/frame_processor.py`:
- Around line 939-954: The early return that aborts _setup_pipeline_chain_sync
when self.pipeline_ids is empty is preventing graph-only sessions from using
get_api_graph(); update _setup_pipeline_chain_sync so it first calls
get_api_graph() (from graph_state) and only enforces the "empty pipeline_ids"
check when get_api_graph() returns None — i.e., move or condition the existing
empty-ID validation to occur after api_graph is obtained, and if api_graph is
None then call _build_linear_graph(self.pipeline_ids) and enforce that
pipeline_ids is non-empty before returning; then call _setup_graph(api_graph) as
currently done.
- Around line 1008-1010: FrameProcessor._input_source_receiver_loop currently
writes incoming Spout/NDI/Syphon frames directly into
pipeline_processors[0].input_queue, bypassing the graph fan-out; change the loop
to route those frames into the graph ingress (self._graph_source_queues /
graph_run.source_queues) the same way FrameProcessor.put() does so that each
source edge receives the frames; locate the _input_source_receiver_loop method
and replace direct writes to pipeline_processors[0].input_queue with enqueuing
to self._graph_source_queues (or calling the same helper used by put()) so
branched graphs get frames on every outgoing edge.
- Around line 1013-1015: The _processors_by_node_id map is only appended to and
can retain stale mappings across restarts/reloads; before repopulating it from
self.pipeline_processors (in the loop that assigns
self._processors_by_node_id[proc.node_id] = proc) reset/clear
self._processors_by_node_id so removed node IDs don't resolve to stopped
processors, and also ensure the teardown/stop logic clears related graph fields
(e.g., self.pipeline_processors and any graph reference fields used for routing)
to fully remove stale state.
- Around line 627-633: The current logic pops node_id from parameters and always
merges the remaining dict into self.parameters, which turns node-scoped updates
into global ones; change it so that if node_id is present and resolves in
self._processors_by_node_id you only call that processor.update_parameters(...)
and persist those values in a per-node store (e.g., add or update
self._node_parameters[node_id] = {...}) and do NOT merge them into
self.parameters, whereas if node_id is absent you broadcast to every processor
in self.pipeline_processors and then merge into self.parameters; also avoid
mutating the incoming dict (copy before popping) so update_parameters and merges
use isolated dict copies.
In `@src/scope/server/graph_executor.py`:
- Around line 68-73: The current stream_queues mapping uses (e.to_node,
e.to_port) which collapses multiple incoming stream edges onto a single queue;
change the keying so each edge gets its own queue (e.g., use a unique edge key
such as (e.from_node, e.from_port, e.to_node, e.to_port) or an edge.id if
available) inside the loop over graph.edges where e.kind == "stream" so that
stream_queues[(...unique-edge-key...)] =
queue.Queue(maxsize=DEFAULT_INPUT_QUEUE_MAXSIZE) and producers are not
interleaved on a single input port.
In `@src/scope/server/pipeline_processor.py`:
- Around line 551-563: The current block builds extra_params and broadcasts them
to every unique consumer_proc via output_consumers, which sends auxiliary
tensors to processors/ports that don't expect them; instead, iterate per output
port in output_consumers and forward extra_params only to the consumers
registered for that specific port (i.e., use the port key when looping through
self.output_consumers.items()), and skip forwarding to consumers that already
accept parameters via a queue by checking for a parameters_queue (or other queue
indicator) on the consumer entry before calling
consumer_proc.update_parameters(extra_params); alternatively remove this
fallback entirely until an explicit parameter-edge path exists.
---
Duplicate comments:
In `@frontend/src/components/graph/hooks/useGraphState.ts`:
- Around line 575-583: The graph load/reset paths (where graphConfig is
converted via graphConfigToFlow and nodes/edges set using enrichNodes, setNodes,
setEdges, colorEdges, setGraphSource) do not persist or hydrate per-node
nodeParams; move nodeParams into GraphConfig.ui_state during saves (use
flowToGraphConfig to include nodeParams into ui_state) and on loads/import/clear
read ui_state to set or clear the local nodeParams state before calling
enrichNodes and setNodes so prompt text and per-node params are restored or
reset and cannot leak across graphs; update all corresponding load spots (the
other instances you flagged) to extract nodeParams from graphConfig.ui_state and
apply/clear them consistently.
- Around line 634-639: The effect currently treats any nodes/edges change as a
user edit and calls onGraphChangeRef, causing non-user actions
(loadGraphFromBackend, refreshGraph, enrichment via setNodes) to flip StreamPage
state; add a suppression/ref flag to distinguish programmatic updates: introduce
a ref like suppressGraphChangeRef (or lastUpdateSource enum) checked in the
useEffect that returns early if set, and set that ref to true around
programmatic mutations inside loadGraphFromBackend, refreshGraph, and enrichment
code paths (clear it after the update completes) so only genuine user-initiated
edits trigger onGraphChangeRef.current().
In `@frontend/src/components/graph/ui/NodePillListInput.tsx`:
- Around line 23-49: handleChange currently calls onChange as soon as any token
parses (so typing "1," propagates [1]) which breaks incremental multi-value
entry; change the logic in handleChange (keep setInputValue and the useEffect
as-is) so that you only call onChange([]) when text === "" and otherwise only
call onChange(numbers) when parts.length > 0 AND numbers.length === parts.length
(i.e. every token parsed successfully); if some tokens are incomplete/invalid,
do not call onChange until they all parse.
In `@frontend/src/pages/StreamPage.tsx`:
- Around line 1884-1904: When switching away from server-side sources, clear the
persisted inputSource so leftover Spout/NDI/Syphon settings aren't reused: in
the branch where sourceMode is not "spout"/"ndi"/"syphon" (the else that
currently only calls switchMode(sourceMode)), call updateSettings to reset
settings.inputSource (e.g., set enabled: false and clear
source_type/source_name) before calling switchMode; use the same symbols shown
(sourceMode, updateSettings, settings.inputSource, switchMode, sourceNode) so
reviewers can locate and apply the change.
- Around line 1615-1634: The code currently derives isServerSideInput and stream
requirement from mode/settings.inputSource which can be stale for running
graphs; change the logic to use the graph's resolved/normalized source object
(e.g., a resolvedGraphSource or graph.inputSource) as the single source of truth
while the graph is active: compute isServerSideInput, needsVideoInput (instead
of reading mode/currentMode/settings.inputSource) from that normalized source,
then set streamToSend and the localStream existence check based on that resolved
source; update the same pattern used later (around the streamToSend/validation
at the other block around 1738-1743) to reference the normalized graph source
rather than mode/settings.inputSource so persisted NDI/Spout/Syphon graphs don't
incorrectly require or inherit local streams.
In `@src/scope/server/graph_executor.py`:
- Around line 156-163: The loop over graph.edges currently breaks after wiring
backpressure for the first video consumer; remove the `break` so every stream
edge with from_port == "video" wires backpressure. Specifically, in the block
iterating `for e in graph.edges` (checking `e.kind != "stream"` and `e.from_port
!= "video"`), call `producer.throttler.set_next_processor(consumer)` for each
edge where `producer = node_processors.get(e.from_node)` and `consumer =
node_processors.get(e.to_node)` are not None, and do not exit the loop early so
multi-stage/branched graphs get throttling on all video paths. Ensure
null-checks for `producer`/`consumer` remain as-is.
---
Nitpick comments:
In `@frontend/src/components/graph/AddNodeModal.tsx`:
- Around line 241-263: The tooltip in AddNodeModal uses a fixed container with
transform: translate(-50%, -100%) (the conditional block rendering tooltip, the
outer div with className "fixed z-[9999] pointer-events-none" and the style
using tooltip.x/tooltip.y) which can cause clipping at viewport edges; update
the positioning logic to perform edge detection before rendering: compute
adjusted x/y coordinates (or flip vertical placement) when tooltip.x/tooltip.y
would overflow the viewport, and apply those computed coordinates and an
optional flipped transform/class to the same tooltip container so it stays fully
visible near top or side edges.
In `@frontend/src/components/graph/ContextMenu.tsx`:
- Around line 54-67: The ContextMenu's items.map currently uses the array index
as the React key (key={index}) which can cause reconciliation bugs if items
change order; update the mapping in ContextMenu.tsx to use a stable unique
identifier from each item instead (e.g., key={item.id} or key={item.label} if
labels are guaranteed unique) and fall back to index only if no stable id/label
exists—ensure you reference the items.map callback and the button element when
making this change.
In `@frontend/src/components/graph/hooks/useOutputSinkSync.ts`:
- Around line 14-23: The current effect in useOutputSinkSync stringifies full
configs on every nodes change which is expensive; instead memoize the relevant
output subset and compute a cheap stable key from only the fields you care about
(e.g. map output nodes to strings like
`${node.id}|${outputSinkType}|${outputSinkEnabled}|${outputSinkName}` and join)
or use useMemo to derive outputNodes/configs so the effect depends on that
memoized value (or a list of node ids+fields) rather than the entire nodes
array; update references to prevOutputConfigsRef, configKey, and the filtering
code in useOutputSinkSync to use the new memoized key/comparison.
In `@frontend/src/components/graph/NodeParametersPanel.tsx`:
- Around line 85-98: The helper isArrayOfNumbers is declared inside
renderPrimitiveField causing it to be recreated on each render; move the
isArrayOfNumbers function to module scope (outside renderPrimitiveField) and
update renderPrimitiveField to call the hoisted helper. Locate the
isArrayOfNumbers definition within renderPrimitiveField in
NodeParametersPanel.tsx, extract it to a top-level function (keeping the same
signature and behavior), and ensure any type references (e.g., ParsedFieldConfig
usage in renderPrimitiveField) still compile after the move.
- Around line 30-31: RUNTIME_EDITABLE_LOAD_PARAMS is hardcoded which won't
scale; update NodeParametersPanel to derive runtime-editability from the load
param schema (e.g., a runtime_editable flag on each load param) instead of
relying solely on the RUNTIME_EDITABLE_LOAD_PARAMS Set: remove or keep the Set
only as a backward-compatible fallback, add logic where parameters are
built/rendered to check paramSchema.runtime_editable (or
param.metadata.runtime_editable) to mark a param editable during streaming, and
ensure any existing usages of RUNTIME_EDITABLE_LOAD_PARAMS are updated to
consult the schema-derived flag first.
In `@frontend/src/components/graph/nodes/KnobsNode.tsx`:
- Around line 267-277: The list is using the array index as the React key in the
knobs.map rendering which can cause incorrect focus/animation preservation when
items are reordered or removed; modify the knobs data structure to assign a
stable unique id when each knob is created (e.g., add an id property), update
any creation logic that returns/dispatches knobs to include that id, and then
change the map to pass key={knob.id} to SingleKnobRow (while keeping handlers
like handleValueChange, handleFieldChange, handleRemove and the canRemove prop
intact).
In `@frontend/src/components/graph/nodes/MathNode.tsx`:
- Around line 227-242: The useLayoutEffect that measures DOM via rowRefs and
updates state via setRowPositions currently has no dependency array and
therefore runs on every render; restrict it by adding a dependency array to the
effect (for example including rowRefs.current, any props/state that affect row
elements like the data/rows prop, and the setter setRowPositions) so the
measurement only re-runs when those inputs change—update the useLayoutEffect
invocation in MathNode.tsx (the effect that reads rowRefs.current, computes
positions, and calls setRowPositions) to include the appropriate dependencies.
- Around line 185-198: The useEffect in MathNode.tsx that calls setNodes (using
id, setNodes, result) can trigger cascading renders; change it so you only call
setNodes when the node's currentValue would actually change: read the node by id
(or compare against a lastResult ref) and return early if result === existing
currentValue to avoid no-op updates, or debounce/batch updates (e.g., via a
useRef timer) when results change rapidly; ensure the update still uses the
functional updater (nds => nds.map(...)) and only maps/returns a new node object
when the value differs to prevent unnecessary re-renders.
In `@frontend/src/components/graph/nodes/NoteNode.tsx`:
- Around line 27-29: The onWheel handler handleWheel (which only calls
e.stopPropagation()) is redundant because the element already uses the "nowheel"
class to suppress wheel events; remove the handleWheel function and any
onWheel={handleWheel} usages in NoteNode.tsx (also remove the duplicate handler
instance used later in the same file) so the component relies on the existing
"nowheel" class instead of an extra stopPropagation call.
In `@frontend/src/components/graph/nodes/OutputNode.tsx`:
- Around line 52-59: The onTitleChange handler duplicates the setNodes mapping
logic already implemented in updateData; refactor onTitleChange to call
updateData({ title: newTitle }) (or updateData with the appropriate FlowNodeData
field) instead of directly calling setNodes, keeping id and current node merge
semantics intact and ensuring the same immutable update pattern used by
updateData is preserved.
- Around line 45-50: The filter for filteredOptions currently uses a final
"return true" fallback that is unreachable for the three known values and would
silently allow any future unknown output types; update the filter in
OutputNode.tsx so unknown opt.value are explicitly excluded—either remove the
fallback and return false for any unhandled opt.value, or replace the switch
with a lookup against an availability map (e.g., const availability = { spout:
spoutAvailable, ndi: ndiAvailable, syphon: syphonAvailable } and return
!!availability[opt.value]); ensure OUTPUT_TYPE_OPTIONS is filtered only when
availability for opt.value is true.
In `@frontend/src/components/graph/nodes/TupleNode.tsx`:
- Around line 189-209: The onPointerDown handler in TupleNode.tsx attaches
pointermove/pointerup listeners to the DOM target (variable target) but doesn't
remove them if the component unmounts during a drag; update the implementation
to track the active pointer target (e.g., a ref like activePointerTarget) and
the bound handlers (onMove/onUp), register them as now but also remove them and
call releasePointerCapture in a useEffect cleanup that runs on unmount (or
attach listeners to window and remove on unmount), ensuring
target.removeEventListener("pointermove", onMove) and
target.removeEventListener("pointerup", onUp) are invoked for the same handler
references used when adding; keep setFromMouse and setValueAt usage the same but
ensure handlers are stable or stored so cleanup can remove them reliably.
In `@frontend/src/components/graph/nodes/XYPadNode.tsx`:
- Around line 21-26: In XYPadNode, validate and normalize pad bounds (minX,
maxX, minY, maxY) before computing padX/padY: ensure min < max (swap values or
set max = min + 1e-6/default range when min >= max), then clamp padX and padY
into [min, max] and compute defaults using the safe midpoint; update the logic
around variables minX, maxX, minY, maxY and padX/padY (which currently compute
midpoint) so it uses the normalized bounds and avoids NaN; you can consolidate
with the existing defensive handling referenced in the component (lines that
already clamp) to keep validation centralized.
In `@frontend/src/components/graph/ui/NodeCard.tsx`:
- Around line 22-41: The effect in NodeCard's useEffect only lists autoMinHeight
in its dependency array so changes to children can leave minH stale; update the
dependency array to include children (alongside autoMinHeight) so the measure
function (which reads measureRef.current, uses el.scrollHeight and calls
setMinH) re-runs when children change, while keeping the ResizeObserver
setup/teardown (ro.observe and ro.disconnect) intact to avoid loops.
In `@frontend/src/components/graph/ui/NodePillInput.tsx`:
- Around line 32-41: handleChange currently converts and forwards typed numbers
without enforcing min/max, so manual input can violate bounds; update
handleChange (and/or add a complementary onBlur handler) to clamp numeric values
to the provided min and max props before calling onChange (use
Number(e.target.value) and ensure Number.isFinite), referencing the existing
handleChange, the type prop, and onChange callback; if you add an onBlur, name
it handleBlur and ensure it also clamps and calls onChange so pasted or
partially-entered values are normalized when the input loses focus.
- Around line 53-98: The document-level mousemove/mouseup listeners added inside
handleMouseDown can leak if the component unmounts during a drag; add a
useEffect that returns a cleanup function which removes those listeners and
clears dragRef and that cleanup should reference the same onMove/onUp handlers
(or guard via stable refs) so they can be removed correctly; specifically update
NodePillInput to add a useEffect that removes
document.removeEventListener("mousemove", onMove) and
document.removeEventListener("mouseup", onUp) on unmount and resets
dragRef.current = null, ensuring onMove/onUp are stable or stored in refs so
removal works.
In `@src/scope/server/app.py`:
- Around line 580-595: The code unconditionally replaces the requested
pipeline_ids with IDs extracted from get_api_graph() (api_graph ->
graph_pipeline_ids), which can unexpectedly discard an explicit
PipelineLoadRequest; update the logic to either merge the two sets or at minimum
log when an override happens: compute graph_pipeline_ids from api_graph, take
the union with the existing pipeline_ids (preserving uniqueness), assign that
back to pipeline_ids, and add a clear log entry indicating api_graph was present
and showing both original requested pipeline_ids and the graph_pipeline_ids
(reference symbols: get_api_graph, api_graph, graph_pipeline_ids, pipeline_ids,
PipelineLoadRequest).
In `@src/scope/server/graph_state.py`:
- Around line 80-89: The current set_api_graph stores _graph_config under _lock
but persists to disk outside the lock, allowing concurrent set_api_graph calls
to interleave file writes and persist an older graph; to fix, either perform the
file write while holding _lock (move the _write_to_file(graph) call inside the
with _lock block) so the in-memory update and disk write are atomic, or
implement a version/timestamp check: increment or attach a monotonic
sequence/timestamp to _graph_config before releasing the lock and have
_write_to_file verify it matches the latest _graph_config (or sequence) so only
the most recent graph is persisted; update references to _graph_config, _lock,
set_api_graph, and _write_to_file accordingly.
Comment on lines
+205
to
+206
| const knobs: KnobDef[] = | ||
| data.knobs && data.knobs.length > 0 ? data.knobs : [defaultKnob(0)]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wrap knobs derivation in useMemo to stabilize callback dependencies.
The static analysis correctly flags that the conditional knobs assignment creates a new array reference on every render when data.knobs is falsy. This causes handleValueChange, handleFieldChange, handleRemove, and handleAdd callbacks to be recreated unnecessarily.
♻️ Proposed fix
+import { useCallback, useMemo, useRef } from "react";
-import { useCallback, useRef } from "react";
...
export function KnobsNode({ id, data, selected }: NodeProps<KnobsNodeType>) {
const { setNodes } = useReactFlow();
- const knobs: KnobDef[] =
- data.knobs && data.knobs.length > 0 ? data.knobs : [defaultKnob(0)];
+ const knobs: KnobDef[] = useMemo(
+ () => (data.knobs && data.knobs.length > 0 ? data.knobs : [defaultKnob(0)]),
+ [data.knobs]
+ );🧰 Tools
🪛 GitHub Check: Frontend Linting (ESLint + Prettier)
[warning] 205-205:
The 'knobs' conditional could make the dependencies of useCallback Hook (at line 248) change on every render. To fix this, wrap the initialization of 'knobs' in its own useMemo() Hook
[warning] 205-205:
The 'knobs' conditional could make the dependencies of useCallback Hook (at line 243) change on every render. To fix this, wrap the initialization of 'knobs' in its own useMemo() Hook
[warning] 205-205:
The 'knobs' conditional could make the dependencies of useCallback Hook (at line 234) change on every render. To fix this, wrap the initialization of 'knobs' in its own useMemo() Hook
[warning] 205-205:
The 'knobs' conditional could make the dependencies of useCallback Hook (at line 225) change on every render. To fix this, wrap the initialization of 'knobs' in its own useMemo() Hook
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/graph/nodes/KnobsNode.tsx` around lines 205 - 206,
The knobs array derivation should be wrapped in React.useMemo so its reference
is stable when data.knobs is falsy; change the existing const knobs = data.knobs
&& data.knobs.length > 0 ? data.knobs : [defaultKnob(0)] to a useMemo that
depends on data.knobs (and defaultKnob if necessary), and ensure React.useMemo
is imported — this will prevent recreating knobs on every render and stabilize
the dependencies for handleValueChange, handleFieldChange, handleRemove, and
handleAdd callbacks.
Comment on lines
+105
to
+126
| useLayoutEffect(() => { | ||
| const positions: Record<string, number> = {}; | ||
| rowRefs.current.forEach((el, key) => { | ||
| if (el) { | ||
| positions[key] = el.offsetTop + el.offsetHeight / 2; | ||
| } | ||
| }); | ||
| setRowPositions(prev => { | ||
| const keysChanged = | ||
| Object.keys(positions).length !== Object.keys(prev).length || | ||
| Object.keys(positions).some( | ||
| key => Math.abs((prev[key] ?? 0) - positions[key]) > 0.5 | ||
| ); | ||
| return keysChanged ? positions : prev; | ||
| }); | ||
| }, [ | ||
| streamInputs, | ||
| streamOutputs, | ||
| parameterInputs, | ||
| supportsPrompts, | ||
| data.pipelineId, | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the file size and content
wc -l frontend/src/components/graph/nodes/PipelineNode.tsxRepository: daydreamlive/scope
Length of output: 118
🏁 Script executed:
# Read the file to understand the full context
cat -n frontend/src/components/graph/nodes/PipelineNode.tsx | head -150Repository: daydreamlive/scope
Length of output: 5762
🏁 Script executed:
# Search for useUpdateNodeInternals usage in the codebase
rg "useUpdateNodeInternals" frontend/src/Repository: daydreamlive/scope
Length of output: 44
🏁 Script executed:
# Search for Handle component usage in PipelineNode
rg "Handle" frontend/src/components/graph/nodes/PipelineNode.tsx -A 2 -B 2Repository: daydreamlive/scope
Length of output: 2265
🏁 Script executed:
# Check imports in PipelineNode.tsx
head -30 frontend/src/components/graph/nodes/PipelineNode.tsxRepository: daydreamlive/scope
Length of output: 897
🏁 Script executed:
# Search for Handle styling with top position
grep -n "style=" frontend/src/components/graph/nodes/PipelineNode.tsx | head -20Repository: daydreamlive/scope
Length of output: 176
🏁 Script executed:
# Get the complete Handle definitions with their styling
sed -n '195,300p' frontend/src/components/graph/nodes/PipelineNode.tsxRepository: daydreamlive/scope
Length of output: 4376
🏁 Script executed:
# Check if useUpdateNodeInternals is available from `@xyflow/react`
grep -r "useUpdateNodeInternals" frontend/Repository: daydreamlive/scope
Length of output: 44
🏁 Script executed:
# Check package.json for `@xyflow/react` version
cat package.json | grep -A 2 "@xyflow"Repository: daydreamlive/scope
Length of output: 106
🏁 Script executed:
# Find package.json location
find . -name "package.json" -type f | head -5Repository: daydreamlive/scope
Length of output: 123
🏁 Script executed:
# Get the complete Handle style definitions with top positioning
sed -n '330,415p' frontend/src/components/graph/nodes/PipelineNode.tsxRepository: daydreamlive/scope
Length of output: 2408
🏁 Script executed:
# Check all imports to see if useUpdateNodeInternals should be available
head -50 frontend/src/components/graph/nodes/PipelineNode.tsxRepository: daydreamlive/scope
Length of output: 1335
🏁 Script executed:
# Check frontend/package.json for `@xyflow/react` version
grep -A 5 "@xyflow" frontend/package.jsonRepository: daydreamlive/scope
Length of output: 243
🏁 Script executed:
# Look for any existing uses of node internals hooks in the codebase
rg "useNode|useUpdateNode" frontend/src/ --type tsx --type tsRepository: daydreamlive/scope
Length of output: 89
🏁 Script executed:
# Check if there are edge issues related to handle positioning
rg "rowPositions|Handle.*style" frontend/src/components/graph/ -A 2 -B 2Repository: daydreamlive/scope
Length of output: 4551
🌐 Web query:
@xyflow/react useUpdateNodeInternals dynamic handle positioning CSS
💡 Result:
To dynamically reposition <Handle />s with CSS in @xyflow/react, you typically:
-
Move the handle with CSS/inline styles (e.g.
top/left/transform) while keeping thepositionprop as the side you want (Left/Right/Top/Bottom). React Flow supports overriding the default centering by styling the handle yourself. [1] -
After the handle’s DOM position changes, call
updateNodeInternals(nodeId)fromuseUpdateNodeInternals()so React Flow re-measures the node/handles and updates edge anchor points. This is required when you “update a node’s handle position”. [2][3]
Minimal pattern:
import { Handle, Position, useUpdateNodeInternals } from '@xyflow/react';
import { useLayoutEffect } from 'react';
function MyNode({ id, data }) {
const updateNodeInternals = useUpdateNodeInternals();
// data.handleTop changes based on your layout/CSS logic
useLayoutEffect(() => {
// ensure React Flow re-measures handle positions after your render
updateNodeInternals(id);
}, [id, data.handleTop, updateNodeInternals]);
return (
<div>
<Handle
type="source"
position={Position.Right}
style={{ top: data.handleTop }} // dynamic CSS positioning
/>
</div>
);
}Notes that often matter in practice:
- If you “hide” handles, don’t use
display: none(it measures as 0×0); usevisibility: hiddenoropacity: 0. [1][3] - If you change handle positions due to async/layout changes, call
updateNodeInternals(id)after that change completes. [3]
Sources: React Flow “Handles” docs [1], useUpdateNodeInternals() reference [2], and troubleshooting notes on updating internals / handle visibility [3].
Call useUpdateNodeInternals() when repositioning handles.
The code updates handle positions via inline styles using rowPositions, but @xyflow/react requires updateNodeInternals(id) to be called whenever handle positions change. Without it, edge anchors and connection hitboxes remain stale after pipeline selection, parameter changes, or layout shifts. Import useUpdateNodeInternals from @xyflow/react and call it in the useLayoutEffect after remeasuring positions. Additionally, consider using a ResizeObserver to retrigger measurements when rows resize (e.g., expanding textareas).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/graph/nodes/PipelineNode.tsx` around lines 105 - 126,
When you remeasure and set rowPositions in PipelineNode's useLayoutEffect, call
the React Flow updater so handle anchors/hitboxes refresh: import
useUpdateNodeInternals from `@xyflow/react`, get updateNodeInternals =
useUpdateNodeInternals(), and after the setRowPositions (when keysChanged leads
to an updated positions object) call updateNodeInternals(id) (use the node's id
prop used by the component). Keep this call inside the same useLayoutEffect so
it runs whenever streamInputs/streamOutputs/parameterInputs/supportsPrompts or
row size changes trigger a remeasure; optionally add a ResizeObserver on rowRefs
to retrigger the same effect when individual rows resize.
Comment on lines
+18
to
+22
| useEffect(() => { | ||
| if (videoRef.current && remoteStream instanceof MediaStream) { | ||
| videoRef.current.srcObject = remoteStream; | ||
| } | ||
| }, [remoteStream]); |
There was a problem hiding this comment.
Missing cleanup for video srcObject.
When remoteStream changes or the component unmounts, the previous stream remains attached to the video element. This can cause stale playback or resource leaks.
🛡️ Proposed fix to add cleanup
useEffect(() => {
- if (videoRef.current && remoteStream instanceof MediaStream) {
- videoRef.current.srcObject = remoteStream;
+ const video = videoRef.current;
+ if (video && remoteStream instanceof MediaStream) {
+ video.srcObject = remoteStream;
}
+ return () => {
+ if (video) {
+ video.srcObject = null;
+ }
+ };
}, [remoteStream]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| if (videoRef.current && remoteStream instanceof MediaStream) { | |
| videoRef.current.srcObject = remoteStream; | |
| } | |
| }, [remoteStream]); | |
| useEffect(() => { | |
| const video = videoRef.current; | |
| if (video && remoteStream instanceof MediaStream) { | |
| video.srcObject = remoteStream; | |
| } | |
| return () => { | |
| if (video) { | |
| video.srcObject = null; | |
| } | |
| }; | |
| }, [remoteStream]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/graph/nodes/SinkNode.tsx` around lines 18 - 22, The
useEffect in SinkNode that sets videoRef.current.srcObject = remoteStream lacks
cleanup, leaving previous MediaStreams attached and causing leaks; update the
effect (useEffect watching remoteStream) to return a cleanup function that
checks videoRef.current and detaches the stream (e.g., set
videoRef.current.srcObject = null and optionally pause the element) and, if
needed, stop or remove tracks from the old MediaStream reference to free
resources when remoteStream changes or the component unmounts.
Comment on lines
+27
to
+28
| const value = typeof data.value === "number" ? data.value : min; | ||
|
|
There was a problem hiding this comment.
Normalize value when Min/Max/Step changes.
The UI clamps to clampedValue, but these handlers only persist the config fields. After editing Min/Max/Step, data.value can stay out of range or off-step until the user drags again, so downstream connections can see a different value than the node is rendering.
Also applies to: 40-41, 119-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/graph/nodes/SliderNode.tsx` around lines 27 - 28, The
node currently computes a rendered clampedValue but leaves data.value unchanged
when min/max/step are edited, causing downstream consumers to see
stale/out-of-range values; update the logic in the initialization and in the
handlers that persist Min/Max/Step (e.g., onMinChange, onMaxChange, onStepChange
or wherever you call setNodeData/saveConfig) to normalize data.value: compute
the clampedValue (bounded by min/max) and quantize it to the nearest valid step,
then persist that normalized value back into data.value (use the same
clamping/quantization used to compute clampedValue) so both the UI and node data
stay in sync.
Comment on lines
+274
to
+287
| {/* Per-row input handles (left, aligned with each value row) */} | ||
| {values.map((_, i) => { | ||
| const yOffset = rowsSectionTop + i * ROW_HEIGHT + ROW_HEIGHT / 2; | ||
| return ( | ||
| <Handle | ||
| key={`row-in-${i}`} | ||
| type="target" | ||
| position={Position.Left} | ||
| id={buildHandleId("param", `row_${i}`)} | ||
| className="!w-2 !h-2 !border-0" | ||
| style={{ top: yOffset, left: 8, backgroundColor: "#fbbf24" }} | ||
| /> | ||
| ); | ||
| })} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for edge cleanup logic when tuple rows are modified
rg -n -C3 "tupleValues|row_" --type=ts --glob="**/graph/**"Repository: daydreamlive/scope
Length of output: 7710
🏁 Script executed:
#!/bin/bash
# Search for edge cleanup/removal logic and node update handlers
rg -n -C3 "setEdges|deleteEdge|removeEdge|onNodesChange" --type=ts --glob="**/graph/**" | head -100Repository: daydreamlive/scope
Length of output: 7528
🏁 Script executed:
#!/bin/bash
# Look for handlers that might validate or clean edges
rg -n -C3 "tupleValues.*=|updateFields" frontend/src/components/graph/nodes/TupleNode.tsx | head -50Repository: daydreamlive/scope
Length of output: 1933
🏁 Script executed:
#!/bin/bash
# Check if there's edge validation when loading or modifying graph
rg -n "edges|connections" frontend/src/components/graph/hooks/useValueForwarding.ts | head -30Repository: daydreamlive/scope
Length of output: 307
🏁 Script executed:
#!/bin/bash
# Check if there's any edge validation/cleanup when tupleValues changes
# Look in useValueForwarding.ts for full context around edge handling
sed -n '1,50p' frontend/src/components/graph/hooks/useValueForwarding.tsRepository: daydreamlive/scope
Length of output: 1514
🏁 Script executed:
#!/bin/bash
# Search for any mechanism that validates/removes invalid edges
rg -n "target.*row_|source.*row_" --type=ts frontend/src/components/graph/hooks/ -A 2 -B 2Repository: daydreamlive/scope
Length of output: 1937
🏁 Script executed:
#!/bin/bash
# Check if there's a useEffect or cleanup mechanism for edges in TupleNode or related hooks
rg -n "useEffect|useMemo" frontend/src/components/graph/nodes/TupleNode.tsxRepository: daydreamlive/scope
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for any mechanism that validates or removes edges with invalid targets
rg -n "getNode|validateEdge|filter.*edge" frontend/src/components/graph/hooks/ --type=ts -B 2 -A 2 | head -80Repository: daydreamlive/scope
Length of output: 7130
🏁 Script executed:
#!/bin/bash
# Look at the full useValueForwarding hook to see the complete edge validation flow
wc -l frontend/src/components/graph/hooks/useValueForwarding.tsRepository: daydreamlive/scope
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Check if there's any edge cleanup when nodes change
rg -n "onNodesChange|useEffect" frontend/src/components/graph/GraphEditor.tsx -A 5 -B 2 | head -100Repository: daydreamlive/scope
Length of output: 568
🏁 Script executed:
#!/bin/bash
# Look at the full context of the useValueForwarding hook to see complete edge handling
sed -n '180,300p' frontend/src/components/graph/hooks/useValueForwarding.tsRepository: daydreamlive/scope
Length of output: 4880
🏁 Script executed:
#!/bin/bash
# Check if edges are ever pruned or validated against available handles in the graph
rg -n "setEdges\|filter.*edge" frontend/src/components/graph/hooks/useValueForwarding.ts -B 3 -A 3Repository: daydreamlive/scope
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check if there's any mechanism that removes/cleans orphaned edges when tuple rows change
rg -n "tupleValues" frontend/src/components/graph/nodes/TupleNode.tsx -B 5 -A 5 | grep -A 10 "updateValues\|setValueAt"Repository: daydreamlive/scope
Length of output: 266
🏁 Script executed:
#!/bin/bash
# Look at what happens after updateFields is called with new tupleValues
sed -n '36,100p' frontend/src/components/graph/nodes/TupleNode.tsxRepository: daydreamlive/scope
Length of output: 2037
🏁 Script executed:
#!/bin/bash
# Check if there's any edge validation mechanism after node updates
rg -n "updateFields|setNodes" frontend/src/components/graph/nodes/TupleNode.tsx -A 5Repository: daydreamlive/scope
Length of output: 2691
🏁 Script executed:
#!/bin/bash
# Verify: when a row is removed, do edges get cleaned up or do they remain orphaned?
# Let's check if setEdges is called when tupleValues changes
rg -n "removeRow|addRow" frontend/src/components/graph/nodes/TupleNode.tsx -A 10Repository: daydreamlive/scope
Length of output: 1395
🏁 Script executed:
#!/bin/bash
# Check if there's any mechanism that validates/removes edges when handles change
# Search for any callback that monitors node handle changes
rg -n "setEdges.*filter\|edges.*filter" frontend/src/components/graph/ --type=ts -B 2 -A 2Repository: daydreamlive/scope
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Final verification: Check if there's any mechanism that removes/cleans edges
# when a node's available handles change
rg -n "setEdges\|removeEdge\|filter.*edges" frontend/src/components/graph/hooks/useGraphState.ts -B 3 -A 3Repository: daydreamlive/scope
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check if ReactFlow or the graph system has built-in cleanup for invalid edges
rg -n "onConnect\|isValidConnection" frontend/src/components/graph/hooks/useConnectionLogic.ts | head -20Repository: daydreamlive/scope
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check if ReactFlow automatically cleans invalid edges
# Look at the full GraphEditor to see if there's a cleanup hook
rg -n "useEffect.*edges\|edges.*useEffect" frontend/src/components/graph/GraphEditor.tsx -A 5 -B 2Repository: daydreamlive/scope
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Look for any validation that removes edges with non-existent target handles
grep -n "getNode\|getEdge" frontend/src/components/graph/hooks/useGraphState.ts -B 2 -A 2Repository: daydreamlive/scope
Length of output: 490
🏁 Script executed:
#!/bin/bash
# Check if there's any mechanism that validates edge targets against available handles
rg -n "Handle|buildHandleId" frontend/src/components/graph/nodes/TupleNode.tsx | grep -E "row_|Handle"Repository: daydreamlive/scope
Length of output: 398
Remove or stabilize per-row handle IDs to prevent orphaned edges when rows change.
When a row is deleted, all subsequent rows shift indices (e.g., row_2 becomes row_1). Edges connected to the old row_2 handle become orphaned—they remain in the graph but no longer target a valid handle, and the forwarding logic silently fails. Consider either (1) using stable identifiers for row handles (e.g., based on unique row IDs rather than indices), or (2) cleaning up invalid edges whenever tupleValues changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/graph/nodes/TupleNode.tsx` around lines 274 - 287,
The per-row handle IDs generated in the values.map loop (id built via
buildHandleId("param", `row_${i}`) in TupleNode.tsx) are index-based and shift
when rows are deleted, causing orphaned edges; change to stable IDs by attaching
a unique identifier to each row (e.g., use the row's unique key or an explicit
rowId on the tupleValues items) and use that when calling buildHandleId (e.g.,
`row_${rowId}`), or alternatively add cleanup logic in the component that runs
when tupleValues change to find and remove edges whose handle id no longer
exists; update references in Handle creation, any edge linking code, and
anywhere buildHandleId is consumed to rely on the stable row identifier.
Comment on lines
+627
to
+633
| # Route to specific node or broadcast to all pipeline processors | ||
| node_id = parameters.pop("node_id", None) | ||
| if node_id and node_id in self._processors_by_node_id: | ||
| self._processors_by_node_id[node_id].update_parameters(parameters) | ||
| else: | ||
| for processor in self.pipeline_processors: | ||
| processor.update_parameters(parameters) |
There was a problem hiding this comment.
Node-targeted updates currently lose their scope.
When node_id is unknown this branch broadcasts to every processor, and even when it is found the same parameters dict is still merged into self.parameters at Lines 635-636. That turns a node-scoped edit into a global value that can leak into unrelated pipelines on the next graph rebuild. Only broadcast when node_id is absent, and persist targeted values separately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scope/server/frame_processor.py` around lines 627 - 633, The current
logic pops node_id from parameters and always merges the remaining dict into
self.parameters, which turns node-scoped updates into global ones; change it so
that if node_id is present and resolves in self._processors_by_node_id you only
call that processor.update_parameters(...) and persist those values in a
per-node store (e.g., add or update self._node_parameters[node_id] = {...}) and
do NOT merge them into self.parameters, whereas if node_id is absent you
broadcast to every processor in self.pipeline_processors and then merge into
self.parameters; also avoid mutating the incoming dict (copy before popping) so
update_parameters and merges use isolated dict copies.
Comment on lines
+1008
to
+1010
| self._graph_source_queues = graph_run.source_queues | ||
| self._sink_processor = graph_run.sink_processor | ||
| self.pipeline_processors = graph_run.processors |
There was a problem hiding this comment.
Generic input sources still bypass graph fan-out.
After this change graph_run.source_queues is the graph ingress, but FrameProcessor._input_source_receiver_loop() still writes directly to pipeline_processors[0].input_queue. In branched graphs, Spout/NDI/Syphon frames will only feed one branch instead of every source edge. Route those frames through _graph_source_queues, the same way FrameProcessor.put() does.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scope/server/frame_processor.py` around lines 1008 - 1010,
FrameProcessor._input_source_receiver_loop currently writes incoming
Spout/NDI/Syphon frames directly into pipeline_processors[0].input_queue,
bypassing the graph fan-out; change the loop to route those frames into the
graph ingress (self._graph_source_queues / graph_run.source_queues) the same way
FrameProcessor.put() does so that each source edge receives the frames; locate
the _input_source_receiver_loop method and replace direct writes to
pipeline_processors[0].input_queue with enqueuing to self._graph_source_queues
(or calling the same helper used by put()) so branched graphs get frames on
every outgoing edge.
Signed-off-by: gioelecerati <gioele@cerati.org>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
frontend/src/pages/StreamPage.tsx (1)
1930-1949:⚠️ Potential issue | 🟠 MajorClear stale server-side source settings when switching back to
video/camera.In Lines 1930-1949 only the Spout/NDI/Syphon branch rewrites
settings.inputSource. If the previous graph source was server-side and the current one isvideoorcamera, perform-mode starts still send the oldinput_sourcebecause theelsebranch only callsswitchMode(sourceMode).Suggested fix
if ( sourceMode === "spout" || sourceMode === "ndi" || sourceMode === "syphon" ) { @@ }); switchMode(sourceMode); } else { + updateSettings({ + inputSource: { + enabled: false, + source_type: "", + source_name: "", + }, + }); switchMode(sourceMode); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/StreamPage.tsx` around lines 1930 - 1949, When switching to non-server modes (e.g., "video" or "camera") ensure you clear any stale server-side input source so start-perform-mode won't send the old input_source: in the else branch (where currently only switchMode(sourceMode) is called) call updateSettings to set inputSource.enabled = false and clear source_type/source_name (e.g., inputSource: { enabled: false, source_type: "", source_name: "" }) before calling switchMode(sourceMode); updateSettings, switchMode, settings.inputSource, sourceMode and sourceNode are the relevant symbols to change.frontend/src/components/graph/hooks/useGraphState.ts (2)
648-649:⚠️ Potential issue | 🟠 MajorSerialize
nodeParamswith the graph config.
nodeParamslives outsidenodes, but every save/export/get-current path here still usesflowToGraphConfig(nodes, edges). That drops per-pipeline prompt text and parameter values on autosave, manual save, export, andGraphEditorHandle.getCurrentGraphConfig(), so they disappear after a refresh or round-trip.Also applies to: 678-679, 774-775, 790-791
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/hooks/useGraphState.ts` around lines 648 - 649, The current serialization uses flowToGraphConfig(nodes, edges) but omits the external nodeParams, causing per-pipeline prompts and parameter values to be lost; update the places that create graphConfig and graphJson (where flowToGraphConfig(nodes, edges) is called and assigned to graphConfig, then JSON.stringify(graphConfig)) to include nodeParams (for example by attaching nodeParams to graphConfig or by serializing an envelope { graphConfig, nodeParams }) so autosave/manual save/export and GraphEditorHandle.getCurrentGraphConfig() include nodeParams alongside the graph config.
634-639:⚠️ Potential issue | 🟠 MajorDon’t report hydrate/refresh as a user graph edit.
This effect fires after
loadGraphFromBackend()andrefreshGraph()too, not just after user mutations. Infrontend/src/pages/StreamPage.tsx, that flipsnonLinearGraphtotrue, so merely loading an existing graph disables perform-mode controls before the user changes anything.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/hooks/useGraphState.ts` around lines 634 - 639, The effect currently notifies on any graph change (including backend loads/refreshes), causing non-user changes (from loadGraphFromBackend/refreshGraph) to flip UI state; add a short-lived suppression flag to avoid reporting those non-user updates: create a ref like suppressNotifyRef used in useGraphState and check it in the effect (alongside initialLoadDone, nodes, edges, onGraphChangeRef) to early-return when true, and then set/clear this ref around calls to loadGraphFromBackend() and refreshGraph() (or when those functions mutate the state) so only true user-initiated mutations trigger onGraphChangeRef.current().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/graph/GraphEditor.tsx`:
- Around line 393-457: The icon-only stream control buttons in GraphEditor
render SVGs without reliable accessible names; update the primary toggle button
(the element using onClick={isStreaming ? onStopStream : onStartStream} and
NODE_TOKENS.toolbarButton) and the secondary stop button
(onClick={onStopStream}) to include proper accessible names by adding aria-label
attributes (e.g., dynamic aria-label on the toggle: "Start stream" when
!isStreaming and "Stop stream" when isStreaming; and a clear label like "Stop
and clear" for the secondary button) or include visually hidden text inside the
buttons for screen readers so the controls are announced correctly.
In `@frontend/src/pages/StreamPage.tsx`:
- Around line 1404-1413: handleStartStream currently fires
setGraph(frontendGraph) as a best-effort async call and then immediately
continues to loadPipeline()/startStream(), which can run against the previous
graph; change handleStartStream to await the save: call
graphEditorRef.current?.getCurrentGraphConfig() as now, and if frontendGraph
exists, await setGraph(frontendGraph) (catching/logging any error) before
calling loadPipeline() or startStream(); keep the same warning behavior but
ensure the save completes (or errors handled) so loadPipeline/startStream use
the latest graph.
---
Duplicate comments:
In `@frontend/src/components/graph/hooks/useGraphState.ts`:
- Around line 648-649: The current serialization uses flowToGraphConfig(nodes,
edges) but omits the external nodeParams, causing per-pipeline prompts and
parameter values to be lost; update the places that create graphConfig and
graphJson (where flowToGraphConfig(nodes, edges) is called and assigned to
graphConfig, then JSON.stringify(graphConfig)) to include nodeParams (for
example by attaching nodeParams to graphConfig or by serializing an envelope {
graphConfig, nodeParams }) so autosave/manual save/export and
GraphEditorHandle.getCurrentGraphConfig() include nodeParams alongside the graph
config.
- Around line 634-639: The effect currently notifies on any graph change
(including backend loads/refreshes), causing non-user changes (from
loadGraphFromBackend/refreshGraph) to flip UI state; add a short-lived
suppression flag to avoid reporting those non-user updates: create a ref like
suppressNotifyRef used in useGraphState and check it in the effect (alongside
initialLoadDone, nodes, edges, onGraphChangeRef) to early-return when true, and
then set/clear this ref around calls to loadGraphFromBackend() and
refreshGraph() (or when those functions mutate the state) so only true
user-initiated mutations trigger onGraphChangeRef.current().
In `@frontend/src/pages/StreamPage.tsx`:
- Around line 1930-1949: When switching to non-server modes (e.g., "video" or
"camera") ensure you clear any stale server-side input source so
start-perform-mode won't send the old input_source: in the else branch (where
currently only switchMode(sourceMode) is called) call updateSettings to set
inputSource.enabled = false and clear source_type/source_name (e.g.,
inputSource: { enabled: false, source_type: "", source_name: "" }) before
calling switchMode(sourceMode); updateSettings, switchMode,
settings.inputSource, sourceMode and sourceNode are the relevant symbols to
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59305282-2e83-40b4-b45a-676fe51d079c
📒 Files selected for processing (3)
frontend/src/components/graph/GraphEditor.tsxfrontend/src/components/graph/hooks/useGraphState.tsfrontend/src/pages/StreamPage.tsx
Comment on lines
+1404
to
+1413
| if (graphMode) { | ||
| const frontendGraph = | ||
| graphEditorRef.current?.getCurrentGraphConfig(); | ||
| if (frontendGraph) { | ||
| graphNodes = frontendGraph.nodes; | ||
| // Force-save to backend so load_pipeline & graph_executor | ||
| // also use the latest graph (best-effort, don't block on failure). | ||
| setGraph(frontendGraph).catch(err => | ||
| console.warn("Failed to force-save graph before stream:", err) | ||
| ); |
There was a problem hiding this comment.
Await the graph save before starting the stream.
Lines 1409-1413 intentionally make setGraph(frontendGraph) best-effort, but the rest of handleStartStream() immediately proceeds to loadPipeline()/startStream(). Clicking Play right after an edit can still execute the previously persisted graph.
Suggested fix
if (graphMode) {
const frontendGraph =
graphEditorRef.current?.getCurrentGraphConfig();
if (frontendGraph) {
graphNodes = frontendGraph.nodes;
- // Force-save to backend so load_pipeline & graph_executor
- // also use the latest graph (best-effort, don't block on failure).
- setGraph(frontendGraph).catch(err =>
- console.warn("Failed to force-save graph before stream:", err)
- );
+ // Persist before loading so the backend cannot start from a stale graph.
+ try {
+ await setGraph(frontendGraph);
+ } catch (err) {
+ console.warn("Failed to save graph before stream:", err);
+ return false;
+ }
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/StreamPage.tsx` around lines 1404 - 1413,
handleStartStream currently fires setGraph(frontendGraph) as a best-effort async
call and then immediately continues to loadPipeline()/startStream(), which can
run against the previous graph; change handleStartStream to await the save: call
graphEditorRef.current?.getCurrentGraphConfig() as now, and if frontendGraph
exists, await setGraph(frontendGraph) (catching/logging any error) before
calling loadPipeline() or startStream(); keep the same warning behavior but
ensure the save completes (or errors handled) so loadPipeline/startStream use
the latest graph.
Signed-off-by: gioelecerati <gioele@cerati.org>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
frontend/src/components/graph/hooks/useGraphState.ts (1)
702-733: Auto-save effect may fire rapidly during drag operations.The debounced auto-save effect at lines 702-733 triggers on every
nodesoredgeschange with a 500ms delay. During node dragging, React Flow firesonNodesChangefrequently, causing repeated timer resets. While the debounce prevents excessive saves, consider whethernodeParamsshould be in the dependency array — it currently is, meaning parameter edits also trigger the save timer.This is likely intentional, but verify that rapid parameter changes (e.g., dragging a slider) don't cause performance issues with repeated JSON stringification.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/graph/hooks/useGraphState.ts` around lines 702 - 733, The auto-save useEffect (debounced) is firing on every change to nodes, edges, and nodeParams which can reset the 500ms timer excessively during frequent updates like dragging; change the dependency from nodeParams to a stable ref (nodeParamsRef.current) or derive a debounced/serialized representation (e.g., useDebouncedValue or memoized JSON) so rapid slider/param changes don't constantly reset the timer, ensuring the effect still watches nodes and edges but uses nodeParamsRef (or a debounced nodeParams snapshot) when building graphConfig passed to attachNodeParams/saveGraphToLocalStorage/setGraph.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/graph/GraphEditor.tsx`:
- Around line 493-499: The hidden file input in GraphEditor.tsx (the <input>
using ref fileInputRef and onChange handler handleImport) lacks an accessible
label; add an aria-label (e.g., aria-label="Import graph JSON file") or
associate it with a visually-hidden <label> so screen readers can identify its
purpose while keeping it visually hidden, and ensure the aria text clearly
describes the expected file type and action.
- Around line 220-318: The right-click handler handleRightMouseDown attaches
window listeners (handleMove, handleUp) but never removes them if the component
unmounts mid-drag; add a cleanup to ensure those listeners are removed on
unmount by storing the active handlers/drag state in a ref (e.g.,
dragHandlersRef or isDraggingRef) and expose a cleanup function that removes
window.removeEventListener("mousemove", handleMove) and
window.removeEventListener("mouseup", handleUp) and clears selection state
(setSelectionRect(null)) and context/pending state as needed; wire that cleanup
in a useEffect return so unmount calls it, or call the stored handleUp from the
cleanup to reuse the same closing logic (ensure handleUp is referenced via ref
to avoid stale closures).
In `@frontend/src/components/graph/hooks/useGraphState.ts`:
- Around line 791-806: handleClear currently closes over callbacks.onGraphClear
but callbacks is not included in the useCallback dependency array, which
triggers an ESLint warning and risks a stale closure; update the implementation
to follow the existing ref pattern used elsewhere by referencing a stable ref
(e.g., callbacksRef) instead of callbacks directly or add callbacks (or
callbacks.onGraphClear) to the dependency array so the callback updates when the
prop changes; ensure you update the dependency list for useCallback to include
the chosen ref or callbacks symbol and read onGraphClear from that ref inside
handleClear (referencing handleClear, callbacks, onGraphClear, and useCallback
to locate the code).
In `@frontend/src/components/graph/nodes/PipelineNode.tsx`:
- Around line 312-324: The PipelineNode list-number inputs currently hardcode
min=0 and max=1000 in the NodePillInput inside values.map; change this to use
the same schema-derived bounds used by the scalar-number editor earlier in this
file: compute min and max from the parameter schema (e.g. param.schema.minimum /
param.schema.maximum or the helper used by the scalar-number editor) and pass
those computed values to NodePillInput instead of 0 and 1000 so signed or wider
list_number ranges are honored when calling onParameterChange(id, param.name,
updated).
- Around line 201-209: Interactive controls inside PipelineNode (e.g., the Reset
button using onParameterChange, the string input/enum select/toggle/textarea
variants rendered by NodePillInput) are missing the React Flow 'nodrag' class,
causing drag/selection conflicts; add the 'nodrag' class to the interactive
element containers (e.g., the button with className `${NODE_TOKENS.pill} ...`,
and the corresponding JSX elements for string input, enum select, toggle, and
textarea in NodePillInput) so these elements include 'nodrag' alongside existing
classes to prevent node dragging while interacting with them.
---
Nitpick comments:
In `@frontend/src/components/graph/hooks/useGraphState.ts`:
- Around line 702-733: The auto-save useEffect (debounced) is firing on every
change to nodes, edges, and nodeParams which can reset the 500ms timer
excessively during frequent updates like dragging; change the dependency from
nodeParams to a stable ref (nodeParamsRef.current) or derive a
debounced/serialized representation (e.g., useDebouncedValue or memoized JSON)
so rapid slider/param changes don't constantly reset the timer, ensuring the
effect still watches nodes and edges but uses nodeParamsRef (or a debounced
nodeParams snapshot) when building graphConfig passed to
attachNodeParams/saveGraphToLocalStorage/setGraph.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17fa9350-9370-485a-b925-d582e7129c8c
📒 Files selected for processing (4)
frontend/src/components/graph/GraphEditor.tsxfrontend/src/components/graph/hooks/useGraphState.tsfrontend/src/components/graph/nodes/PipelineNode.tsxfrontend/src/lib/graphUtils.ts
Comment on lines
+493
to
+499
| <input | ||
| ref={fileInputRef} | ||
| type="file" | ||
| accept=".json" | ||
| onChange={handleImport} | ||
| className="hidden" | ||
| /> |
There was a problem hiding this comment.
File input lacks accessible label.
The hidden file input at lines 493-499 has no associated label or aria-label. While visually hidden, assistive technologies may still interact with it. Consider adding an aria-label for clarity.
Suggested fix
<input
ref={fileInputRef}
type="file"
accept=".json"
onChange={handleImport}
className="hidden"
+ aria-label="Import graph JSON file"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <input | |
| ref={fileInputRef} | |
| type="file" | |
| accept=".json" | |
| onChange={handleImport} | |
| className="hidden" | |
| /> | |
| <input | |
| ref={fileInputRef} | |
| type="file" | |
| accept=".json" | |
| onChange={handleImport} | |
| className="hidden" | |
| aria-label="Import graph JSON file" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/graph/GraphEditor.tsx` around lines 493 - 499, The
hidden file input in GraphEditor.tsx (the <input> using ref fileInputRef and
onChange handler handleImport) lacks an accessible label; add an aria-label
(e.g., aria-label="Import graph JSON file") or associate it with a
visually-hidden <label> so screen readers can identify its purpose while keeping
it visually hidden, and ensure the aria text clearly describes the expected file
type and action.
Comment on lines
+791
to
+806
| // Clear graph | ||
| const handleClear = useCallback(async () => { | ||
| try { | ||
| await clearGraph(); | ||
| clearGraphFromLocalStorage(); | ||
| setNodes([]); | ||
| setEdges([]); | ||
| setGraphSource(null); | ||
| setSelectedNodeId(null); | ||
| setStatus("Graph cleared"); | ||
| callbacks.onGraphClear?.(); | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : "Unknown error"; | ||
| setStatus(`Clear failed: ${message}`); | ||
| } | ||
| }, [setNodes, setEdges, callbacks.onGraphClear]); |
There was a problem hiding this comment.
Fix ESLint warning: callbacks missing from dependency array.
The handleClear callback references callbacks.onGraphClear directly rather than through a ref, but callbacks is not in the dependency array. This could cause stale closure issues if the onGraphClear callback identity changes.
Suggested fix – use a ref pattern consistent with other callbacks
+ const onGraphClearRef = useRef(callbacks.onGraphClear);
+ onGraphClearRef.current = callbacks.onGraphClear;
const handleClear = useCallback(async () => {
try {
await clearGraph();
clearGraphFromLocalStorage();
setNodes([]);
setEdges([]);
setGraphSource(null);
setSelectedNodeId(null);
setStatus("Graph cleared");
- callbacks.onGraphClear?.();
+ onGraphClearRef.current?.();
} catch (err) {
const message = err instanceof Error ? err.message : "Unknown error";
setStatus(`Clear failed: ${message}`);
}
- }, [setNodes, setEdges, callbacks.onGraphClear]);
+ }, [setNodes, setEdges]);🧰 Tools
🪛 GitHub Check: Frontend Linting (ESLint + Prettier)
[warning] 806-806:
React Hook useCallback has a missing dependency: 'callbacks'. Either include it or remove the dependency array
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/graph/hooks/useGraphState.ts` around lines 791 - 806,
handleClear currently closes over callbacks.onGraphClear but callbacks is not
included in the useCallback dependency array, which triggers an ESLint warning
and risks a stale closure; update the implementation to follow the existing ref
pattern used elsewhere by referencing a stable ref (e.g., callbacksRef) instead
of callbacks directly or add callbacks (or callbacks.onGraphClear) to the
dependency array so the callback updates when the prop changes; ensure you
update the dependency list for useCallback to include the chosen ref or
callbacks symbol and read onGraphClear from that ref inside handleClear
(referencing handleClear, callbacks, onGraphClear, and useCallback to locate the
code).
Comment on lines
+201
to
+209
| <button | ||
| type="button" | ||
| onClick={() => onParameterChange?.(id, "reset_cache", true)} | ||
| className={`${NODE_TOKENS.pill} flex items-center justify-center gap-1 w-[110px] cursor-pointer hover:bg-[#2a2a2a] active:bg-[#333] transition-colors`} | ||
| title="Clear longlive cache to regenerate fresh frames" | ||
| > | ||
| <RotateCcw className="h-3 w-3 text-[#fafafa]" /> | ||
| <span className={NODE_TOKENS.primaryText}>Reset</span> | ||
| </button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l frontend/src/components/graph/nodes/PipelineNode.tsxRepository: daydreamlive/scope
Length of output: 118
🏁 Script executed:
cat -n frontend/src/components/graph/nodes/PipelineNode.tsx | sed -n '195,215p'Repository: daydreamlive/scope
Length of output: 1128
🏁 Script executed:
cat -n frontend/src/components/graph/nodes/PipelineNode.tsx | sed -n '240,280p'Repository: daydreamlive/scope
Length of output: 1892
🏁 Script executed:
cat -n frontend/src/components/graph/nodes/PipelineNode.tsx | sed -n '340,360p'Repository: daydreamlive/scope
Length of output: 818
🏁 Script executed:
find frontend -name "*.tsx" -o -name "*.ts" | grep -i "pill" | head -20Repository: daydreamlive/scope
Length of output: 433
🏁 Script executed:
rg "NodePillInput|NodePillSelect|NodePillToggle|NodePillTextarea" frontend/src --type tsx -lRepository: daydreamlive/scope
Length of output: 89
🏁 Script executed:
rg "NodePillInput|NodePillSelect|NodePillToggle|NodePillTextarea" frontend/src -l --type tsRepository: daydreamlive/scope
Length of output: 777
🏁 Script executed:
cat frontend/src/components/graph/ui/NodePillInput.tsxRepository: daydreamlive/scope
Length of output: 3220
🏁 Script executed:
cat frontend/src/components/graph/ui/NodePillSelect.tsxRepository: daydreamlive/scope
Length of output: 775
🏁 Script executed:
cat frontend/src/components/graph/ui/NodePillToggle.tsxRepository: daydreamlive/scope
Length of output: 680
🏁 Script executed:
cat frontend/src/components/graph/ui/NodePillTextarea.tsxRepository: daydreamlive/scope
Length of output: 1044
Add nodrag class to prevent drag gesture conflicts on interactive elements.
Only the numeric branch of NodePillInput applies React Flow's nodrag class; the string input, enum select, toggle, textarea, and reset button do not. In practice, this makes text selection, textarea resizing, and drag gestures compete with node dragging.
🛠️ Minimal fix
<button
type="button"
onClick={() => onParameterChange?.(id, "reset_cache", true)}
- className={`${NODE_TOKENS.pill} flex items-center justify-center gap-1 w-[110px] cursor-pointer hover:bg-[`#2a2a2a`] active:bg-[`#333`] transition-colors`}
+ className={`nodrag ${NODE_TOKENS.pill} flex items-center justify-center gap-1 w-[110px] cursor-pointer hover:bg-[`#2a2a2a`] active:bg-[`#333`] transition-colors`}
title="Clear longlive cache to regenerate fresh frames"
>
@@
<NodePillSelect
+ className="nodrag"
value={String(currentValue ?? "")}
onChange={val => onParameterChange?.(id, param.name, val)}
@@
<NodePillInput
type="text"
+ className="nodrag"
value={String(currentValue ?? "")}
onChange={val => onParameterChange?.(id, param.name, val)}
@@
<NodePillToggle
+ className="nodrag"
checked={Boolean(
currentValue ?? param.defaultValue ?? false
)}
@@
<NodePillTextarea
+ className="nodrag"
value={promptText}
onChange={text => onPromptChange?.(id, text)}
placeholder="Enter prompt..."Also applies to: 243-250, 252-256, 269-274, 345-349
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/graph/nodes/PipelineNode.tsx` around lines 201 - 209,
Interactive controls inside PipelineNode (e.g., the Reset button using
onParameterChange, the string input/enum select/toggle/textarea variants
rendered by NodePillInput) are missing the React Flow 'nodrag' class, causing
drag/selection conflicts; add the 'nodrag' class to the interactive element
containers (e.g., the button with className `${NODE_TOKENS.pill} ...`, and the
corresponding JSX elements for string input, enum select, toggle, and textarea
in NodePillInput) so these elements include 'nodrag' alongside existing classes
to prevent node dragging while interacting with them.
Signed-off-by: gioelecerati <gioele@cerati.org>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
Signed-off-by: gioelecerati <gioele@cerati.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Rafał Leszko rafal@livepeer.org
TODO
Summary by CodeRabbit
New Features
Chores