Browse Source

Prevent Vidu model selection from looping node updates

Vidu-style video models can load parameter schemas that change the Generate Video node height and trigger React Flow dimension updates. The fix keeps schema-driven resize notifications stable, skips no-op node height writes, and ignores duplicate React Flow node changes before they enter the workflow store.

It also preserves completed media outputs when a stopped or reloaded workflow still has transient loading/skipped state, so existing video results do not appear stuck after recovery.

Constraint: The crash was reported from a browser state where choosing Vidu hit React's maximum update depth in onNodesChange.

Rejected: Clear browser storage as the fix | it would hide persisted workflow loops without fixing the update path.

Rejected: Remove dynamic video handles | visible media/text inputs are product behavior and required for model-specific workflows.

Confidence: high

Scope-risk: moderate

Directive: Keep React Flow dimension and schema resize writes idempotent; new node resize paths should no-op when dimensions are already applied.

Tested: npm test -- --run src/components/__tests__/GenerateVideoNode.test.tsx -t 'should not resize repeatedly'

Tested: npm test -- --run src/components/__tests__/GenerateVideoNode.test.tsx src/components/__tests__/ModelParameters.test.tsx src/store/__tests__/workflowStore.integration.test.ts src/store/__tests__/skipPropagation.test.ts src/store/execution/__tests__/generateVideoExecutor.test.ts

Tested: npm run build

Tested: npm run test:run
feature/add_agent_md
jiajia 2 months ago
parent
commit
dde17e44ac
  1. 55
      src/components/__tests__/GenerateVideoNode.test.tsx
  2. 28
      src/components/nodes/GenerateVideoNode.tsx
  3. 14
      src/components/nodes/ModelParameters.tsx
  4. 26
      src/store/__tests__/skipPropagation.test.ts
  5. 69
      src/store/__tests__/workflowStore.integration.test.ts
  6. 164
      src/store/workflowStore.ts

55
src/components/__tests__/GenerateVideoNode.test.tsx

