Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/sdk/src/components/Model/ModelCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ export function ModelCell({
result={queryData.data.result}
maxHeight={600}
maxResultSize={maxResultSize}
renderLogs={queryData.data.renderLogs}
/>
)}
</CleanMetricCard>
Expand All @@ -188,6 +189,7 @@ export function ModelCell({
open={resultsDialogOpen}
onClose={() => setResultsDialogOpen(false)}
result={queryData?.data?.result || ""}
renderLogs={queryData?.data?.renderLogs}
title={`Query: ${queryName}`}
/>
</CleanNotebookCell>
Expand Down
6 changes: 5 additions & 1 deletion packages/sdk/src/components/QueryResult/QueryResult.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,11 @@ export default function QueryResult({
)}
{isSuccess && (
<Suspense fallback={<div>Loading...</div>}>
<ResultContainer result={data.data.result} maxHeight={height} />
<ResultContainer
result={data.data.result}
maxHeight={height}
renderLogs={data.data.renderLogs}
/>
</Suspense>
)}
{isError && (
Expand Down
41 changes: 40 additions & 1 deletion packages/sdk/src/components/RenderedResult/ResultContainer.tsx
Original file line number Diff line number Diff line change
@@ -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"));

Expand All @@ -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.
Expand All @@ -22,10 +28,12 @@ export default function ResultContainer({
result,
maxHeight,
maxResultSize = 0,
renderLogs,
}: ResultContainerProps) {
const containerRef = useRef<HTMLDivElement>(null);
const [measuredHeight, setMeasuredHeight] = useState(maxHeight);
const [userAcknowledged, setUserAcknowledged] = useState(false);
const renderLogSummary = summarizeRenderLogs(renderLogs);

if (!result) {
return null;
Expand Down Expand Up @@ -90,6 +98,37 @@ export default function ResultContainer({
/>
</Suspense>
)}
{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.
<Box sx={{ position: "absolute", top: 4, right: 4, zIndex: 1 }}>
<Tooltip
title={renderLogSummary.title}
slotProps={{ tooltip: { sx: { whiteSpace: "pre-line" } } }}
>
<IconButton
size="small"
aria-label="Render tag warnings"
sx={{
backgroundColor: "rgba(255, 255, 255, 0.9)",
"&:hover": {
backgroundColor: "rgba(255, 255, 255, 1)",
},
}}
>
<InfoOutlinedIcon
fontSize="small"
color={
renderLogSummary.severity === "error"
? "error"
: "warning"
}
/>
</IconButton>
</Tooltip>
</Box>
)}
</Box>
);
}
55 changes: 55 additions & 0 deletions packages/sdk/src/components/RenderedResult/renderLogs.spec.ts
Original file line number Diff line number Diff line change
@@ -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");
});
});
40 changes: 40 additions & 0 deletions packages/sdk/src/components/RenderedResult/renderLogs.ts
Original file line number Diff line number Diff line change
@@ -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"),
};
}
4 changes: 4 additions & 0 deletions packages/sdk/src/components/ResultsDialog.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
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 {
open: boolean;
onClose: () => void;
result: string;
title?: string;
renderLogs?: LogMessage[];
}

export default function ResultsDialog({
open,
onClose,
result,
title = "Results",
renderLogs,
}: ResultsDialogProps) {
return (
<Dialog
Expand Down Expand Up @@ -52,6 +55,7 @@ export default function ResultsDialog({
result={result}
maxHeight={800}
maxResultSize={1000000}
renderLogs={renderLogs}
/>
</DialogContent>
</Dialog>
Expand Down
Loading