fix(review-gates): green success badges, null-safe preview rows, optional section items
- Extract shared statusBadgeClass() helper (src/viewers/statusBadge.ts) so SqlViewer, CteResultViewer and PhaseSummaryViewer can't drift: ok/success/ passed/promoted render green (--success), warn renders amber (--warning), error/failed stay destructive red, everything else stays neutral outline. Previously CteResultViewer/PhaseSummaryViewer mapped "ok"/"promoted" to the default badge variant, which is bg-primary (GSD red) — success states rendered red. - enrich.js: buildCteResultV2 now falls back preview.rows to [] instead of null when last_test.preview_rows is missing (pre-upgrade ok records), and CteResultViewer reads result.preview?.rows?.length with a null-safe fallback so it degrades to the "No preview rows" empty state instead of crashing. - PhaseSummaryViewer: section.items is optional (model-authored sections can be prose-only); render (section.items ?? []) instead of crashing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -76,6 +76,16 @@ test("shows empty-state when preview has no rows", async () => {
|
|||||||
expect(screen.getByText(/no preview rows/i)).toBeInTheDocument();
|
expect(screen.getByText(/no preview rows/i)).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("shows empty-state, no crash, when preview.rows is null (pre-upgrade records)", async () => {
|
||||||
|
const nullRows = {
|
||||||
|
...okResult,
|
||||||
|
preview: { columns: ["paziente_id"], rows: null as unknown as (string | number)[][] },
|
||||||
|
};
|
||||||
|
render(<CteResultViewer result={nullRows} />);
|
||||||
|
await screen.findByTestId("hl");
|
||||||
|
expect(screen.getByText(/no preview rows/i)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
test("renders columns definition list with descriptions", async () => {
|
test("renders columns definition list with descriptions", async () => {
|
||||||
const { container } = render(<CteResultViewer result={okResult} />);
|
const { container } = render(<CteResultViewer result={okResult} />);
|
||||||
await screen.findByTestId("hl");
|
await screen.findByTestId("hl");
|
||||||
|
|||||||
@@ -1,16 +1,11 @@
|
|||||||
import { Badge } from "../components/ui/badge";
|
import { Badge } from "../components/ui/badge";
|
||||||
import { SqlViewer, type SqlBlock } from "./SqlViewer";
|
import { SqlViewer, type SqlBlock } from "./SqlViewer";
|
||||||
import { PreviewGrid } from "./PreviewGrid";
|
import { PreviewGrid } from "./PreviewGrid";
|
||||||
|
import { statusBadgeClass } from "./statusBadge";
|
||||||
import type { CteResultV2 } from "./artifactV2";
|
import type { CteResultV2 } from "./artifactV2";
|
||||||
|
|
||||||
const CHIP = "rounded-md bg-muted px-1.5 py-0.5 font-mono text-xs text-foreground/90";
|
const CHIP = "rounded-md bg-muted px-1.5 py-0.5 font-mono text-xs text-foreground/90";
|
||||||
|
|
||||||
function statusBadgeVariant(status: string): "default" | "destructive" | "outline" {
|
|
||||||
if (status === "ok") return "default";
|
|
||||||
if (status === "error") return "destructive";
|
|
||||||
return "outline";
|
|
||||||
}
|
|
||||||
|
|
||||||
function statusLabel(status: string): string {
|
function statusLabel(status: string): string {
|
||||||
if (status === "ok") return "success";
|
if (status === "ok") return "success";
|
||||||
if (status === "error") return "error";
|
if (status === "error") return "error";
|
||||||
@@ -24,7 +19,7 @@ function testStatus(status: string): SqlBlock["testStatus"] {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function CteResultViewer({ result }: { result: CteResultV2 }) {
|
export function CteResultViewer({ result }: { result: CteResultV2 }) {
|
||||||
const hasPreviewRows = !!result.preview && result.preview.rows.length > 0;
|
const hasPreviewRows = (result.preview?.rows?.length ?? 0) > 0;
|
||||||
const descriptions = (result.columns ?? []).map((c) => c.description ?? "");
|
const descriptions = (result.columns ?? []).map((c) => c.description ?? "");
|
||||||
const showColumns = descriptions.some((d) => d.trim() !== "");
|
const showColumns = descriptions.some((d) => d.trim() !== "");
|
||||||
const dependsOn = result.depends_on ?? [];
|
const dependsOn = result.depends_on ?? [];
|
||||||
@@ -42,7 +37,7 @@ export function CteResultViewer({ result }: { result: CteResultV2 }) {
|
|||||||
CTE {result.index}/{result.total}
|
CTE {result.index}/{result.total}
|
||||||
</Badge>
|
</Badge>
|
||||||
<span className="font-mono text-sm font-semibold">{result.name}</span>
|
<span className="font-mono text-sm font-semibold">{result.name}</span>
|
||||||
<Badge variant={statusBadgeVariant(result.status)}>{statusLabel(result.status)}</Badge>
|
<Badge variant="outline" className={statusBadgeClass(result.status)}>{statusLabel(result.status)}</Badge>
|
||||||
{result.execution_ms !== undefined && (
|
{result.execution_ms !== undefined && (
|
||||||
<span className="text-xs text-muted-foreground">{result.execution_ms} ms</span>
|
<span className="text-xs text-muted-foreground">{result.execution_ms} ms</span>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -81,6 +81,16 @@ test("renders open_questions as a bullet list", () => {
|
|||||||
expect(screen.getByText("Serve confermare la finestra temporale?")).toBeInTheDocument();
|
expect(screen.getByText("Serve confermare la finestra temporale?")).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("renders a section with title only (no items key) without crashing", () => {
|
||||||
|
const proseOnly: PhaseSummaryV2 = {
|
||||||
|
schema_version: 2,
|
||||||
|
phase: { id: "F5", num: 5, name: "sintesi" },
|
||||||
|
sections: [{ title: "Note libere" }],
|
||||||
|
};
|
||||||
|
render(<PhaseSummaryViewer phase={proseOnly} />);
|
||||||
|
expect(screen.getByText("Note libere")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
test("omits open_questions section when empty and shows no 'undefined' text", () => {
|
test("omits open_questions section when empty and shows no 'undefined' text", () => {
|
||||||
const minimal: PhaseSummaryV2 = {
|
const minimal: PhaseSummaryV2 = {
|
||||||
schema_version: 2,
|
schema_version: 2,
|
||||||
|
|||||||
@@ -1,19 +1,14 @@
|
|||||||
import { Badge } from "../components/ui/badge";
|
import { Badge } from "../components/ui/badge";
|
||||||
import { MarkdownView } from "./MarkdownView";
|
import { MarkdownView } from "./MarkdownView";
|
||||||
|
import { statusBadgeClass } from "./statusBadge";
|
||||||
import type { PhaseCheck, PhaseSection, PhaseSectionItem, PhaseSummaryV2, PhaseTable } from "./artifactV2";
|
import type { PhaseCheck, PhaseSection, PhaseSectionItem, PhaseSummaryV2, PhaseTable } from "./artifactV2";
|
||||||
|
|
||||||
const CODE_CHIP = "rounded-md bg-muted px-1.5 py-0.5 font-mono text-xs text-foreground/90";
|
const CODE_CHIP = "rounded-md bg-muted px-1.5 py-0.5 font-mono text-xs text-foreground/90";
|
||||||
|
|
||||||
function checkBadgeVariant(status: string): "default" | "destructive" | "outline" {
|
|
||||||
if (status === "ok") return "default";
|
|
||||||
if (status === "fail") return "destructive";
|
|
||||||
return "outline";
|
|
||||||
}
|
|
||||||
|
|
||||||
function Check({ check }: { check: PhaseCheck }) {
|
function Check({ check }: { check: PhaseCheck }) {
|
||||||
return (
|
return (
|
||||||
<div className="flex items-start gap-2">
|
<div className="flex items-start gap-2">
|
||||||
<Badge variant={checkBadgeVariant(check.status)}>{check.status}</Badge>
|
<Badge variant="outline" className={statusBadgeClass(check.status)}>{check.status}</Badge>
|
||||||
<div className="flex flex-col">
|
<div className="flex flex-col">
|
||||||
<span className="text-sm text-foreground/90">{check.label}</span>
|
<span className="text-sm text-foreground/90">{check.label}</span>
|
||||||
{check.detail && <span className="text-xs text-muted-foreground">{check.detail}</span>}
|
{check.detail && <span className="text-xs text-muted-foreground">{check.detail}</span>}
|
||||||
@@ -42,7 +37,7 @@ function Section({ section }: { section: PhaseSection }) {
|
|||||||
<div>
|
<div>
|
||||||
<h4 className="thot-label mb-1.5">{section.title}</h4>
|
<h4 className="thot-label mb-1.5">{section.title}</h4>
|
||||||
<div className="flex flex-col gap-2">
|
<div className="flex flex-col gap-2">
|
||||||
{section.items.map((item, i) => (
|
{(section.items ?? []).map((item, i) => (
|
||||||
<SectionItemRow key={i} item={item} />
|
<SectionItemRow key={i} item={item} />
|
||||||
))}
|
))}
|
||||||
</div>
|
</div>
|
||||||
@@ -55,7 +50,7 @@ function TableRecap({ table }: { table: PhaseTable }) {
|
|||||||
<div className="flex flex-col gap-1">
|
<div className="flex flex-col gap-1">
|
||||||
<div className="flex items-center gap-2">
|
<div className="flex items-center gap-2">
|
||||||
<span className="font-mono text-sm">{table.name}</span>
|
<span className="font-mono text-sm">{table.name}</span>
|
||||||
<Badge variant={table.role === "promoted" ? "default" : "outline"}>{table.role}</Badge>
|
<Badge variant="outline" className={table.role === "promoted" ? statusBadgeClass(table.role) : undefined}>{table.role}</Badge>
|
||||||
</div>
|
</div>
|
||||||
{table.description && <p className="text-xs text-muted-foreground">{table.description}</p>}
|
{table.description && <p className="text-xs text-muted-foreground">{table.description}</p>}
|
||||||
{table.columns && table.columns.length > 0 && (
|
{table.columns && table.columns.length > 0 && (
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import React, { useEffect, useState } from "react";
|
import React, { useEffect, useState } from "react";
|
||||||
import { highlightSql } from "./highlight";
|
import { highlightSql } from "./highlight";
|
||||||
|
import { statusBadgeClass } from "./statusBadge";
|
||||||
|
|
||||||
export interface SqlBlock {
|
export interface SqlBlock {
|
||||||
name: string;
|
name: string;
|
||||||
@@ -17,12 +18,6 @@ const STATUS_LABELS: Record<string, string> = {
|
|||||||
untested: "untested",
|
untested: "untested",
|
||||||
};
|
};
|
||||||
|
|
||||||
const STATUS_CLASSES: Record<string, string> = {
|
|
||||||
passed: "bg-[oklch(var(--success)/0.15)] text-[oklch(0.45_0.12_165)] ring-1 ring-[oklch(var(--success)/0.3)]",
|
|
||||||
failed: "bg-destructive/10 text-destructive ring-1 ring-destructive/25",
|
|
||||||
untested: "bg-muted text-muted-foreground ring-1 ring-border",
|
|
||||||
};
|
|
||||||
|
|
||||||
function Block({ block }: { block: SqlBlock }) {
|
function Block({ block }: { block: SqlBlock }) {
|
||||||
const [open, setOpen] = useState(true);
|
const [open, setOpen] = useState(true);
|
||||||
const [html, setHtml] = useState<string>("");
|
const [html, setHtml] = useState<string>("");
|
||||||
@@ -39,7 +34,7 @@ function Block({ block }: { block: SqlBlock }) {
|
|||||||
|
|
||||||
const badge = block.testStatus ? STATUS_LABELS[block.testStatus] : null;
|
const badge = block.testStatus ? STATUS_LABELS[block.testStatus] : null;
|
||||||
const badgeClass = block.testStatus
|
const badgeClass = block.testStatus
|
||||||
? STATUS_CLASSES[block.testStatus]
|
? statusBadgeClass(block.testStatus)
|
||||||
: "";
|
: "";
|
||||||
|
|
||||||
return (
|
return (
|
||||||
|
|||||||
@@ -94,7 +94,7 @@ export interface PhaseSectionItem {
|
|||||||
|
|
||||||
export interface PhaseSection {
|
export interface PhaseSection {
|
||||||
title: string;
|
title: string;
|
||||||
items: PhaseSectionItem[];
|
items?: PhaseSectionItem[];
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface PhaseTableColumn {
|
export interface PhaseTableColumn {
|
||||||
|
|||||||
@@ -0,0 +1,25 @@
|
|||||||
|
// Shared status → badge class mapping so success/warning/error colors can't drift
|
||||||
|
// between SqlViewer, CteResultViewer and PhaseSummaryViewer.
|
||||||
|
// ok/success/passed/promoted -> green (--success)
|
||||||
|
// warn/untested-with-warning -> amber (--warning)
|
||||||
|
// error/failed -> destructive/red
|
||||||
|
// anything else -> neutral outline
|
||||||
|
const SUCCESS_STATUSES = new Set(["ok", "success", "passed", "promoted"]);
|
||||||
|
const WARN_STATUSES = new Set(["warn", "warning"]);
|
||||||
|
const ERROR_STATUSES = new Set(["error", "failed", "fail"]);
|
||||||
|
|
||||||
|
export const STATUS_BADGE_CLASSES = {
|
||||||
|
success:
|
||||||
|
"bg-[oklch(var(--success)/0.15)] text-[oklch(0.45_0.12_165)] ring-1 ring-[oklch(var(--success)/0.3)]",
|
||||||
|
warning:
|
||||||
|
"bg-[oklch(var(--warning)/0.15)] text-[oklch(0.46_0.11_79)] ring-1 ring-[oklch(var(--warning)/0.4)]",
|
||||||
|
error: "bg-destructive/10 text-destructive ring-1 ring-destructive/25",
|
||||||
|
neutral: "bg-muted text-muted-foreground ring-1 ring-border",
|
||||||
|
} as const;
|
||||||
|
|
||||||
|
export function statusBadgeClass(status: string): string {
|
||||||
|
if (SUCCESS_STATUSES.has(status)) return STATUS_BADGE_CLASSES.success;
|
||||||
|
if (WARN_STATUSES.has(status)) return STATUS_BADGE_CLASSES.warning;
|
||||||
|
if (ERROR_STATUSES.has(status)) return STATUS_BADGE_CLASSES.error;
|
||||||
|
return STATUS_BADGE_CLASSES.neutral;
|
||||||
|
}
|
||||||
@@ -132,6 +132,15 @@ test("buildCteResultV2 builds preview from last_test.columns/preview_rows", () =
|
|||||||
assert.deepEqual(out.warnings, []);
|
assert.deepEqual(out.warnings, []);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("buildCteResultV2: last_test.preview_rows null (pre-upgrade ok records) -> preview.rows is []", () => {
|
||||||
|
const cteInfo = {
|
||||||
|
name: "a", index: 1, total: 1, sql: "WITH a AS (SELECT 1)", doc: null,
|
||||||
|
last_test: { status: "ok", execution_ms: 1, row_sample: 1, warnings: [], sql_hash: "h", columns: ["cod_paz"], preview_rows: null },
|
||||||
|
};
|
||||||
|
const out = buildCteResultV2({ schema_version: 2 }, cteInfo);
|
||||||
|
assert.deepEqual(out.preview.rows, []);
|
||||||
|
});
|
||||||
|
|
||||||
test("buildCteResultV2: columns[] carries name, with description left as an empty placeholder (filled separately by the gate)", () => {
|
test("buildCteResultV2: columns[] carries name, with description left as an empty placeholder (filled separately by the gate)", () => {
|
||||||
const cteInfo = {
|
const cteInfo = {
|
||||||
name: "a", index: 1, total: 1, sql: "WITH a AS (SELECT 1)", doc: null,
|
name: "a", index: 1, total: 1, sql: "WITH a AS (SELECT 1)", doc: null,
|
||||||
|
|||||||
@@ -85,7 +85,7 @@ function buildCteResultV2(thinData, cteInfo) {
|
|||||||
warnings: lt.warnings || [],
|
warnings: lt.warnings || [],
|
||||||
sql_hash: lt.sql_hash,
|
sql_hash: lt.sql_hash,
|
||||||
columns: (lt.columns || []).map((name) => ({ name, description: "" })),
|
columns: (lt.columns || []).map((name) => ({ name, description: "" })),
|
||||||
preview: { columns: lt.columns || [], rows: lt.preview_rows || null },
|
preview: { columns: lt.columns || [], rows: lt.preview_rows || [] },
|
||||||
note: thin.note,
|
note: thin.note,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user