Browse Source

fix: address code review findings from CodeRabbit

- Remove duplicate idle-state tests in GenerateImageNode and GenerateVideoNode
- Fix misleading "Text target labels" comment to "Text source labels"
- Use object-contain on AnnotationNode so full image is visible for annotation
- Add abort signal checks to Router/Switch/ConditionalSwitch executors

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
handoff-20260429-1057
shrimbly 4 months ago
parent
commit
04b8821dfa
  1. 2
      src/components/__tests__/ConnectionDropMenu.test.tsx
  2. 9
      src/components/__tests__/GenerateImageNode.test.tsx
  3. 9
      src/components/__tests__/GenerateVideoNode.test.tsx
  4. 2
      src/components/nodes/AnnotationNode.tsx
  5. 12
      src/store/execution/simpleNodeExecutors.ts

2
src/components/__tests__/ConnectionDropMenu.test.tsx

@ -245,7 +245,7 @@ describe("ConnectionDropMenu", () => {
it("should wrap around when navigating past last item", () => {
render(<ConnectionDropMenu {...defaultProps} handleType="text" connectionType="source" />);
// Text target labels: Prompt, Prompt Constructor, Array, Generate Image, Generate Video, Generate Audio, LLM Generate, Router, Switch, Conditional Switch (10 items)
// Text source labels: Prompt, Prompt Constructor, Array, Generate Image, Generate Video, Generate Audio, LLM Generate, Router, Switch, Conditional Switch (10 items)
// Navigate down 10 times to wrap to first
for (let i = 0; i < 10; i++) {
fireEvent.keyDown(document, { key: "ArrowDown" });

9
src/components/__tests__/GenerateImageNode.test.tsx

@ -237,15 +237,6 @@ describe("GenerateImageNode", () => {
expect(screen.getByText("Run to generate")).toBeInTheDocument();
});
it("should render empty state placeholder when no output image", () => {
render(
<TestWrapper>
<GenerateImageNode {...createNodeProps({ status: "idle", outputImage: null })} />
</TestWrapper>
);
expect(screen.getByText("Run to generate")).toBeInTheDocument();
});
});
describe("Loading State", () => {

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

@ -307,15 +307,6 @@ describe("GenerateVideoNode", () => {
expect(screen.getByText("Run to generate")).toBeInTheDocument();
});
it("should render empty state placeholder when no output video", () => {
render(
<TestWrapper>
<GenerateVideoNode {...createNodeProps({ status: "idle", outputVideo: null })} />
</TestWrapper>
);
expect(screen.getByText("Run to generate")).toBeInTheDocument();
});
});
describe("Loading State", () => {

2
src/components/nodes/AnnotationNode.tsx

@ -126,7 +126,7 @@ export function AnnotationNode({ id, data, selected }: NodeProps<AnnotationNodeT
<img
src={displayImage}
alt="Annotated"
className="w-full h-full object-cover"
className="w-full h-full object-contain"
/>
<button
onClick={(e) => {

12
src/store/execution/simpleNodeExecutors.ts

@ -323,7 +323,9 @@ export async function executeRouter(ctx: NodeExecutionContext): Promise<void> {
// Brief status flash to show execution occurred.
ctx.updateNodeData(ctx.node.id, { status: "loading" });
await new Promise(resolve => setTimeout(resolve, 50));
ctx.updateNodeData(ctx.node.id, { status: "complete" });
if (!ctx.signal?.aborted) {
ctx.updateNodeData(ctx.node.id, { status: "complete" });
}
}
/**
@ -334,7 +336,9 @@ export async function executeSwitch(ctx: NodeExecutionContext): Promise<void> {
// Disabled outputs are filtered during traversal.
ctx.updateNodeData(ctx.node.id, { status: "loading" });
await new Promise(resolve => setTimeout(resolve, 50));
ctx.updateNodeData(ctx.node.id, { status: "complete" });
if (!ctx.signal?.aborted) {
ctx.updateNodeData(ctx.node.id, { status: "complete" });
}
}
/**
@ -345,7 +349,9 @@ export async function executeConditionalSwitch(ctx: NodeExecutionContext): Promi
// Brief status flash to show execution occurred.
ctx.updateNodeData(ctx.node.id, { status: "loading" });
await new Promise(resolve => setTimeout(resolve, 50));
ctx.updateNodeData(ctx.node.id, { status: "complete" });
if (!ctx.signal?.aborted) {
ctx.updateNodeData(ctx.node.id, { status: "complete" });
}
}
/**

Loading…
Cancel
Save