diff --git a/docs/superpowers/plans/2026-07-14-workflow-ui-regressions.md b/docs/superpowers/plans/2026-07-14-workflow-ui-regressions.md index 5dbc2b23..0ec4b0c2 100644 --- a/docs/superpowers/plans/2026-07-14-workflow-ui-regressions.md +++ b/docs/superpowers/plans/2026-07-14-workflow-ui-regressions.md @@ -169,30 +169,30 @@ Expected: PASS. - `SqlViewer({ blocks })` shows the layout toggle only when `blocks.length > 1`. - CTE plan data shape remains unchanged. -- [ ] **Step 1: Write failing semantic/layout tests** +- [x] **Step 1: Write failing semantic/layout tests** Assert long filter data is rendered in distinct elements labeled Column, Operator, and Value; assert card sections expose stable headings. Assert a one-block SQL viewer has no Horizontal or Vertical controls while a two-block viewer retains both. -- [ ] **Step 2: Run tests and verify RED** +- [x] **Step 2: Run tests and verify RED** Run: `cd frontend && npx vitest run src/viewers/CtePlanViewer.test.tsx src/viewers/SqlViewer.test.tsx src/viewers/CteResultViewer.test.tsx` Expected: structured filter labels are absent and the one-block toggle is present. -- [ ] **Step 3: Implement responsive CTE cards** +- [x] **Step 3: Implement responsive CTE cards** Use a bordered header grid, prose blocks with `leading-relaxed`, metadata rows with fixed labels, `break-words`/`font-mono` for physical identifiers, and a responsive filter grid such as `grid-cols-1 sm:grid-cols-[minmax(0,1fr)_auto_minmax(0,1fr)]`. Keep output columns wrapping. -- [ ] **Step 4: Hide the single-block toggle** +- [x] **Step 4: Hide the single-block toggle** Wrap the layout control in `{blocks.length > 1 && (...)}` without changing multi-block state or rendering. -- [ ] **Step 5: Run targeted tests and typecheck** +- [x] **Step 5: Run targeted tests and typecheck** Run: `cd frontend && npx vitest run src/viewers/CtePlanViewer.test.tsx src/viewers/SqlViewer.test.tsx src/viewers/CteResultViewer.test.tsx && npx tsc -b` diff --git a/frontend/src/viewers/CtePlanViewer.test.tsx b/frontend/src/viewers/CtePlanViewer.test.tsx index 78d38b3f..4d8d66a7 100644 --- a/frontend/src/viewers/CtePlanViewer.test.tsx +++ b/frontend/src/viewers/CtePlanViewer.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from "@testing-library/react"; +import { render, screen, within } from "@testing-library/react"; import { CtePlanViewer } from "./CtePlanViewer"; import type { CtePlanV2 } from "./artifactV2"; @@ -56,6 +56,18 @@ test("renders one card per CTE with index badge, purpose, and filter value", () expect(screen.getByText(/TRUE/)).toBeInTheDocument(); }); +test("renders each filter as aligned column, operator, and value fields", () => { + render(); + + const filter = screen.getByRole("group", { name: "Filter 1" }); + expect(within(filter).getByText("Column")).toBeInTheDocument(); + expect(within(filter).getByText("Operator")).toBeInTheDocument(); + expect(within(filter).getByText("Value")).toBeInTheDocument(); + expect(within(filter).getByText("pazienti.stato")).toBeInTheDocument(); + expect(within(filter).getByText("IS")).toBeInTheDocument(); + expect(within(filter).getByText("TRUE")).toBeInTheDocument(); +}); + test("shows 'no dependencies' for a CTE with an empty depends_on", () => { render(); expect(screen.getByText(/no dependencies/i)).toBeInTheDocument(); diff --git a/frontend/src/viewers/CtePlanViewer.tsx b/frontend/src/viewers/CtePlanViewer.tsx index 674d1372..9a6731e4 100644 --- a/frontend/src/viewers/CtePlanViewer.tsx +++ b/frontend/src/viewers/CtePlanViewer.tsx @@ -4,19 +4,55 @@ import { Badge } from "../components/ui/badge"; import type { CtePlanCte, CtePlanFilter, CtePlanV2 } from "./artifactV2"; const CHIP = "rounded-md bg-muted px-1.5 py-0.5 font-mono text-xs text-foreground/90"; +const FIELD_LABEL = "text-[0.65rem] font-semibold uppercase tracking-[0.08em] text-muted-foreground"; function Chip({ children }: { children: ReactNode }) { return {children}; } -function FilterChip({ filter }: { filter: CtePlanFilter }) { +function FilterCard({ filter, index }: { filter: CtePlanFilter; index: number }) { return ( -
- - {filter.column} {filter.op} {filter.value} - - {filter.description &&

{filter.description}

} - {filter.rationale &&

{filter.rationale}

} +
+
+
+

Column

+ + {filter.column} + +
+
+

Operator

+ + {filter.op} + +
+
+

Value

+ + {filter.value} + +
+
+ {(filter.description || filter.rationale) && ( +
+ {filter.description && ( + <> + Description +

{filter.description}

+ + )} + {filter.rationale && ( + <> + Rationale +

{filter.rationale}

+ + )} +
+ )}
); } @@ -25,84 +61,97 @@ function CteCard({ cte, total }: { cte: CtePlanCte; total: number }) { const dependsOn = cte.depends_on ?? []; return ( - - -
+ + +
CTE {cte.index}/{total} - {cte.name} + + {cte.name} +
- {cte.purpose &&

{cte.purpose}

} + {cte.purpose && ( +

{cte.purpose}

+ )} - -
-

Depends on

- {dependsOn.length ? ( -
- {dependsOn.map((d) => ( - {d} - ))} -
- ) : ( -

no dependencies

+ +
+
+

Depends on

+ {dependsOn.length ? ( +
+ {dependsOn.map((dependency) => ( + {dependency} + ))} +
+ ) : ( +

no dependencies

+ )} +
+ + {cte.keys && cte.keys.length > 0 && ( +
+

Keys

+
+ {cte.keys.map((key) => ( + {key} + ))} +
+
)}
{cte.tables && cte.tables.length > 0 && ( -
-

Tables

-
- {cte.tables.map((t) => ( -
- {t.name} - {t.description && ( -

{t.description}

+
+

Tables

+
+ {cte.tables.map((table) => ( +
+ + {table.name} + + {table.description && ( +

+ {table.description} +

)}
))}
-
- )} - - {cte.keys && cte.keys.length > 0 && ( -
-

Keys

-
- {cte.keys.map((k) => ( - {k} - ))} -
-
+ )} {cte.filters && cte.filters.length > 0 && ( -
-

Filters

-
+
+

Filters

+
{cte.filters.map((f, i) => ( - + ))}
-
+ )} {cte.output_columns && cte.output_columns.length > 0 && ( -
-

Output columns

+
+

Output columns

- {cte.output_columns.map((c) => ( - {c} + {cte.output_columns.map((column) => ( + {column} ))}
-
+ )} {cte.rationale && ( -
-

Rationale

-

{cte.rationale}

-
+
+

Rationale

+

{cte.rationale}

+
)} @@ -112,8 +161,8 @@ function CteCard({ cte, total }: { cte: CtePlanCte; total: number }) { export function CtePlanViewer({ plan }: { plan: CtePlanV2 }) { return (
- {plan.question &&

{plan.question}

} - {plan.strategy &&

{plan.strategy}

} + {plan.question &&

{plan.question}

} + {plan.strategy &&

{plan.strategy}

}
{plan.ctes.map((c, i) => ( diff --git a/frontend/src/viewers/CteResultViewer.test.tsx b/frontend/src/viewers/CteResultViewer.test.tsx index 5b8fb6b9..812a0e7f 100644 --- a/frontend/src/viewers/CteResultViewer.test.tsx +++ b/frontend/src/viewers/CteResultViewer.test.tsx @@ -45,6 +45,7 @@ test("passes sql to SqlViewer with mapped test status", async () => { render(); await screen.findByTestId("hl"); expect(screen.getByText(/passed/i)).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /horizontal|vertical/i })).not.toBeInTheDocument(); }); test("error status maps to failed test status and destructive badge", async () => { diff --git a/frontend/src/viewers/SqlViewer.test.tsx b/frontend/src/viewers/SqlViewer.test.tsx index 8e58b60e..edf27bb4 100644 --- a/frontend/src/viewers/SqlViewer.test.tsx +++ b/frontend/src/viewers/SqlViewer.test.tsx @@ -84,3 +84,11 @@ test("(c) layout toggle flips its label between Orizzontale and Verticale", asyn screen.queryByRole("button", { name: /horizontal/i }) ).not.toBeInTheDocument(); }); + +test("a single SQL block hides the multi-block layout toggle", async () => { + render(); + await waitFor(() => expect(screen.getByTestId("hl")).toBeInTheDocument()); + + expect(screen.queryByRole("button", { name: /horizontal/i })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /vertical/i })).not.toBeInTheDocument(); +}); diff --git a/frontend/src/viewers/SqlViewer.tsx b/frontend/src/viewers/SqlViewer.tsx index 8778b551..3dca5ad3 100644 --- a/frontend/src/viewers/SqlViewer.tsx +++ b/frontend/src/viewers/SqlViewer.tsx @@ -89,16 +89,18 @@ export function SqlViewer({ blocks }: { blocks: SqlBlock[] }) { return (
-
- -
+ {blocks.length > 1 && ( +
+ +
+ )}