Browse Source

fix: address 16 code review findings across codebase

- Restore FTUX production logic (remove testing override)
- Fix generate3dExecutor clearOutput field and add modelId validation
- Remap startLevel index after loop partitioning in executeWorkflow
- Add missing mockTutorialExecution dep in FloatingActionBar
- Reset settingsTab to "primary" when fallbackModel is removed
- Clear orphaned fallbackParameters when removing fallback model
- Fix OutputGalleryNode removeMedia to splice single item, not all duplicates
- Add data-tutorial="save-button" to untitled save button in Header
- Move data-tutorial to layout-producing element in BaseNode
- Hydrate FTUXApiKeysStep localKeys from providerSettings
- Remove broken docs URL from ftuxStore tutorial steps
- Add error handling in loadTutorialSampleImage
- Fix mediaStorage ref array misalignment in outputGallery hydration
- Make OutputGalleryNodeData.videos optional to match usage
- Remove unused fitView destructuring from WorkflowCanvas

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
handoff-20260429-1057
shrimbly 3 months ago
parent
commit
da9d25f5f4
  1. 6
      src/app/page.tsx
  2. 2
      src/components/FloatingActionBar.tsx
  3. 1
      src/components/Header.tsx
  4. 4
      src/components/WorkflowCanvas.tsx
  5. 3
      src/components/nodes/BaseNode.tsx
  6. 6
      src/components/nodes/GenerateImageNode.tsx
  7. 7
      src/components/nodes/GenerateVideoNode.tsx
  8. 2
      src/components/nodes/LLMFallbackPopover.tsx
  9. 8
      src/components/nodes/LLMGenerateNode.tsx
  10. 19
      src/components/nodes/OutputGalleryNode.tsx
  11. 24
      src/components/onboarding/FTUXApiKeysStep.tsx
  12. 7
      src/store/execution/generate3dExecutor.ts
  13. 11
      src/store/ftuxStore.ts
  14. 9
      src/store/workflowStore.ts
  15. 2
      src/types/nodes.ts
  16. 6
      src/utils/mediaStorage.ts

6
src/app/page.tsx

@ -36,9 +36,9 @@ export default function Home() {
// Client-side only FTUX check (SSR-safe)
useEffect(() => {
// TODO: REVERT BEFORE PRODUCTION - Testing override to always show FTUX
setShowFTUX(true);
// Production code: if (!getFTUXCompleted()) { setShowFTUX(true); }
if (!getFTUXCompleted()) {
setShowFTUX(true);
}
}, []);
const handleFTUXComplete = () => {

2
src/components/FloatingActionBar.tsx

@ -457,7 +457,7 @@ export function FloatingActionBar() {
// Normal execution
executeWorkflow();
}
}, [isRunning, stopWorkflow, executeWorkflow]);
}, [isRunning, stopWorkflow, executeWorkflow, mockTutorialExecution]);
const handleRunFromSelected = () => {
if (selectedNode) {

1
src/components/Header.tsx

@ -312,6 +312,7 @@ export function Header() {
onClick={handleNewProject}
className="relative p-1.5 text-neutral-400 hover:text-neutral-200 hover:bg-neutral-800 rounded transition-colors"
title="Save project"
data-tutorial="save-button"
>
<svg
className="w-4 h-4"

4
src/components/WorkflowCanvas.tsx

@ -316,7 +316,7 @@ export function WorkflowCanvas() {
const clearWorkflow = useWorkflowStore((state) => state.clearWorkflow);
const setHoveredNodeId = useWorkflowStore((state) => state.setHoveredNodeId);
const openAnnotationModal = useAnnotationStore((state) => state.openModal);
const { screenToFlowPosition, getViewport, zoomIn, zoomOut, setViewport, setCenter, fitView } = useReactFlow();
const { screenToFlowPosition, getViewport, zoomIn, zoomOut, setViewport, setCenter } = useReactFlow();
const { show: showToast } = useToast();
const [isDragOver, setIsDragOver] = useState(false);
const [dropType, setDropType] = useState<"image" | "audio" | "workflow" | "node" | null>(null);
@ -2488,7 +2488,7 @@ export function WorkflowCanvas() {
initialCapabilityFilter={fallbackDialogState.capability}
showClearOption
onClearSelection={() => {
updateNodeData(fallbackDialogState.nodeId, { fallbackModel: undefined });
updateNodeData(fallbackDialogState.nodeId, { fallbackModel: undefined, fallbackParameters: undefined });
setFallbackDialogState(null);
}}
onModelSelected={(model) => {

3
src/components/nodes/BaseNode.tsx

@ -293,7 +293,7 @@ export function BaseNode({
? `relative flex flex-col w-full h-full overflow-visible bg-neutral-800 rounded-lg ${selected ? "ring-2 ring-blue-500/40 shadow-lg shadow-blue-500/25" : ""}`
: "contents"}
onDoubleClick={handleResizeHandleDblClick}
data-tutorial={dataTutorial}
data-tutorial={hasExpandedSettings ? dataTutorial : undefined}
>
<NodeResizer
isVisible={selected}
@ -316,6 +316,7 @@ export function BaseNode({
${!fullBleed && selected && settingsExpanded ? "border-blue-500" : ""}
${className}
`}
data-tutorial={!hasExpandedSettings ? dataTutorial : undefined}
onMouseEnter={(e) => {
if (e.buttons !== 0 || isPanningRef.current || isDraggingNodeRef.current) return;
setHoveredNodeId(id);

6
src/components/nodes/GenerateImageNode.tsx

@ -68,6 +68,12 @@ export function GenerateImageNode({ id, data, selected }: NodeProps<NanoBananaNo
const [isBrowseDialogOpen, setIsBrowseDialogOpen] = useState(false);
const [settingsTab, setSettingsTab] = useState<"primary" | "fallback">("primary");
useEffect(() => {
if (!nodeData.fallbackModel && settingsTab === "fallback") {
setSettingsTab("primary");
}
}, [nodeData.fallbackModel, settingsTab]);
// Inline parameters infrastructure
const { inlineParametersEnabled } = useInlineParameters();

7
src/components/nodes/GenerateVideoNode.tsx

@ -56,6 +56,13 @@ export function GenerateVideoNode({ id, data, selected }: NodeProps<GenerateVide
const [isBrowseDialogOpen, setIsBrowseDialogOpen] = useState(false);
const [isLoadingCarouselVideo, setIsLoadingCarouselVideo] = useState(false);
const [settingsTab, setSettingsTab] = useState<"primary" | "fallback">("primary");
useEffect(() => {
if (!nodeData.fallbackModel && settingsTab === "fallback") {
setSettingsTab("primary");
}
}, [nodeData.fallbackModel, settingsTab]);
const videoBlobUrl = useVideoBlobUrl(nodeData.outputVideo ?? null);
const videoAutoplayRef = useVideoAutoplay(id, selected);

2
src/components/nodes/LLMFallbackPopover.tsx

@ -88,7 +88,7 @@ export function LLMFallbackPopover({ nodeId, onClose }: LLMFallbackPopoverProps)
};
const handleRemove = () => {
updateNodeData(nodeId, { fallbackModel: undefined });
updateNodeData(nodeId, { fallbackModel: undefined, fallbackParameters: undefined });
onClose();
};

8
src/components/nodes/LLMGenerateNode.tsx

@ -1,6 +1,6 @@
"use client";
import { useCallback, useState } from "react";
import { useCallback, useEffect, useState } from "react";
import { Handle, Position, NodeProps, Node } from "@xyflow/react";
import { BaseNode } from "./BaseNode";
import { useWorkflowStore } from "@/store/workflowStore";
@ -57,6 +57,12 @@ export function LLMGenerateNode({ id, data, selected }: NodeProps<LLMGenerateNod
const [copied, setCopied] = useState(false);
const [settingsTab, setSettingsTab] = useState<"primary" | "fallback">("primary");
useEffect(() => {
if (!nodeData.fallbackModel && settingsTab === "fallback") {
setSettingsTab("primary");
}
}, [nodeData.fallbackModel, settingsTab]);
const handleCopyOutput = useCallback(async () => {
if (nodeData.outputText) {
try {

19
src/components/nodes/OutputGalleryNode.tsx

@ -82,19 +82,24 @@ export function OutputGalleryNode({ id, data, selected }: NodeProps<OutputGaller
if (!item) return;
if (item.type === "image") {
const filtered = (nodeData.images || []).filter((img) => img !== item.src);
updateNodeData(id, { images: filtered });
const images = [...(nodeData.images || [])];
const imgIndex = images.indexOf(item.src);
if (imgIndex !== -1) images.splice(imgIndex, 1);
updateNodeData(id, { images });
} else {
const filtered = (nodeData.videos || []).filter((vid) => vid !== item.src);
updateNodeData(id, { videos: filtered });
const videos = [...(nodeData.videos || [])];
const vidIndex = videos.indexOf(item.src);
if (vidIndex !== -1) videos.splice(vidIndex, 1);
updateNodeData(id, { videos });
}
// Adjust lightbox after removal
if (lightboxIndex !== null) {
if (displayMedia.length <= 1) {
const newLength = displayMedia.length - 1;
if (newLength <= 0) {
setLightboxIndex(null);
} else if (lightboxIndex >= displayMedia.length - 1) {
setLightboxIndex(displayMedia.length - 2);
} else if (lightboxIndex >= newLength) {
setLightboxIndex(newLength - 1);
}
}
}, [displayMedia, nodeData.images, nodeData.videos, updateNodeData, id, lightboxIndex]);

24
src/components/onboarding/FTUXApiKeysStep.tsx

@ -75,6 +75,7 @@ const providers: ProviderInfo[] = [
export function FTUXApiKeysStep({}: FTUXStepProps) {
const updateProviderApiKey = useWorkflowStore((state) => state.updateProviderApiKey);
const providerSettings = useWorkflowStore((state) => state.providerSettings);
const [envStatus, setEnvStatus] = useState<EnvStatusResponse | null>(null);
const [showKey, setShowKey] = useState<Record<ProviderType, boolean>>({
gemini: false,
@ -85,14 +86,21 @@ export function FTUXApiKeysStep({}: FTUXStepProps) {
kie: false,
wavespeed: false,
});
const [localKeys, setLocalKeys] = useState<Record<ProviderType, string>>({
gemini: "",
openai: "",
anthropic: "",
replicate: "",
fal: "",
kie: "",
wavespeed: "",
const [localKeys, setLocalKeys] = useState<Record<ProviderType, string>>(() => {
const keys: Record<ProviderType, string> = {
gemini: "",
openai: "",
anthropic: "",
replicate: "",
fal: "",
kie: "",
wavespeed: "",
};
for (const id of Object.keys(keys) as ProviderType[]) {
const saved = providerSettings.providers[id]?.apiKey;
if (saved) keys[id] = saved;
}
return keys;
});
useEffect(() => {

7
src/store/execution/generate3dExecutor.ts

@ -184,6 +184,11 @@ export async function executeGenerate3D(
displayName: "",
};
if (!primaryModel.modelId) {
updateNodeData(node.id, { status: "error", error: "No model selected" });
throw new Error("No model selected");
}
await runWithFallback({
nodeId: node.id,
primary: primaryModel,
@ -191,6 +196,6 @@ export async function executeGenerate3D(
fallbackParameters: nodeData.fallbackParameters,
updateNodeData,
runOnce,
clearOutput: { outputVideo: null },
clearOutput: { output3dUrl: null },
});
}

11
src/store/ftuxStore.ts

@ -212,7 +212,6 @@ const initialTutorialSteps: TutorialStep[] = [
waitForClick: true,
links: [
{ text: "Join our Discord community", url: "https://discord.gg/node-banana" },
{ text: "View documentation", url: "https://docs.nodebanana.com" },
],
completed: false,
},
@ -240,15 +239,25 @@ export const useFTUXStore = create<FTUXState>((set, get) => ({
loadTutorialSampleImage: async () => {
try {
const response = await fetch("/sample-images/owl.jpg");
if (!response.ok) {
console.error("Failed to load tutorial sample image: HTTP", response.status);
set({ tutorialSampleImage: null });
return;
}
const blob = await response.blob();
const reader = new FileReader();
reader.onloadend = () => {
const base64 = reader.result as string;
set({ tutorialSampleImage: base64 });
};
reader.onerror = () => {
console.error("Failed to read tutorial sample image as data URL");
set({ tutorialSampleImage: null });
};
reader.readAsDataURL(blob);
} catch (error) {
console.error("Failed to load tutorial sample image:", error);
set({ tutorialSampleImage: null });
}
},

9
src/store/workflowStore.ts

@ -1534,9 +1534,16 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
}
}
// Remap startLevel to prefixLevels index (original index may not match after partitioning)
let prefixStartLevel = 0;
if (startFromNodeId) {
const idx = prefixLevels.findIndex((l) => l.nodeIds.includes(startFromNodeId));
prefixStartLevel = idx !== -1 ? idx : 0;
}
// Execute prefix once
logger.info('node.execution', 'Executing prefix levels', { count: prefixLevels.length });
await executeLevels(prefixLevels, startLevel);
await executeLevels(prefixLevels, prefixStartLevel);
// Execute loop N times
const loopCount = loopEdge.data?.loopCount ?? 3;

2
src/types/nodes.ts

@ -334,7 +334,7 @@ export interface OutputNodeData extends BaseNodeData {
export interface OutputGalleryNodeData extends BaseNodeData {
images: string[]; // Array of base64 data URLs from connected nodes
imageRefs?: string[]; // External storage refs for images
videos: string[]; // Array of video URLs from connected nodes
videos?: string[]; // Array of video URLs from connected nodes
videoRefs?: string[]; // External storage refs for videos
}

6
src/utils/mediaStorage.ts

@ -455,7 +455,7 @@ async function externalizeNodeMedia(
// Externalize gallery videos
for (let i = 0; i < (d.videos?.length || 0); i++) {
const vid = d.videos[i];
const vid = d.videos![i];
const existingRef = galleryVideoRefs[i];
if (existingRef && isDataUrl(vid)) {
// Already has ref, just keep it
@ -1084,9 +1084,9 @@ async function hydrateNodeMedia(
newData = {
...d,
images: filteredImages,
imageRefs: filteredImageRefs.some(r => r !== "") ? filteredImageRefs : d.imageRefs,
imageRefs: filteredImageRefs,
videos: filteredVideos,
videoRefs: filteredVideoRefs.some(r => r !== "") ? filteredVideoRefs : d.videoRefs,
videoRefs: filteredVideoRefs,
};
break;
}

Loading…
Cancel
Save