@ -819,6 +819,61 @@ describe("GenerateVideoNode", () => {
expect(screen.getByLabelText("Sound")).toBeChecked();
});
it("should not resize repeatedly when a non-sound video model schema is stable", async () => {
window.localStorage.removeItem("node-banana-schema-cache");
mockFetch.mockImplementation((url) => {
const requestUrl = String(url);
if (requestUrl.startsWith("/api/models?")) {
return Promise.resolve({
ok: true,
json: () => Promise.resolve({ models: [] }),
});
}
if (requestUrl.startsWith("/api/models/")) {
return Promise.resolve({
ok: true,
json: () => Promise.resolve({
parameters: [{ name: "motion", type: "string", label: "Motion" }],
inputs: [{ name: "prompt", type: "text", required: true, label: "Prompt" }],
}),
});
}
return Promise.resolve({
ok: true,
json: () => Promise.resolve({}),
});
});
const props = createNodeProps({
selectedModel: {
provider: "newapiwg",
modelId: "vidu-q1",
displayName: "Vidu Q1",
capabilities: ["text-to-video"],
},
});
const { rerender } = render(
<TestWrapper>
<GenerateVideoNode {...props} />
</TestWrapper>
);
await waitFor(() => {
expect(mockSetNodes.mock.calls.length).toBeGreaterThan(0);
});
const resizeCallsAfterSchemaLoad = mockSetNodes.mock.calls.length;
rerender(
<TestWrapper>
<GenerateVideoNode {...props} />
</TestWrapper>
);
await new Promise((resolve) => setTimeout(resolve, 0));
expect(mockSetNodes.mock.calls.length).toBe(resizeCallsAfterSchemaLoad);
});
it("should render ModelParameters when model is selected", async () => {
render(
<TestWrapper>

28
src/components/nodes/GenerateVideoNode.tsx

@ -39,6 +39,10 @@ import {
// Video generation capabilities
const VIDEO_CAPABILITIES: ModelCapability[] = ["text-to-video", "image-to-video", "video-to-video", "audio-to-video"];
const VIDEO_BASIC_PARAMETER_NAMES = [
...VIDEO_DURATION_PARAMETER_NAMES,
...VIDEO_RESOLUTION_PARAMETER_NAMES,
];
/** Returns true for Gemini-native Veo video models */
function isVeoModel(modelId: string | undefined): boolean {
@ -312,13 +316,16 @@ export function GenerateVideoNode({ id, data, selected }: NodeProps<GenerateVide
const baseHeight = 300; // Default node height
const newHeight = baseHeight + parameterHeight;
setNodes((nodes) =>
nodes.map((node) =>
node.id === id
? { ...node, style: { ...node.style, height: newHeight } }
: node
)
);
setNodes((nodes) => {
let changed = false;
const nextNodes = nodes.map((node) => {
if (node.id !== id) return node;
if (node.style?.height === newHeight) return node;
changed = true;
return { ...node, style: { ...node.style, height: newHeight } };
});
return changed ? nextNodes : nodes;
});
},
[id, setNodes]
);
@ -436,9 +443,10 @@ export function GenerateVideoNode({ id, data, selected }: NodeProps<GenerateVide
}, [nodeData.lastUsedModel?.displayName, nodeData.outputVideo, nodeData.selectedModel?.displayName, nodeData.selectedModel?.modelId, t]);
const displayError = formatUserFacingError(nodeData.error, t, displayTitle);
const videoSoundSupported = modelSupportsVideoSound(nodeData.selectedModel);
const hiddenVideoParameterNames = videoSoundSupported
? VIDEO_FIRST_CLASS_PARAMETER_NAMES
: [...VIDEO_DURATION_PARAMETER_NAMES, ...VIDEO_RESOLUTION_PARAMETER_NAMES];
const hiddenVideoParameterNames = useMemo(
() => videoSoundSupported ? VIDEO_FIRST_CLASS_PARAMETER_NAMES : VIDEO_BASIC_PARAMETER_NAMES,
[videoSoundSupported]
);
// Provider badge as title prefix
const titlePrefix = useMemo(() => (

14
src/components/nodes/ModelParameters.tsx

@ -85,6 +85,7 @@ function ModelParametersInner({
const [error, setError] = useState<string | null>(null);
// Use stable selector for API keys to prevent unnecessary re-fetches
const { replicateApiKey, falApiKey, kieApiKey, wavespeedApiKey } = useProviderApiKeys();
const lastExpandCountRef = useRef<number | null>(null);
// Fetch schema when modelId changes
useEffect(() => {
@ -163,6 +164,7 @@ function ModelParametersInner({
() => schema.filter((param) => !hiddenParameterSet.has(param.name)),
[hiddenParameterSet, schema]
);
const visibleParameterCount = visibleSchema.length;
// Remove parameters controlled by a higher-priority surface, such as the bottom composer.
useEffect(() => {
@ -202,10 +204,16 @@ function ModelParametersInner({
// Notify parent to resize node when schema loads
useEffect(() => {
if (visibleSchema.length > 0 && onExpandChange) {
onExpandChange(true, visibleSchema.length);
if (visibleParameterCount === 0) {
lastExpandCountRef.current = null;
return;
}
if (lastExpandCountRef.current === visibleParameterCount) return;
lastExpandCountRef.current = visibleParameterCount;
if (visibleParameterCount > 0 && onExpandChange) {
onExpandChange(true, visibleParameterCount);
}
}, [visibleSchema, onExpandChange]);
}, [visibleParameterCount, onExpandChange]);
const handleParameterChange = useCallback(
(name: string, value: unknown) => {

26
src/store/__tests__/skipPropagation.test.ts

@ -438,6 +438,32 @@ describe("Skip propagation", () => {
expect(useWorkflowStore.getState().skippedNodeIds.size).toBe(0);
});
it("should clear loading status on stopped nodes with existing video output", () => {
useWorkflowStore.setState({
nodes: [
createTestNode("video-1", "generateVideo", {
inputImages: [],
inputPrompt: "happy fuji",
outputVideo: "https://example.com/fuji.mp4",
outputVideoStorageStatus: "remote-only",
status: "loading",
error: null,
videoHistory: [],
selectedVideoHistoryIndex: 0,
}),
],
currentNodeIds: ["video-1"],
isRunning: true,
});
const store = useWorkflowStore.getState();
store.stopWorkflow();
const node = useWorkflowStore.getState().nodes[0];
expect(node.data.status).toBe("complete");
expect(node.data.outputVideo).toBe("https://example.com/fuji.mp4");
});
});
describe("Multi-level cascade", () => {

69
src/store/__tests__/workflowStore.integration.test.ts

@ -98,6 +98,43 @@ describe("workflowStore integration tests", () => {
resetStore();
});
describe("onNodesChange no-op guards", () => {
it("should ignore duplicate measured dimensions from React Flow", () => {
const node = {
...createTestNode("video-1", "generateVideo", {}),
measured: { width: 360, height: 300 },
} as WorkflowNode;
const nodes = [node];
useWorkflowStore.setState({ nodes, hasUnsavedChanges: false });
useWorkflowStore.getState().onNodesChange([
{ id: "video-1", type: "dimensions", dimensions: { width: 360, height: 300 } },
]);
expect(useWorkflowStore.getState().nodes).toBe(nodes);
expect(useWorkflowStore.getState().hasUnsavedChanges).toBe(false);
});
it("should apply dimensions when the measured value changes", () => {
useWorkflowStore.setState({
nodes: [
{
...createTestNode("video-1", "generateVideo", {}),
measured: { width: 360, height: 300 },
} as WorkflowNode,
],
hasUnsavedChanges: false,
});
useWorkflowStore.getState().onNodesChange([
{ id: "video-1", type: "dimensions", dimensions: { width: 420, height: 300 } },
]);
expect(useWorkflowStore.getState().nodes[0].measured?.width).toBe(420);
expect(useWorkflowStore.getState().hasUnsavedChanges).toBe(false);
});
});
describe("getConnectedInputs", () => {
describe("Basic data extraction scenarios", () => {
it("should extract image from imageInput node", () => {
@ -2298,15 +2335,45 @@ describe("workflowStore integration tests", () => {
const store = useWorkflowStore.getState();
await store.loadWorkflow({
version: 1,
id: "test-workflow",
name: "Test",
nodes: [],
edges: [],
edgeStyle: "angular",
});
expect(useWorkflowStore.getState().viewedCommentNodeIds.size).toBe(0);
});
it("should clear transient loading status when loading a workflow with an existing video output", async () => {
const store = useWorkflowStore.getState();
await store.loadWorkflow({
version: 1,
id: "test-workflow",
name: "Happy Fuji",
nodes: [
createTestNode("video-1", "generateVideo", {
inputImages: [],
inputPrompt: "happy fuji",
outputVideo: "https://example.com/fuji.mp4",
outputVideoStorageStatus: "remote-only",
status: "loading",
error: null,
videoHistory: [],
selectedVideoHistoryIndex: 0,
}),
],
edges: [],
edgeStyle: "angular",
});
const node = useWorkflowStore.getState().nodes[0];
expect(node.data.status).toBe("complete");
expect(node.data.outputVideo).toBe("https://example.com/fuji.mp4");
expect(node.data.outputVideoStorageStatus).toBe("remote-only");
});
it("should promote a NewApiWG fallback when env status is known before workflow load", async () => {
useWorkflowStore.setState({
providerSettings: defaultProviderSettings,
@ -2324,6 +2391,7 @@ describe("workflowStore integration tests", () => {
const store = useWorkflowStore.getState();
await store.loadWorkflow({
version: 1,
id: "test-workflow",
name: "Test",
nodes: [
@ -2347,6 +2415,7 @@ describe("workflowStore integration tests", () => {
}),
],
edges: [],
edgeStyle: "angular",
});
const node = useWorkflowStore.getState().nodes[0];

164
src/store/workflowStore.ts

@ -596,6 +596,134 @@ function promoteUnavailableGeminiFallbacks(
return { nodes: nextNodes, changed };
}
const NODE_OUTPUT_KEYS = [
"outputImage",
"outputImageRef",
"outputVideo",
"outputVideoRef",
"outputVideoRemoteUrl",
"outputAudio",
"outputAudioRef",
"outputText",
"output3dUrl",
"savedFilePath",
"capturedImage",
"capturedImageRef",
"image",
"imageRef",
"video",
"videoRef",
"audio",
"model3d",
"glbUrl",
"images",
"imageRefs",
"videos",
"videoRefs",
"outputItems",
] as const;
function hasUsableOutputValue(value: unknown): boolean {
if (value === null || value === undefined) return false;
if (typeof value === "string") {
const trimmed = value.trim();
return trimmed.length > 0 && trimmed !== "[]";
}
if (Array.isArray(value)) return value.some(hasUsableOutputValue);
return true;
}
function hasNodeOutput(data: WorkflowNodeData): boolean {
const record = data as Record<string, unknown>;
return NODE_OUTPUT_KEYS.some((key) => hasUsableOutputValue(record[key]));
}
function resetTransientExecutionState(nodes: WorkflowNode[]): { nodes: WorkflowNode[]; changed: boolean } {
let changed = false;
const nextNodes = nodes.map((node) => {
const data = node.data as WorkflowNodeData;
const status = (data as Record<string, unknown>).status;
if (status !== "loading" && status !== "skipped") return node;
changed = true;
return {
...node,
data: {
...data,
status: hasNodeOutput(data) ? "complete" : "idle",
error: null,
} as WorkflowNodeData,
} as WorkflowNode;
});
return { nodes: nextNodes as WorkflowNode[], changed };
}
function numbersMatch(a: unknown, b: unknown): boolean {
return typeof a === "number" && typeof b === "number" && Math.abs(a - b) < 0.5;
}
function positionMatches(a: XYPosition | undefined, b: XYPosition | undefined): boolean {
return !!a && !!b && numbersMatch(a.x, b.x) && numbersMatch(a.y, b.y);
}
function isDimensionValueEffective(
node: WorkflowNode,
axis: "width" | "height",
value: number | undefined,
setAttributes: boolean | "width" | "height" | undefined,
): boolean {
if (value === undefined) return false;
if (!numbersMatch(node.measured?.[axis], value)) {
return true;
}
if (
setAttributes === true ||
setAttributes === axis
) {
return !numbersMatch(node[axis], value);
}
return false;
}
function isNodeChangeEffective(
change: NodeChange<WorkflowNode>,
nodeById: Map<string, WorkflowNode>,
): boolean {
switch (change.type) {
case "add":
case "replace":
return true;
case "remove":
return nodeById.has(change.id);
case "select": {
const node = nodeById.get(change.id);
return !!node && node.selected !== change.selected;
}
case "position": {
const node = nodeById.get(change.id);
if (!node) return false;
const positionChanged = change.position !== undefined && !positionMatches(node.position, change.position);
const draggingChanged = typeof change.dragging === "boolean" && node.dragging !== change.dragging;
return positionChanged || draggingChanged;
}
case "dimensions": {
const node = nodeById.get(change.id);
if (!node) return false;
const widthChanged = isDimensionValueEffective(node, "width", change.dimensions?.width, change.setAttributes);
const heightChanged = isDimensionValueEffective(node, "height", change.dimensions?.height, change.setAttributes);
const resizing = (node as WorkflowNode & { resizing?: boolean }).resizing;
const resizingChanged = typeof change.resizing === "boolean" && resizing !== change.resizing;
return widthChanged || heightChanged || resizingChanged;
}
default:
return true;
}
}
const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
nodes: [],
edges: [],
@ -829,18 +957,22 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
},
onNodesChange: (changes: NodeChange<WorkflowNode>[]) => {
const nodeById = new Map(get().nodes.map((node) => [node.id, node]));
const effectiveChanges = changes.filter((change) => isNodeChangeEffective(change, nodeById));
if (effectiveChanges.length === 0) return;
// Only mark as unsaved for meaningful changes (not selection changes)
const hasMeaningfulChange = changes.some(
const hasMeaningfulChange = effectiveChanges.some(
(c) => c.type !== "select" && c.type !== "dimensions"
);
// Track manual changes only for remove operations (not position/selection/dimensions)
const hasRemoveChange = changes.some((c) => c.type === "remove");
const hasRemoveChange = effectiveChanges.some((c) => c.type === "remove");
// Undo: capture snapshot on drag start
const hasDragStart = changes.some(
const hasDragStart = effectiveChanges.some(
(c) => c.type === "position" && (c as { dragging?: boolean }).dragging === true
);
const hasDragEnd = changes.some(
const hasDragEnd = effectiveChanges.some(
(c) => c.type === "position" && (c as { dragging?: boolean }).dragging === false
);
if (hasDragStart && !isDragging) {
@ -861,7 +993,7 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
}
set((state) => ({
nodes: applyNodeChanges(changes, state.nodes),
nodes: applyNodeChanges(effectiveChanges, state.nodes),
...(hasMeaningfulChange ? { hasUnsavedChanges: true } : {}),
}));
@ -1724,7 +1856,17 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
if (controller) {
controller.abort("user-cancelled");
}
set({ isRunning: false, currentNodeIds: [], skippedNodeIds: new Set(), _abortController: null });
set((state) => {
const reset = resetTransientExecutionState(state.nodes);
return {
nodes: reset.nodes,
isRunning: false,
currentNodeIds: [],
skippedNodeIds: new Set(),
_abortController: null,
hasUnsavedChanges: state.hasUnsavedChanges || reset.changed,
};
});
},
mockTutorialExecution: async () => {
@ -2315,6 +2457,14 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
};
}
const restoredExecutionState = resetTransientExecutionState(hydratedWorkflow.nodes as WorkflowNode[]);
if (restoredExecutionState.changed) {
hydratedWorkflow = {
...hydratedWorkflow,
nodes: restoredExecutionState.nodes,
};
}
// Load cost data for this workflow
const costData = workflow.id ? loadWorkflowCostData(workflow.id) : null;
@ -2343,7 +2493,6 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
? joinBrowserFileSystemPath(directoryPath, "generations")
: null),
lastSavedAt: savedConfig?.lastSavedAt || null,
hasUnsavedChanges: promotedModels.changed,
// Restore cost data
incurredCost: costData?.incurredCost || 0,
// Track where imageRefs are valid from
@ -2354,6 +2503,7 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
viewedCommentNodeIds: new Set<string>(),
// Dismiss welcome modal after loading a workflow
showQuickstart: false,
hasUnsavedChanges: promotedModels.changed || restoredExecutionState.changed,
});
// Clear snapshot unless explicitly preserving (e.g., AI workflow generation)

Loading…
Cancel
Save