Skip to content

Add FlowIconText component for UI v2 migration#9

Open
tomerqodo wants to merge 3 commits intoclaude_claude_vs_qodo_base_add_flowicontext_component_for_ui_v2_migration_pr9from
claude_claude_vs_qodo_head_add_flowicontext_component_for_ui_v2_migration_pr9
Open

Add FlowIconText component for UI v2 migration#9
tomerqodo wants to merge 3 commits intoclaude_claude_vs_qodo_base_add_flowicontext_component_for_ui_v2_migration_pr9from
claude_claude_vs_qodo_head_add_flowicontext_component_for_ui_v2_migration_pr9

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#9

devin-ai-integration Bot and others added 3 commits January 25, 2026 12:10
Create a new FlowIconText component that displays a flow's icon and name
as a clickable link, fetching flow data by ID. This component follows the
existing Suspense pattern used in similar link components like FlowLink
and DeploymentLink.

Files added:
- flow-icon-text.tsx: Main component with Suspense wrapper
- flow-icon-text.stories.tsx: Storybook stories
- flow-icon-text.test.tsx: Unit tests
- index.ts: Export file

Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>
Add RouterProvider decorator to the Storybook story so the component
renders correctly in Storybook. The Link component from TanStack Router
requires a router context to function properly.

Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>
Comment on lines +20 to +49
const createTestRouter = (flowId: string) => {
const rootRoute = createRootRoute({
component: () => (
<Suspense fallback={<div>Loading...</div>}>
<FlowIconText flowId={flowId} />
</Suspense>
),
});

return createRouter({
routeTree: rootRoute,
history: createMemoryHistory({ initialEntries: ["/"] }),
context: { queryClient: new QueryClient() },
});
};

const meta: Meta<typeof FlowIconText> = {
title: "Components/Flows/FlowIconText",
component: FlowIconText,
decorators: [
(_Story, context) => {
const router = createTestRouter(context.args.flowId ?? "flow-123");
return (
<QueryClientProvider client={queryClient}>
<RouterProvider router={router} />
</QueryClientProvider>
);
},
],
parameters: {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 The story bypasses the standard reactQueryDecorator and routerDecorator from @/storybook/utils, manually creates two separate QueryClient instances (line 12 with retry:false, line 32 with defaults), and lacks MSW handlers for the useSuspenseQuery(buildFLowDetailsQuery(...)) call. Without MSW mocking, the story will hang in suspense or error out since the API query never resolves. Use the standard decorators and add parameters: { msw: { handlers: [...] } } with mock flow data, which also eliminates the duplicate QueryClient problem.

Extended reasoning...

What the bug is

The FlowIconText story has three interrelated issues: (1) it manually sets up QueryClientProvider and RouterProvider instead of using the standard reactQueryDecorator and routerDecorator from @/storybook/utils, (2) it creates two separate QueryClient instances with different configurations, and (3) it does not include MSW handlers to mock the API call made by useSuspenseQuery(buildFLowDetailsQuery(flowId)).

The code path that triggers the problem

When Storybook renders the Default story, the decorator calls createTestRouter("flow-123"), which creates a router whose root component renders <FlowIconText flowId="flow-123" />. The FlowIconTextImplementation inside calls useSuspenseQuery(buildFLowDetailsQuery("flow-123")), which initiates an API request. Since no MSW handlers are registered in the story’s parameters, there is nothing to intercept and mock this request. The suspense boundary will never resolve, leaving the story stuck on a loading state or erroring out.

Step-by-step proof

  1. Storybook loads the Default story with flowId: "flow-123".
  2. The decorator creates a QueryClient at module scope (line 12, retry: false) and wraps the router in QueryClientProvider.
  3. createTestRouter on line 32 creates a second QueryClient with default config and passes it to the router context.
  4. The FlowIconText component renders, hitting useSuspenseQuery(buildFLowDetailsQuery("flow-123")) on line 21 of flow-icon-text.tsx.
  5. The query attempts a fetch. No MSW handler intercepts it. The request fails or hangs.
  6. Because useSuspenseQuery is used, the component suspends indefinitely or throws, and the story never renders the actual FlowIconText output.

Convention violations

Both ui-v2/src/components/AGENTS.md and ui-v2/src/storybook/AGENTS.md mandate using reactQueryDecorator and routerDecorator (pattern: decorators: [reactQueryDecorator, routerDecorator]) and MSW handlers for API mocking (parameters: { msw: { handlers: [...] } }). Every other story in the flows/ directory follows this pattern (e.g., flow-page-header.stories.tsx, delete-flow-dialog.stories.tsx, flow-menu.stories.tsx, flow-stats-summary/index.stories.tsx).

Impact and fix

The story is non-functional as written — it will never render the FlowIconText component. The fix is straightforward: replace the manual decorator with decorators: [reactQueryDecorator, routerDecorator], remove the manual QueryClient / router creation, and add parameters: { msw: { handlers: [http.get("/api/flows/:id", () => HttpResponse.json(mockFlowData))] } } with appropriate mock data. This also eliminates the duplicate QueryClient issue since the standard reactQueryDecorator manages a single client internally.

Comment on lines +6 to +17
import { Skeleton } from "@/components/ui/skeleton";

type FlowIconTextProps = {
flowId: string;
};

export const FlowIconText = ({ flowId }: FlowIconTextProps) => {
return (
<Suspense>
<FlowIconTextImplementation flowId={flowId} />
</Suspense>
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 <Suspense> on line 14 is missing a fallback prop, so nothing renders while useSuspenseQuery loads (blank space instead of a placeholder). The Skeleton import on line 6 is unused — it was likely intended as the fallback, matching the pattern in flow-link.tsx: <Suspense fallback={<Skeleton className="h-4 w-full" />}>. Add the fallback prop (or remove the dead import if intentionally omitted).

Extended reasoning...

What the bug is

The new FlowIconText component wraps its implementation in <Suspense> on line 14 but provides no fallback prop. In React, a Suspense boundary without a fallback renders nothing (null) while its children are suspended. Since FlowIconTextImplementation uses useSuspenseQuery, it will suspend on every initial render while the flow data is being fetched, resulting in blank space in the UI.

Evidence of intent

Line 6 imports Skeleton from @/components/ui/skeleton, but Skeleton is never referenced anywhere else in the file. This is a dead import that strongly suggests the author intended to use it as a Suspense fallback but forgot to wire it up.

Established pattern in the codebase

The existing FlowLink component in the same directory (flow-link.tsx:14) uses:

<Suspense fallback={<Skeleton className="h-4 w-full" />}>

This is the exact pattern that FlowIconText should follow. The Skeleton import is already present — it just needs to be passed as the fallback.

Step-by-step proof

  1. User navigates to a page that renders <FlowIconText flowId="some-id" />.
  2. FlowIconText renders <Suspense> (no fallback) wrapping <FlowIconTextImplementation>.
  3. FlowIconTextImplementation calls useSuspenseQuery(buildFLowDetailsQuery(flowId)), which suspends while the network request is in flight.
  4. React hits the Suspense boundary. Since fallback is undefined, React renders nothing — the component area is blank.
  5. Once the query resolves, the link with the flow icon and name appears. The user experiences a flash of empty space before content loads.

Impact

This is a UX issue: users see blank space where a loading skeleton should appear. It is inconsistent with the existing FlowLink component and leaves the unused Skeleton import as dead code (violating AGENTS.md Rule 12).

Fix

Change line 14 from:

<Suspense>

to:

<Suspense fallback={<Skeleton className="h-4 w-full" />}>

This uses the already-imported Skeleton component and matches the established pattern in flow-link.tsx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant