Skip to content

Run page links to tasks & replay run modal improvements #1364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
0ebd79e
Removed the inline-code accessory from the logs when calling trigger …
samejr Sep 25, 2024
b18129a
Removed re2 from the v3 catalog
samejr Sep 25, 2024
77455f3
Added a Root badge to the runs list
samejr Sep 25, 2024
77bd829
Keep the side panel open when switching tasks & remove links from det…
samejr Sep 25, 2024
35bb7a2
Root and parent task styling inspector
samejr Sep 25, 2024
701d5af
Hide the root badge if the task isn’t the root
samejr Sep 25, 2024
5a33a00
margin between the dev not running message
samejr Sep 26, 2024
8425361
improved spacing of items
samejr Sep 26, 2024
84c38c1
Improved Root badge style
samejr Sep 26, 2024
28af219
Show a table of triggered runs in the inspector
samejr Sep 26, 2024
d538eca
Merge remote-tracking branch 'origin/main' into run-page-additional-data
samejr Sep 26, 2024
7932a05
Add parentSpanId index to the TaskRun table
samejr Sep 26, 2024
4ee4963
Fix for the run inspector now opening when linked from another run/re…
samejr Sep 26, 2024
be23df4
Triggered runs table has a max height
samejr Sep 26, 2024
3953e8b
Added a description to the replay run modal and improved the styling …
samejr Sep 26, 2024
9b1c664
Only include a bottom border when the triggered run table is more tha…
samejr Sep 26, 2024
ae9adb5
Improved the triggered runs table borders
samejr Sep 26, 2024
ba0ec21
Improved the tables so they can have an optional sticky header
samejr Sep 26, 2024
ba60cb6
Added table types to storybook
samejr Sep 26, 2024
50d5680
Fix for hover states on different backgrounds & runs table select cell
samejr Sep 27, 2024
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
5 changes: 5 additions & 0 deletions .changeset/three-olives-love.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@trigger.dev/sdk": patch
---

Removed the inline-code accessory from the logs when calling trigger or batchTrigger from a run
2 changes: 2 additions & 0 deletions apps/webapp/app/components/primitives/Badge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const variants = {
"grid place-items-center rounded-full px-2 h-5 tracking-wider text-xxs bg-charcoal-750 text-text-bright uppercase whitespace-nowrap",
small:
"grid place-items-center rounded-full px-[0.4rem] h-4 tracking-wider text-xxs bg-background-dimmed text-text-dimmed uppercase whitespace-nowrap",
"extra-small":
"grid place-items-center border border-charcoal-650 rounded-sm px-1 h-4 border-tracking-wider text-xxs bg-background-bright text-blue-500 whitespace-nowrap",
outline:
"grid place-items-center rounded-sm px-1.5 h-5 tracking-wider text-xxs border border-dimmed text-text-dimmed uppercase whitespace-nowrap",
"outline-rounded":
Expand Down
29 changes: 21 additions & 8 deletions apps/webapp/app/components/primitives/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const Table = forwardRef<HTMLTableElement, TableProps>(
fullWidth && "w-full"
)}
>
<table ref={ref} className={cn("w-full divide-y", className)}>
<table ref={ref} className={cn("w-full", className)}>
{children}
</table>
</div>
Expand All @@ -40,7 +40,10 @@ export const TableHeader = forwardRef<HTMLTableSectionElement, TableHeaderProps>
return (
<thead
ref={ref}
className={cn("rounded-t-md", "relative divide-y divide-grid-dimmed", className)}
className={cn(
"sticky top-0 z-10 divide-y divide-grid-dimmed rounded-t-md bg-background-dimmed after:absolute after:bottom-0 after:left-0 after:right-0 after:h-px after:bg-grid-dimmed",
className
)}
>
{children}
</thead>
Expand All @@ -56,7 +59,10 @@ type TableBodyProps = {
export const TableBody = forwardRef<HTMLTableSectionElement, TableBodyProps>(
({ className, children }, ref) => {
return (
<tbody ref={ref} className={cn("relative divide-y divide-grid-dimmed", className)}>
<tbody
ref={ref}
className={cn("relative divide-y divide-grid-dimmed overflow-y-auto", className)}
>
{children}
</tbody>
);
Expand Down Expand Up @@ -135,10 +141,17 @@ type TableCellProps = TableCellBasicProps & {
hasAction?: boolean;
isSticky?: boolean;
actionClassName?: string;
rowHoverStyle?: keyof typeof rowHoverStyles;
};

const rowHoverStyles = {
default: "group-hover/table-row:bg-charcoal-800",
dimmed: "group-hover/table-row:bg-charcoal-850",
bright: "group-hover/table-row:bg-charcoal-750",
};

const stickyStyles =
"sticky right-0 z-10 w-[2.8rem] min-w-[2.8rem] bg-background-dimmed before:absolute before:pointer-events-none before:-left-8 before:top-0 before:h-full before:min-w-[2rem] before:bg-gradient-to-r before:from-transparent before:to-background before:content-[''] group-hover/table-row:before:to-charcoal-900";
"sticky right-0 z-10 w-[2.8rem] min-w-[2.8rem] bg-background-dimmed before:absolute before:pointer-events-none before:-left-8 before:top-0 before:h-full before:min-w-[2rem]";

export const TableCell = forwardRef<HTMLTableCellElement, TableCellProps>(
(
Expand All @@ -152,6 +165,7 @@ export const TableCell = forwardRef<HTMLTableCellElement, TableCellProps>(
onClick,
hasAction = false,
isSticky = false,
rowHoverStyle = "default",
},
ref
) => {
Expand All @@ -178,12 +192,11 @@ export const TableCell = forwardRef<HTMLTableCellElement, TableCellProps>(
<td
ref={ref}
className={cn(
"text-xs text-charcoal-400",
to || onClick || hasAction
? "cursor-pointer group-hover/table-row:bg-charcoal-900"
: "px-3 py-3 align-middle",
"text-xs text-charcoal-400 transition-colors",
to || onClick || hasAction ? "cursor-pointer" : "px-3 py-3 align-middle",
!to && !onClick && alignmentClassName,
isSticky && stickyStyles,
rowHoverStyles[rowHoverStyle],
className
)}
colSpan={colSpan}
Expand Down
38 changes: 24 additions & 14 deletions apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { DialogClose } from "@radix-ui/react-dialog";
import { Form, useNavigation, useSubmit } from "@remix-run/react";
import { useCallback, useEffect, useRef } from "react";
import { UseDataFunctionReturn, useTypedFetcher } from "remix-typedjson";
Expand All @@ -8,6 +9,7 @@ import { DialogContent, DialogHeader } from "~/components/primitives/Dialog";
import { Header3 } from "~/components/primitives/Headers";
import { InputGroup } from "~/components/primitives/InputGroup";
import { Label } from "~/components/primitives/Label";
import { Paragraph } from "~/components/primitives/Paragraph";
import { Select, SelectItem } from "~/components/primitives/Select";
import { ButtonSpinner, Spinner } from "~/components/primitives/Spinner";
import { type loader } from "~/routes/resources.taskruns.$runParam.replay";
Expand All @@ -19,7 +21,7 @@ type ReplayRunDialogProps = {

export function ReplayRunDialog({ runFriendlyId, failedRedirect }: ReplayRunDialogProps) {
return (
<DialogContent key={`replay`} className="md:max-w-3xl">
<DialogContent key={`replay`} className="md:max-w-xl">
<ReplayContent runFriendlyId={runFriendlyId} failedRedirect={failedRedirect} />
</DialogContent>
);
Expand Down Expand Up @@ -95,8 +97,12 @@ function ReplayForm({
<Form action={formAction} method="post" onSubmit={(e) => submitForm(e)} className="pt-2">
{editablePayload ? (
<>
<Paragraph className="mb-3">
Replaying will create a new run using the same or modified payload, executing against
the latest version in your selected environment.
</Paragraph>
<Header3 spacing>Payload</Header3>
<div className="mb-3 max-h-[70vh] overflow-y-auto rounded-sm border border-grid-dimmed bg-charcoal-900 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600">
<div className="mb-3 max-h-[70vh] min-h-40 overflow-y-auto rounded-sm border border-grid-dimmed bg-charcoal-900 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600">
<JSONEditor
autoFocus
defaultValue={currentJson.current}
Expand All @@ -123,8 +129,8 @@ function ReplayForm({
defaultValue={environment.id}
items={environments}
dropdownIcon
variant="tertiary/medium"
className="w-fit pl-2"
variant="tertiary/small"
className="w-fit pl-1"
text={(value) => {
const env = environments.find((env) => env.id === value)!;
return (
Expand All @@ -144,16 +150,20 @@ function ReplayForm({
</Select>
</InputGroup>
<input type="hidden" name="failedRedirect" value={failedRedirect} />
<Button
type="submit"
variant="primary/medium"
LeadingIcon={isSubmitting ? ButtonSpinner : undefined}
disabled={isSubmitting}
shortcut={{ modifiers: ["meta"], key: "enter", enabledOnInputElements: true }}
className="mt-5"
>
{isSubmitting ? "Replaying..." : "Replay run"}
</Button>
<div className="mt-3 flex items-center justify-between gap-2 border-t border-grid-dimmed pt-3.5">
<DialogClose asChild>
<Button variant="tertiary/small">Cancel</Button>
</DialogClose>
<Button
type="submit"
variant="primary/small"
LeadingIcon={isSubmitting ? ButtonSpinner : undefined}
disabled={isSubmitting}
shortcut={{ modifiers: ["meta"], key: "enter", enabledOnInputElements: true }}
>
{isSubmitting ? "Replaying..." : "Replay run"}
</Button>
</div>
</Form>
);
}
10 changes: 8 additions & 2 deletions apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { LiveTimer } from "./LiveTimer";
import { ReplayRunDialog } from "./ReplayRunDialog";
import { TaskRunStatusCombo } from "./TaskRunStatus";
import { RunTag } from "./RunTag";
import { Badge } from "~/components/primitives/Badge";

type RunsTableProps = {
total: number;
Expand Down Expand Up @@ -123,7 +124,7 @@ export function TaskRunsTable({
)}
<TableHeaderCell alignment="right">Run #</TableHeaderCell>
<TableHeaderCell>Env</TableHeaderCell>
<TableHeaderCell>Task ID</TableHeaderCell>
<TableHeaderCell>Task</TableHeaderCell>
<TableHeaderCell>Version</TableHeaderCell>
<TableHeaderCell>Status</TableHeaderCell>
<TableHeaderCell>Started</TableHeaderCell>
Expand Down Expand Up @@ -278,7 +279,12 @@ export function TaskRunsTable({
userName={run.environment.userName}
/>
</TableCell>
<TableCell to={path}>{run.taskIdentifier}</TableCell>
<TableCell to={path}>
<span className="flex items-center gap-x-1">
{run.taskIdentifier}
{run.rootTaskRunId === null ? <Badge variant="extra-small">Root</Badge> : null}
</span>
</TableCell>
<TableCell to={path}>{run.version ?? "–"}</TableCell>
<TableCell to={path}>
<TaskRunStatusCombo status={run.status} />
Expand Down
32 changes: 0 additions & 32 deletions apps/webapp/app/hooks/useReplaceLocation.ts

This file was deleted.

22 changes: 22 additions & 0 deletions apps/webapp/app/hooks/useReplaceSearchParams.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { useSearchParams } from "@remix-run/react";
import { useCallback } from "react";

export function useReplaceSearchParams() {
const [searchParams, setSearchParams] = useSearchParams();

const replaceSearchParam = useCallback(
(key: string, value?: string) => {
setSearchParams((s) => {
if (value) {
s.set(key, value);
} else {
s.delete(key);
}
return s;
});
},
[searchParams]
);

return { searchParams, setSearchParams, replaceSearchParam };
}
Comment on lines +4 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

LGTM: Well-implemented custom hook with a minor optimization suggestion.

The useReplaceSearchParams hook is well-implemented, providing a clean API for managing search parameters. The use of useCallback for memoization is a good practice. However, there's a minor optimization that can be made:

Consider removing searchParams from the useCallback dependency array:

  const replaceSearchParam = useCallback(
    (key: string, value?: string) => {
      setSearchParams((s) => {
        if (value) {
          s.set(key, value);
        } else {
          s.delete(key);
        }
        return s;
      });
    },
-   [searchParams]
+   [setSearchParams]
  );

This change is safe because setSearchParams always provides the most up-to-date searchParams object, and removing searchParams from the dependency array can prevent unnecessary re-renders.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function useReplaceSearchParams() {
const [searchParams, setSearchParams] = useSearchParams();
const replaceSearchParam = useCallback(
(key: string, value?: string) => {
setSearchParams((s) => {
if (value) {
s.set(key, value);
} else {
s.delete(key);
}
return s;
});
},
[searchParams]
);
return { searchParams, setSearchParams, replaceSearchParam };
}
export function useReplaceSearchParams() {
const [searchParams, setSearchParams] = useSearchParams();
const replaceSearchParam = useCallback(
(key: string, value?: string) => {
setSearchParams((s) => {
if (value) {
s.set(key, value);
} else {
s.delete(key);
}
return s;
});
},
[setSearchParams]
);
return { searchParams, setSearchParams, replaceSearchParam };
}

3 changes: 3 additions & 0 deletions apps/webapp/app/presenters/v3/RunListPresenter.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ export class RunListPresenter extends BasePresenter {
usageDurationMs: BigInt;
tags: string[];
depth: number;
rootTaskRunId: string | null;
}[]
>`
SELECT
Expand All @@ -192,6 +193,7 @@ export class RunListPresenter extends BasePresenter {
tr."costInCents" AS "costInCents",
tr."usageDurationMs" AS "usageDurationMs",
tr."depth" AS "depth",
tr."rootTaskRunId" AS "rootTaskRunId",
array_remove(array_agg(tag.name), NULL) AS "tags"
FROM
${sqlDatabaseSchema}."TaskRun" tr
Expand Down Expand Up @@ -336,6 +338,7 @@ WHERE
usageDurationMs: Number(run.usageDurationMs),
tags: run.tags.sort((a, b) => a.localeCompare(b)),
depth: run.depth,
rootTaskRunId: run.rootTaskRunId,
};
}),
pagination: {
Expand Down
9 changes: 9 additions & 0 deletions apps/webapp/app/presenters/v3/RunPresenter.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ export class RunPresenter {
status: true,
completedAt: true,
logsDeletedAt: true,
rootTaskRun: {
select: {
friendlyId: true,
taskIdentifier: true,
spanId: true,
},
},
runtimeEnvironment: {
select: {
id: true,
Expand Down Expand Up @@ -80,6 +87,7 @@ export class RunPresenter {
isFinished: isFinalRunStatus(run.status),
completedAt: run.completedAt,
logsDeletedAt: run.logsDeletedAt,
rootTaskRun: run.rootTaskRun,
environment: {
id: run.runtimeEnvironment.id,
organizationId: run.runtimeEnvironment.organizationId,
Expand Down Expand Up @@ -141,6 +149,7 @@ export class RunPresenter {
isFinished: isFinalRunStatus(run.status),
completedAt: run.completedAt,
logsDeletedAt: run.logsDeletedAt,
rootTaskRun: run.rootTaskRun,
environment: {
id: run.runtimeEnvironment.id,
organizationId: run.runtimeEnvironment.organizationId,
Expand Down
44 changes: 43 additions & 1 deletion apps/webapp/app/presenters/v3/SpanPresenter.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,21 @@ export class SpanPresenter extends BasePresenter {
exportName: true,
},
},
//relationships
rootTaskRun: {
select: {
taskIdentifier: true,
friendlyId: true,
spanId: true,
},
},
parentTaskRun: {
select: {
taskIdentifier: true,
friendlyId: true,
spanId: true,
},
},
},
where: {
spanId,
Expand Down Expand Up @@ -281,7 +296,15 @@ export class SpanPresenter extends BasePresenter {
output,
outputType: finishedAttempt?.outputType ?? "application/json",
error,
links: span?.links,
relationships: {
root: run.rootTaskRun
? {
...run.rootTaskRun,
isParent: run.parentTaskRun?.friendlyId === run.rootTaskRun.friendlyId,
}
: undefined,
parent: run.parentTaskRun ?? undefined,
},
context: JSON.stringify(context, null, 2),
metadata,
};
Expand All @@ -307,10 +330,29 @@ export class SpanPresenter extends BasePresenter {
return;
}

const triggeredRuns = await this._replica.taskRun.findMany({
select: {
friendlyId: true,
taskIdentifier: true,
spanId: true,
createdAt: true,
number: true,
lockedToVersion: {
select: {
version: true,
},
},
},
where: {
parentSpanId: spanId,
},
});

Comment on lines +333 to +350
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential performance issue: Consider implementing pagination or limiting the results for triggeredRuns.

Fetching all triggeredRuns without a limit might lead to performance issues if there are many triggered runs associated with a span. To prevent potential delays or excessive memory usage, it's advisable to implement pagination or set a reasonable limit on the number of results returned.

Apply this diff to add a limit to the query:

        const triggeredRuns = await this._replica.taskRun.findMany({
          select: {
            friendlyId: true,
            taskIdentifier: true,
            spanId: true,
            createdAt: true,
            number: true,
            lockedToVersion: {
              select: {
                version: true,
              },
            },
          },
          where: {
            parentSpanId: spanId,
          },
+         take: 100, // Limit the results to 100
        });

Alternatively, consider adding pagination parameters to the query to allow clients to request specific ranges of results.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const triggeredRuns = await this._replica.taskRun.findMany({
select: {
friendlyId: true,
taskIdentifier: true,
spanId: true,
createdAt: true,
number: true,
lockedToVersion: {
select: {
version: true,
},
},
},
where: {
parentSpanId: spanId,
},
});
const triggeredRuns = await this._replica.taskRun.findMany({
select: {
friendlyId: true,
taskIdentifier: true,
spanId: true,
createdAt: true,
number: true,
lockedToVersion: {
select: {
version: true,
},
},
},
where: {
parentSpanId: spanId,
},
take: 100, // Limit the results to 100
});

return {
...span,
events: span.events,
properties: span.properties ? JSON.stringify(span.properties, null, 2) : undefined,
triggeredRuns,
showActionBar: span.show?.actions === true,
};
}
Expand Down
Loading