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
23 changes: 0 additions & 23 deletions src/components/flow/flow-toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,6 @@ export function FlowToolbar({
const queryClient = useQueryClient();
const { data: session } = useSession();

const healthQuery = useQuery(
trpc.pipeline.health.queryOptions(
{ pipelineId: pipelineId! },
{ enabled: !!pipelineId && !isDraft && !!deployedAt, refetchInterval: 30_000 },
),
);
const healthStatus = healthQuery.data?.status ?? null;
const sliTotal = healthQuery.data?.slis?.length ?? 0;
const slisBreached = healthQuery.data?.slis?.filter((s: { status: string }) => s.status === "breached").length ?? 0;

// Query pending deploy requests for this pipeline
const pendingRequestsQuery = useQuery({
...trpc.deploy.listPendingRequests.queryOptions({ pipelineId: pipelineId! }),
Expand Down Expand Up @@ -470,19 +460,6 @@ export function FlowToolbar({
{processStatus === "CRASHED" && "Crashed"}
{processStatus === "PENDING" && "Pending..."}
</span>
{/* Health SLI badge */}
{healthStatus === "healthy" && (
<span className="inline-flex items-center gap-1 rounded-full bg-green-500/10 px-2 py-0.5 text-xs font-medium text-green-700 dark:text-green-400">
<span className="h-1.5 w-1.5 rounded-full bg-green-500" />
SLIs: OK
</span>
)}
{healthStatus === "degraded" && (
<span className="inline-flex items-center gap-1 rounded-full bg-yellow-500/10 px-2 py-0.5 text-xs font-medium text-yellow-700 dark:text-yellow-400">
<span className="h-1.5 w-1.5 rounded-full bg-yellow-500" />
SLIs: {slisBreached}/{sliTotal} breached
</span>
)}
</div>
)}

Expand Down
35 changes: 27 additions & 8 deletions src/components/flow/pipeline-settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,16 @@ function SliSettings({ pipelineId }: { pipelineId: string }) {
);
const slis = slisQuery.data ?? [];

const healthQuery = useQuery(
trpc.pipeline.health.queryOptions(
{ pipelineId },
{ enabled: slis.length > 0, refetchInterval: 30_000 },
),
Comment on lines +334 to +338
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Health query fires on draft/undeployed pipelines

The original toolbar version of this query guarded with !isDraft && !!deployedAt before enabling the health endpoint. The new SliSettings component only checks slis.length > 0, so if a pipeline has SLIs configured but has never been deployed (or is still a draft), this query will fire and likely return an error or empty payload from the server.

The UI handles the missing data gracefully (healthQuery.data?.slis ?? []), so there's no crash — but it will generate unnecessary failed requests every 30 seconds as long as the Settings panel is open.

Consider passing isDraft and deployedAt into SliSettings and mirroring the original guard:

Suggested change
const healthQuery = useQuery(
trpc.pipeline.health.queryOptions(
{ pipelineId },
{ enabled: slis.length > 0, refetchInterval: 30_000 },
),
const healthQuery = useQuery(
trpc.pipeline.health.queryOptions(
{ pipelineId },
{ enabled: slis.length > 0 && !isDraft && !!deployedAt, refetchInterval: 30_000 },
),
);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/flow/pipeline-settings.tsx
Line: 334-338

Comment:
Health query fires on draft/undeployed pipelines

The original toolbar version of this query guarded with `!isDraft && !!deployedAt` before enabling the health endpoint. The new `SliSettings` component only checks `slis.length > 0`, so if a pipeline has SLIs configured but has never been deployed (or is still a draft), this query will fire and likely return an error or empty payload from the server.

The UI handles the missing data gracefully (`healthQuery.data?.slis ?? []`), so there's no crash — but it will generate unnecessary failed requests every 30 seconds as long as the Settings panel is open.

Consider passing `isDraft` and `deployedAt` into `SliSettings` and mirroring the original guard:

```suggestion
  const healthQuery = useQuery(
    trpc.pipeline.health.queryOptions(
      { pipelineId },
      { enabled: slis.length > 0 && !isDraft && !!deployedAt, refetchInterval: 30_000 },
    ),
  );
```

How can I resolve this? If you propose a fix, please make it concise.

);
const sliStatuses = new Map(
(healthQuery.data?.slis ?? []).map((s: { metric: string; status: string }) => [s.metric, s.status]),
);

const [sliOpen, setSliOpen] = useState(false);
const [newMetric, setNewMetric] = useState<string>("error_rate");
const [newCondition, setNewCondition] = useState<string>("lt");
Expand Down Expand Up @@ -412,14 +422,23 @@ function SliSettings({ pipelineId }: { pipelineId: string }) {
key={sli.id}
className="flex items-center justify-between rounded-md border px-3 py-2 text-xs"
>
<div>
<span className="font-medium">{metricLabel(sli.metric)}</span>{" "}
<span className="text-muted-foreground">
{sli.condition === "lt" ? "<" : ">"} {sli.threshold}
</span>{" "}
<span className="text-muted-foreground">
({sli.windowMinutes}m)
</span>
<div className="flex items-center gap-2">
{sliStatuses.has(sli.metric) && (
<span className={`h-2 w-2 shrink-0 rounded-full ${
sliStatuses.get(sli.metric) === "met" ? "bg-green-500" :
sliStatuses.get(sli.metric) === "breached" ? "bg-red-500" :
"bg-gray-400"
}`} />
)}
<div>
<span className="font-medium">{metricLabel(sli.metric)}</span>{" "}
<span className="text-muted-foreground">
{sli.condition === "lt" ? "<" : ">"} {sli.threshold}
</span>{" "}
<span className="text-muted-foreground">
({sli.windowMinutes}m)
</span>
</div>
</div>
<Button
variant="ghost"
Expand Down
8 changes: 4 additions & 4 deletions src/components/pipeline/version-history-dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ export function VersionHistoryDialog({
<Table>
<TableHeader>
<TableRow>
<TableHead className="w-[80px]">Version</TableHead>
<TableHead className="max-w-0">Changelog</TableHead>
<TableHead className="w-[120px]">Version</TableHead>
<TableHead>Changelog</TableHead>
<TableHead className="w-[180px]">Created</TableHead>
<TableHead className="w-[120px] text-right">
Actions
Expand All @@ -171,8 +171,8 @@ export function VersionHistoryDialog({
)}
</div>
</TableCell>
<TableCell className="max-w-0">
<span className="text-sm text-muted-foreground truncate block">
<TableCell>
<span className="text-sm text-muted-foreground truncate block max-w-[200px]">
{version.changelog || "No changelog"}
</span>
</TableCell>
Expand Down
2 changes: 1 addition & 1 deletion src/server/routers/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ export const pipelineRouter = router({
if (!latestVersion.nodesSnapshot || !latestVersion.edgesSnapshot) {
throw new TRPCError({
code: "PRECONDITION_FAILED",
message: "Deployed version has no snapshot — deploy once more to enable discard",
message: "Deploy once more to enable discard — this version predates snapshot support",
});
}

Expand Down
Loading