diff --git a/packages/sdk/src/components/Model/ModelCell.tsx b/packages/sdk/src/components/Model/ModelCell.tsx index c6d2a0d61..578278669 100644 --- a/packages/sdk/src/components/Model/ModelCell.tsx +++ b/packages/sdk/src/components/Model/ModelCell.tsx @@ -179,6 +179,7 @@ export function ModelCell({ result={queryData.data.result} maxHeight={600} maxResultSize={maxResultSize} + renderLogs={queryData.data.renderLogs} /> )} @@ -188,6 +189,7 @@ export function ModelCell({ open={resultsDialogOpen} onClose={() => setResultsDialogOpen(false)} result={queryData?.data?.result || ""} + renderLogs={queryData?.data?.renderLogs} title={`Query: ${queryName}`} /> diff --git a/packages/sdk/src/components/QueryResult/QueryResult.tsx b/packages/sdk/src/components/QueryResult/QueryResult.tsx index 307e8b713..794d6c3fa 100644 --- a/packages/sdk/src/components/QueryResult/QueryResult.tsx +++ b/packages/sdk/src/components/QueryResult/QueryResult.tsx @@ -99,7 +99,11 @@ export default function QueryResult({ )} {isSuccess && ( Loading...}> - + )} {isError && ( diff --git a/packages/sdk/src/components/RenderedResult/ResultContainer.tsx b/packages/sdk/src/components/RenderedResult/ResultContainer.tsx index 49edf1614..4ee0311bd 100644 --- a/packages/sdk/src/components/RenderedResult/ResultContainer.tsx +++ b/packages/sdk/src/components/RenderedResult/ResultContainer.tsx @@ -1,7 +1,10 @@ import { Warning } from "@mui/icons-material"; -import { Box, Button, Typography } from "@mui/material"; +import InfoOutlinedIcon from "@mui/icons-material/InfoOutlined"; +import { Box, Button, IconButton, Tooltip, Typography } from "@mui/material"; import { lazy, Suspense, useRef, useState } from "react"; +import { LogMessage } from "../../client"; import { Loading } from "../Loading"; +import { summarizeRenderLogs } from "./renderLogs"; const RenderedResult = lazy(() => import("../RenderedResult/RenderedResult")); @@ -12,6 +15,9 @@ interface ResultContainerProps { // this is to prevent performance issues with large results. // the default is 0, which means no warning will be shown. maxResultSize?: number; + // Render tag findings from the query response. Callers whose result did not + // come from a query response have none to pass. + renderLogs?: LogMessage[]; } // ResultContainer is a component that renders a result, with a toggle button to expand/collapse the result. @@ -22,10 +28,12 @@ export default function ResultContainer({ result, maxHeight, maxResultSize = 0, + renderLogs, }: ResultContainerProps) { const containerRef = useRef(null); const [measuredHeight, setMeasuredHeight] = useState(maxHeight); const [userAcknowledged, setUserAcknowledged] = useState(false); + const renderLogSummary = summarizeRenderLogs(renderLogs); if (!result) { return null; @@ -90,6 +98,37 @@ export default function ResultContainer({ /> )} + {renderLogSummary && ( + // Overlaid rather than stacked, so the note cannot change the height + // the container just measured. The button is what makes the message + // reachable: an icon alone is aria-hidden and cannot take focus. + + + + + + + + )} ); } diff --git a/packages/sdk/src/components/RenderedResult/renderLogs.spec.ts b/packages/sdk/src/components/RenderedResult/renderLogs.spec.ts new file mode 100644 index 000000000..0a3c69820 --- /dev/null +++ b/packages/sdk/src/components/RenderedResult/renderLogs.spec.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "bun:test"; +import { summarizeRenderLogs } from "./renderLogs"; + +describe("summarizeRenderLogs", () => { + it("returns nothing when there are no logs", () => { + expect(summarizeRenderLogs(undefined)).toBeUndefined(); + expect(summarizeRenderLogs([])).toBeUndefined(); + }); + + it("surfaces a warning, which is the severity a bad render tag reports", () => { + expect( + summarizeRenderLogs([ + { + severity: "warn", + message: "Unknown render tag 'viz.stack.y' on field 'root'", + }, + ]), + ).toEqual({ + severity: "warn", + title: "Unknown render tag 'viz.stack.y' on field 'root'", + }); + }); + + it("reports the worst severity when both are present", () => { + const summary = summarizeRenderLogs([ + { severity: "warn", message: "first" }, + { severity: "error", message: "second" }, + ]); + expect(summary?.severity).toBe("error"); + expect(summary?.title).toBe("first\nsecond"); + }); + + it("drops debug and info, which are not worth interrupting for", () => { + expect( + summarizeRenderLogs([ + { severity: "debug", message: "noise" }, + { severity: "info", message: "more noise" }, + ]), + ).toBeUndefined(); + }); + + it("drops entries with no message or no severity rather than showing an empty tooltip", () => { + // Every LogMessage field is optional in the generated client. + expect(summarizeRenderLogs([{ severity: "warn" }])).toBeUndefined(); + expect(summarizeRenderLogs([{ message: "orphan" }])).toBeUndefined(); + }); + + it("dedupes repeated messages", () => { + const summary = summarizeRenderLogs([ + { severity: "warn", message: "same" }, + { severity: "warn", message: "same" }, + ]); + expect(summary?.title).toBe("same"); + }); +}); diff --git a/packages/sdk/src/components/RenderedResult/renderLogs.ts b/packages/sdk/src/components/RenderedResult/renderLogs.ts new file mode 100644 index 000000000..2f76c74f1 --- /dev/null +++ b/packages/sdk/src/components/RenderedResult/renderLogs.ts @@ -0,0 +1,40 @@ +import { LogMessage } from "../../client"; + +export interface RenderLogSummary { + severity: "warn" | "error"; + title: string; +} + +// Severity describes the tag defect, not whether the chart drew, so findings are +// only ever annotated, never used to withhold a result. +const SHOWN_SEVERITIES = new Set(["warn", "error"]); + +// Every LogMessage field is optional, so entries without a severity we show or +// without a message are dropped rather than rendered as an empty tooltip. +export function summarizeRenderLogs( + logs: LogMessage[] | undefined, +): RenderLogSummary | undefined { + if (!logs?.length) { + return undefined; + } + const messages: string[] = []; + let hasError = false; + for (const log of logs) { + if (!log.message || !SHOWN_SEVERITIES.has(log.severity ?? "")) { + continue; + } + if (log.severity === "error") { + hasError = true; + } + if (!messages.includes(log.message)) { + messages.push(log.message); + } + } + if (!messages.length) { + return undefined; + } + return { + severity: hasError ? "error" : "warn", + title: messages.join("\n"), + }; +} diff --git a/packages/sdk/src/components/ResultsDialog.tsx b/packages/sdk/src/components/ResultsDialog.tsx index 40795058c..d54873e03 100644 --- a/packages/sdk/src/components/ResultsDialog.tsx +++ b/packages/sdk/src/components/ResultsDialog.tsx @@ -1,5 +1,6 @@ import CloseIcon from "@mui/icons-material/Close"; import { Dialog, DialogContent, DialogTitle, IconButton } from "@mui/material"; +import { LogMessage } from "../client"; import ResultContainer from "./RenderedResult/ResultContainer"; interface ResultsDialogProps { @@ -7,6 +8,7 @@ interface ResultsDialogProps { onClose: () => void; result: string; title?: string; + renderLogs?: LogMessage[]; } export default function ResultsDialog({ @@ -14,6 +16,7 @@ export default function ResultsDialog({ onClose, result, title = "Results", + renderLogs, }: ResultsDialogProps) { return (