feat: tri-6738 Create aggregated logs page #2862
Conversation
…-logs-in-main-page' into feature/tri-6738-show-aggregated-logs-in-main-page
debug level only for admins removed AI helper
keep header when scrolling in logs page fix logs table header being resized change highlight color for search
Update packet display to highlight text
…logs-in-main-page # Conflicts: # apps/webapp/app/components/primitives/DateTime.tsx # apps/webapp/app/components/primitives/Tooltip.tsx
fixed admin access to logs feature added alpha badge to logs
|
WalkthroughAdds full logs functionality: new UI components (LogsIcon, LogsTable, LogDetailView, LogsSearchInput, LogsLevelFilter, LogsRunIdFilter), table variants and popover tweak, path helper v3LogsPath and SideMenu entry, search-term propagation to CodeBlock/PacketDisplay, many new Remix routes/loaders for logs and related resources, ClickHouse query builders and PREWHERE support, presenters (LogsListPresenter, LogDetailPresenter), repository additions (listFriendlyRunIds), environment ClickHouse settings, and wiring of logsQuerySettings through the ClickHouse client. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Review: feat: tri-6738 Create aggregated logs pageOverall, this is a well-structured feature implementation that adds a comprehensive aggregated logs page to the Trigger.dev dashboard. The code is generally clean and follows existing patterns in the codebase. Here's my detailed review: ✅ Strengths
🐛 Potential Issues1. Empty file detected
2. Duplicate
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/webapp/app/components/primitives/DateTime.tsx (1)
85-97: RemoveasChild={true}or replace Fragment with a real element (span/div)
SimpleTooltippassesasChildto Radix UI'sTooltipTrigger. WhenasChild={true}, Radix attempts to merge props (tabIndex,className,style) and attach refs/listeners to the child element.React.Fragmentcannot receive refs or props, so the trigger will fail to initialize properly and lose hover/focus behavior.Replace the
Fragmentwrappers with<span>at both locations:
- Line ~75-97:
formattedDateTimevariable- Line ~273-278:
buttonprop inSimpleTooltipcallExample fix
- const formattedDateTime = ( - <Fragment> + const formattedDateTime = ( + <span> {formatDateTime(...).replace(/\s/g, String.fromCharCode(32))} {showTimezone ? ` (${timeZone ?? "UTC"})` : null} - </Fragment> + </span> );return ( <SimpleTooltip - button={<Fragment>{formattedDateTime.replace(/\s/g, String.fromCharCode(32))}</Fragment>} + button={<span>{formattedDateTime.replace(/\s/g, String.fromCharCode(32))}</span>} content={tooltipContent} side="right" asChild={true} /> );apps/webapp/app/components/primitives/Table.tsx (1)
23-30: Missing variant properties in "bright/no-hover" may cause undefined class values.The
"bright/no-hover"variant is missing the newheaderCell,cellSize, andcellTextproperties that other variants define. WhenTableHeaderCellorTableCellaccessesvariants[variant].headerCell(line 187),variants[variant].cellSize(line 262), orvariants[variant].cellText(lines 263, 276), it will result inundefinedbeing passed tocn().While
cn()handlesundefinedgracefully, this creates inconsistent behavior and may cause layout issues if this variant is used.🔧 Proposed fix to add missing properties
"bright/no-hover": { header: "bg-transparent", + headerCell: "px-3 py-2.5 pb-3 text-sm", cell: "group-hover/table-row:bg-transparent", + cellSize: "px-3 py-3", + cellText: "text-xs", stickyCell: "bg-background-bright", menuButton: "bg-background-bright", menuButtonDivider: "", rowSelected: "bg-charcoal-750", },
🤖 Fix all issues with AI agents
In @apps/webapp/app/components/logs/LogsTable.tsx:
- Around line 267-289: The BlankState component uses TableBlankRow with
colSpan={7} but the table has only 6 columns; update both occurrences of
TableBlankRow inside the BlankState function to use colSpan={6} (locate the
BlankState function and the two TableBlankRow usages) so the empty-state row
aligns correctly with the table columns.
In @apps/webapp/app/components/navigation/SideMenu.tsx:
- Around line 272-281: The Logs visibility condition is redundant because
useHasAdminAccess() already accounts for impersonation; replace the current
conditional (isAdmin || user.isImpersonating) around the SideMenuItem for "Logs"
with just isAdmin to rely on the single source of truth (symbol: isAdmin) and
remove the tautological check while keeping the same SideMenuItem (name="Logs",
icon=LogsIcon, to={v3LogsPath(...)}).
In @apps/webapp/app/presenters/v3/LogsListPresenter.server.ts:
- Around line 266-269: The code unsafely casts this.replica to PrismaClient when
constructing new RunsRepository; remove the assertion and fix types by making
RunsRepositoryOptions.prisma accept PrismaReplicaClient (preferred) or by
passing a value already typed as PrismaClient; specifically, update the
RunsRepositoryOptions.prisma type to PrismaReplicaClient and then construct
RunsRepository({ clickhouse: this.clickhouse, prisma: this.replica }) without
the as PrismaClient cast, ensuring references to PrismaClientOrTransaction and
this.replica are adjusted/typed consistently.
In
@apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx:
- Around line 169-180: The LogsList component's props type includes an unused
"filters" field; remove "filters" from the props type annotation for LogsList
(and from any places that construct its props type) or, if the component should
use filters, actually destructure and use it — most simply delete the "filters:
TaskRunListSearchFilters" line from the LogsList props type so the declared
props match the function parameters (and remove any passed-in "filters" at call
sites if present).
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx:
- Around line 26-29: The call to findEnvironmentBySlug uses arguments in the
wrong order (project.id, envParam, userId) but the helper expects (userId,
projectId, envSlug); fix by calling findEnvironmentBySlug with userId first,
then project.id, then envParam (i.e., swap the argument order) so environment
lookup and access control work correctly, and keep the existing null check that
throws the 404 Response if no environment is returned.
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsx:
- Around line 78-82: The code throws queryError directly which breaks the file's
Response-based error pattern; update the error handling after
queryBuilder.execute() to throw a Response instead (for example, throw new
Response(String(queryError?.message || queryError), { status: 500 })) so the
loader/route returns a consistent Response object like the other error branches;
locate the queryBuilder.execute() call and replace throw queryError with a
Response-wrapped error that includes the error message and appropriate status.
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts:
- Around line 15-19: parseLevelsFromUrl currently returns an empty array when
all provided "levels" are invalid; change it to return undefined in that case so
callers like LogsListPresenter don’t treat it as an explicit empty filter. After
building levelParams and early-returning when there are none, compute the
filtered array (using validLevels / LogLevel), and if the filtered array’s
length is 0 return undefined; otherwise return the filtered array.
- Around line 30-33: The call to findEnvironmentBySlug has its arguments
swapped; change the invocation from findEnvironmentBySlug(project.id, envParam,
userId) to pass userId before the slug (findEnvironmentBySlug(project.id,
userId, envParam)) so the function receives (projectId, userId, envSlug) as
intended; update the same pattern in the run-context loader if present to match.
In @apps/webapp/app/services/clickhouseInstance.server.ts:
- Around line 15-35: The logsQuerySettings object mixes string and numeric types
for ClickHouse settings; convert max_threads and max_execution_time to strings
to match other numeric entries. Update both occurrences inside
logsQuerySettings.list and logsQuerySettings.detail so that
env.CLICKHOUSE_LOGS_LIST_MAX_THREADS,
env.CLICKHOUSE_LOGS_LIST_MAX_EXECUTION_TIME,
env.CLICKHOUSE_LOGS_DETAIL_MAX_THREADS, and
env.CLICKHOUSE_LOGS_DETAIL_MAX_EXECUTION_TIME are passed via .toString() (and
keep the optional spread checks intact) so all numeric settings are consistently
string-typed for the ClickHouse client.
🧹 Nitpick comments (23)
apps/webapp/tailwind.config.js (1)
163-163: Consider using a distinct color for better visual distinction.The
logsicon color is set tocolors.blue[500], which is already used bytasks,projectSettings,orgSettings, anddocs. Using a unique color for logs would improve icon recognition and visual hierarchy in the UI.🎨 Suggested alternative colors
Consider one of these unused colors for logs:
-const logs = colors.blue[500]; +const logs = colors.cyan[500]; // Option 1: cyan for a distinct blue variantOr:
-const logs = colors.blue[500]; +const logs = colors.teal[500]; // Option 2: teal for logs/monitoring aestheticOr:
-const logs = colors.blue[500]; +const logs = colors.violet[500]; // Option 3: violet for clear distinctionapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts (1)
55-76: Order of friendly IDs may not match ClickHouse query ordering.The Prisma
findManyquery doesn't guarantee the returned order matches theinclause order. SincelistRunIdsreturns IDs ordered bycreated_at DESC, run_id DESC, and pagination relies on this ordering, the friendly IDs returned here may be in a different order.If ordering matters for the consumer (e.g., for pagination or display), consider preserving the order:
♻️ Proposed fix to preserve order
async listFriendlyRunIds(options: ListRunsOptions) { // First get internal IDs from ClickHouse const internalIds = await this.listRunIds(options); if (internalIds.length === 0) { return []; } // Then get friendly IDs from Prisma const runs = await this.options.prisma.taskRun.findMany({ where: { id: { in: internalIds, }, }, select: { + id: true, friendlyId: true, }, }); - return runs.map((run) => run.friendlyId); + // Preserve the order from ClickHouse + const idToFriendlyId = new Map(runs.map((run) => [run.id, run.friendlyId])); + return internalIds.map((id) => idToFriendlyId.get(id)).filter((id): id is string => id !== undefined); }apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx (1)
31-38:logIdis validated but otherwise unused; either remove the comment or actually deriverunIdfromlogId.
Right now the loader is effectively “fetch run context by runId”, withlogIdonly acting as a required route segment.apps/webapp/app/components/code/CodeBlock.tsx (2)
336-349: Search highlighting is only enabled in the non-modal render path.
If users expect the same highlight behavior in “Expand”, passsearchTermto the modal’sHighlightCodetoo.Also applies to: 392-404
563-617: Avoid per-token regex compilation + fixReact.ReactNodetyping usage.
You’re rebuilding the escaped string +RegExpfor every token; on large blocks this is avoidable overhead. Also preferReactNode(already imported) instead ofReact.ReactNodeto avoid relying on theReactnamespace being in scope.Proposed refactor (precompute regex once per render)
function HighlightCode({ theme, code, language, showLineNumbers, highlightLines, maxLineWidth, className, preClassName, isWrapped, searchTerm, }: HighlightCodeProps) { const [isLoaded, setIsLoaded] = useState(false); + const searchRegex = + searchTerm && searchTerm.trim() !== "" + ? new RegExp(searchTerm.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), "gi") + : undefined; useEffect(() => { Promise.all([ //@ts-ignore import("prismjs/components/prism-json"), //@ts-ignore import("prismjs/components/prism-typescript"), //@ts-ignore import("prismjs/components/prism-sql.js"), ]).then(() => setIsLoaded(true)); }, []); @@ <div className="flex-1"> {line.map((token, key) => { const tokenProps = getTokenProps({ token, key }); // Highlight search term matches in token - let content: React.ReactNode = token.content; - if (searchTerm && searchTerm.trim() !== "" && token.content) { - const escapedSearch = searchTerm.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - const regex = new RegExp(escapedSearch, "gi"); + let content: ReactNode = token.content; + if (searchRegex && token.content) { + searchRegex.lastIndex = 0; - const parts: React.ReactNode[] = []; + const parts: ReactNode[] = []; let lastIndex = 0; let match; let matchCount = 0; - while ((match = regex.exec(token.content)) !== null) { + while ((match = searchRegex.exec(token.content)) !== null) { if (match.index > lastIndex) { parts.push(token.content.substring(lastIndex, match.index)); } parts.push( <span key={`match-${matchCount}`} style={{ backgroundColor: "#facc15", color: "#000000", fontWeight: "500", }} > {match[0]} </span> ); - lastIndex = regex.lastIndex; + lastIndex = searchRegex.lastIndex; matchCount++; }apps/webapp/app/components/logs/LogsRunIdFilter.tsx (1)
74-81: Run ID validation is clear, but consider a single regex to avoid “startsWith + length” drift.
Not blocking, just reduces edge-case confusion as formats evolve.apps/webapp/app/components/logs/LogsSearchInput.tsx (1)
22-28: Missing dependencies in useEffect may cause stale closure issues.The effect depends on
textandisFocusedbut they're not in the dependency array. While omittingisFocusedis intentional (to avoid overwriting user input), omittingtextmeans the comparisonurlSearch !== textuses a staletextvalue after the user types.Consider using a ref to track focus state instead, which avoids the stale closure:
♻️ Suggested improvement using a ref for focus tracking
+ const isFocusedRef = useRef(false); const [text, setText] = useState(initialSearch); - const [isFocused, setIsFocused] = useState(false); // Update text when URL search param changes (only when not focused to avoid overwriting user input) useEffect(() => { const params = new URLSearchParams(location.search); const urlSearch = params.get("search") ?? ""; - if (urlSearch !== text && !isFocused) { + if (urlSearch !== text && !isFocusedRef.current) { setText(urlSearch); } - }, [location.search]); + }, [location.search, text]); // ... in JSX: - onFocus={() => setIsFocused(true)} - onBlur={() => setIsFocused(false)} + onFocus={() => { isFocusedRef.current = true; }} + onBlur={() => { isFocusedRef.current = false; }} - className={cn(isFocused && "placeholder:text-text-dimmed/70")} + // Note: You'll need useState for isFocused if you need the className reactivityapps/webapp/app/components/logs/LogDetailView.tsx (3)
36-60: Consider importing the type from the loader route.The
RunContextDatatype is defined inline here. If the loader at the/runendpoint exports this type, importing it would ensure type consistency and reduce duplication.
195-201: Missingfetcherin useEffect dependency array.The effect uses
fetcher.loadbutfetcheris not in the dependency array. WhilefetcherfromuseTypedFetcheris typically stable, ESLint's exhaustive-deps rule would flag this.♻️ Add fetcher to dependencies or use fetcher.load directly
useEffect(() => { if (!logId) return; fetcher.load( `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug}/logs/${encodeURIComponent(logId)}` ); - }, [organization.slug, project.slug, environment.slug, logId]); + }, [organization.slug, project.slug, environment.slug, logId, fetcher]);
449-455: Same missingfetcherdependency in RunTab.Similar to the parent component,
fetchershould be in the dependency array.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsx (1)
1-2: Consider usingtypedjsonfor type-safe client consumption.The sibling route
logs.$logId.tsxusestypedjsonfromremix-typedjsonfor better type inference on the client. Using plainjsonhere loses that benefit.♻️ Switch to typedjson
import { type LoaderFunctionArgs } from "@remix-run/server-runtime"; - import { json } from "@remix-run/node"; + import { typedjson } from "remix-typedjson"; // ... at the end: - return json({ spans }); + return typedjson({ spans });apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsx (1)
42-42: Unused variable in destructuring - intentional?The
runId(third part of the logId) is extracted but discarded. The presenter usesspanId,traceId, andstartTimewhich should be sufficient to uniquely identify the log. This appears intentional but could benefit from a brief comment.♻️ Add comment for clarity
- const [traceId, spanId, , startTime] = parts; + // runId (parts[2]) is embedded in the composite ID but not needed for the query + // as traceId + spanId + startTime uniquely identify the log entry + const [traceId, spanId, /* runId */, startTime] = parts;apps/webapp/app/components/logs/LogsLevelFilter.tsx (1)
35-52: Potential duplication with LogDetailView's getLevelColor.The
getLevelBadgeColorfunction here is nearly identical togetLevelColorinLogDetailView.tsx(lines 74-91). Consider extracting to a shared utility to maintain consistency.internal-packages/clickhouse/src/taskEvents.ts (2)
238-254: Consider usingz.unknown()instead ofz.any()for attributes.Using
z.any()on line 253 bypasses type checking entirely.z.unknown()is a safer alternative that still allows any value but requires type narrowing before use.♻️ Use z.unknown() for better type safety
duration: z.number().or(z.string()), metadata: z.string(), - attributes: z.any(), + attributes: z.unknown(),
284-300: Samez.any()concern for LogDetailV2Result attributes.Similar to LogsListResult, consider using
z.unknown()for theattributesfield on line 299.apps/webapp/app/presenters/v3/LogDetailPresenter.server.ts (2)
49-53: Unusedreplicaparameter in constructor.The
replicaparameter is injected but never used in thecallmethod. If it's intended for future use, consider documenting that intent. Otherwise, remove it to avoid confusion.♻️ If not needed, simplify the constructor
export class LogDetailPresenter { constructor( - private readonly replica: PrismaClientOrTransaction, private readonly clickhouse: ClickHouse ) {}
20-47: DuplicatekindToLevelfunction.This function is identical to the one in
LogsListPresenter.server.ts. Consider extracting it to a shared utility module to maintain DRY principles.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (2)
60-79: Consider using Remix'sResponsefor not-found errors.Throwing raw
Errorobjects for missing project/environment will result in 500 errors. Consider using proper HTTP responses for better error handling and user experience.♻️ Proposed fix using Response
const project = await findProjectBySlug(organizationSlug, projectParam, userId); if (!project) { - throw new Error("Project not found"); + throw new Response("Project not found", { status: 404 }); } const environment = await findEnvironmentBySlug(project.id, envParam, userId); if (!environment) { - throw new Error("Environment not found"); + throw new Response("Environment not found", { status: 404 }); }
197-208: Full page reload for debug toggle.Using
window.location.hreftriggers a full page reload. If a smoother transition is desired, consider using Remix'suseNavigateoruseSearchParams. However, if the intent is to ensure fresh data load, this approach is acceptable.apps/webapp/app/components/logs/LogsTable.tsx (1)
26-38: Unusedfiltersprop inLogsTableProps.The
filtersprop is defined in the type but is not destructured or used in theLogsTablecomponent. If not needed, consider removing it from the type definition.apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (3)
347-366: Fragile nanosecond string formatting.The string slicing approach (
fromNs.slice(0, 10) + "." + fromNs.slice(10)) assumes the nanosecond string is always at least 10 digits. While this holds for modern timestamps, consider using explicit formatting for robustness.♻️ More robust formatting
if (effectiveFrom) { const fromNs = convertDateToNanoseconds(effectiveFrom).toString(); + const fromFormatted = fromNs.padStart(19, '0'); queryBuilder.prewhere("inserted_at >= {insertedAtStart: DateTime64(3)}", { insertedAtStart: convertDateToClickhouseDateTime(effectiveFrom), }); queryBuilder.where("start_time >= {fromTime: String}", { - fromTime: fromNs.slice(0, 10) + "." + fromNs.slice(10), + fromTime: fromFormatted.slice(0, 10) + "." + fromFormatted.slice(10), }); }
393-430: Dynamic query construction usesanytypes.The level filtering logic uses
Record<string, any>and casts toanywhen callingqueryBuilder.where. While pragmatic for dynamic key construction, consider defining a more specific type if the query builder supports it.
489-499: Useconstinstead ofletfor non-reassigned variable.The
attributesvariable is never reassigned, so it should be declared withconst.♻️ Proposed fix
if (log.status === "ERROR" && log.attributes) { try { - let attributes = log.attributes as Record<string, any>; + const attributes = log.attributes as Record<string, any>; if (attributes?.error?.message && typeof attributes.error.message === 'string') { displayMessage = attributes.error.message; } } catch { // If attributes parsing fails, use the regular message } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlreferences/nextjs-realtime/.eslintrc.jsonis excluded by!references/**references/seed/.gitignoreis excluded by!references/**references/seed/package.jsonis excluded by!references/**references/seed/src/index.tsis excluded by!references/**references/seed/src/trigger/logSpammer.tsis excluded by!references/**references/seed/src/trigger/seedTask.tsis excluded by!references/**references/seed/src/trigger/spanSpammer.tsis excluded by!references/**references/seed/src/trigger/throwError.tsis excluded by!references/**references/seed/trigger.config.tsis excluded by!references/**references/seed/tsconfig.jsonis excluded by!references/**
📒 Files selected for processing (30)
apps/webapp/app/assets/icons/LogsIcon.tsxapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsLevelFilter.tsxapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/components/runs/v3/PacketDisplay.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/env.server.tsapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/presenters/v3/LogPresenter.server.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/services/clickhouseInstance.server.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/services/runsRepository/postgresRunsRepository.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/tailwind.config.jsinternal-packages/clickhouse/src/client/queryBuilder.tsinternal-packages/clickhouse/src/index.tsinternal-packages/clickhouse/src/taskEvents.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/tailwind.config.jsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/clickhouseInstance.server.tsapps/webapp/app/assets/icons/LogsIcon.tsxapps/webapp/app/components/runs/v3/PacketDisplay.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxinternal-packages/clickhouse/src/client/queryBuilder.tsapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsLevelFilter.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsxapps/webapp/app/services/runsRepository/postgresRunsRepository.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsxinternal-packages/clickhouse/src/taskEvents.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/code/CodeBlock.tsxinternal-packages/clickhouse/src/index.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/tailwind.config.jsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/clickhouseInstance.server.tsapps/webapp/app/assets/icons/LogsIcon.tsxapps/webapp/app/components/runs/v3/PacketDisplay.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxinternal-packages/clickhouse/src/client/queryBuilder.tsapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsLevelFilter.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsxapps/webapp/app/services/runsRepository/postgresRunsRepository.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsxinternal-packages/clickhouse/src/taskEvents.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/code/CodeBlock.tsxinternal-packages/clickhouse/src/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/clickhouseInstance.server.tsapps/webapp/app/assets/icons/LogsIcon.tsxapps/webapp/app/components/runs/v3/PacketDisplay.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxinternal-packages/clickhouse/src/client/queryBuilder.tsapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsLevelFilter.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsxapps/webapp/app/services/runsRepository/postgresRunsRepository.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsxinternal-packages/clickhouse/src/taskEvents.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/code/CodeBlock.tsxinternal-packages/clickhouse/src/index.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/clickhouseInstance.server.tsapps/webapp/app/assets/icons/LogsIcon.tsxapps/webapp/app/components/runs/v3/PacketDisplay.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsLevelFilter.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsxapps/webapp/app/services/runsRepository/postgresRunsRepository.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/code/CodeBlock.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/clickhouseInstance.server.tsapps/webapp/app/assets/icons/LogsIcon.tsxapps/webapp/app/components/runs/v3/PacketDisplay.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsLevelFilter.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsxapps/webapp/app/services/runsRepository/postgresRunsRepository.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/code/CodeBlock.tsx
apps/webapp/app/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Separate testable services from configuration files; follow the pattern of
realtimeClient.server.ts(testable service) andrealtimeClientGlobal.server.ts(configuration) in the webapp
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/clickhouseInstance.server.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/services/runsRepository/postgresRunsRepository.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/clickhouseInstance.server.tsapps/webapp/app/assets/icons/LogsIcon.tsxapps/webapp/app/components/runs/v3/PacketDisplay.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsLevelFilter.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsxapps/webapp/app/services/runsRepository/postgresRunsRepository.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/code/CodeBlock.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/clickhouseInstance.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsinternal-packages/clickhouse/src/client/queryBuilder.tsapps/webapp/app/services/runsRepository/postgresRunsRepository.server.tsapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsinternal-packages/clickhouse/src/taskEvents.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/env.server.tsinternal-packages/clickhouse/src/index.ts
🧠 Learnings (21)
📚 Learning: 2025-07-12T18:06:04.133Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
Applied to files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.tsapps/webapp/app/services/runsRepository/postgresRunsRepository.server.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/v3/services/**/*.server.{ts,tsx} : Organize services in the webapp following the pattern `app/v3/services/*/*.server.ts`
Applied to files:
apps/webapp/app/utils/pathBuilder.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.{ts,tsx} : Organize presenters in the webapp following the pattern `app/v3/presenters/*/*.server.ts` to move complex loader code into classes
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Use the Run Engine 2.0 from `internal/run-engine` for new run lifecycle code in the webapp instead of the legacy run engine
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure log level in trigger.config.ts using `logLevel` property
Applied to files:
apps/webapp/app/components/logs/LogsLevelFilter.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : Use zod for validation in packages/core and apps/webapp
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use logger methods (debug, log, info, warn, error) from `trigger.dev/sdk/v3` for structured logging in tasks
Applied to files:
internal-packages/clickhouse/src/taskEvents.tsinternal-packages/clickhouse/src/index.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Export tasks with unique IDs within the project to enable proper task discovery and execution
Applied to files:
internal-packages/clickhouse/src/taskEvents.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure OpenTelemetry instrumentations and exporters in trigger.config.ts for enhanced logging
Applied to files:
internal-packages/clickhouse/src/taskEvents.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schedules.task()` for scheduled/cron tasks instead of regular `task()`
Applied to files:
internal-packages/clickhouse/src/taskEvents.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `.withStreams()` to subscribe to realtime streams from task metadata in addition to run changes
Applied to files:
internal-packages/clickhouse/src/taskEvents.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Attach metadata to task runs using the metadata option when triggering, and access/update it inside runs using metadata functions
Applied to files:
internal-packages/clickhouse/src/taskEvents.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
Applied to files:
internal-packages/clickhouse/src/taskEvents.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use metadata methods (set, del, replace, append, remove, increment, decrement, stream, flush) to update metadata during task execution
Applied to files:
internal-packages/clickhouse/src/taskEvents.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Export every task, including subtasks
Applied to files:
internal-packages/clickhouse/src/taskEvents.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Use `useRun`, `useRealtimeRun` and other SWR/realtime hooks from `trigger.dev/react-hooks` for data fetching
Applied to files:
apps/webapp/app/components/navigation/SideMenu.tsx
📚 Learning: 2025-06-14T08:07:46.625Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2175
File: apps/webapp/app/services/environmentMetricsRepository.server.ts:202-207
Timestamp: 2025-06-14T08:07:46.625Z
Learning: In apps/webapp/app/services/environmentMetricsRepository.server.ts, the ClickHouse methods (getTaskActivity, getCurrentRunningStats, getAverageDurations) intentionally do not filter by the `tasks` parameter at the ClickHouse level, even though the tasks parameter is accepted by the public methods. This is done on purpose as there is not much benefit from adding that filtering at the ClickHouse layer.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: The webapp at apps/webapp is a Remix 2.1 application using Node.js v20
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use the `task()` function from `trigger.dev/sdk/v3` to define tasks with id and run properties
Applied to files:
internal-packages/clickhouse/src/index.ts
🧬 Code graph analysis (14)
apps/webapp/app/services/clickhouseInstance.server.ts (1)
apps/webapp/app/env.server.ts (1)
env(1307-1307)
apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts (1)
apps/webapp/app/services/runsRepository/runsRepository.server.ts (1)
ListRunsOptions(110-110)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx (5)
apps/webapp/app/services/session.server.ts (1)
requireUserId(25-35)apps/webapp/app/utils/pathBuilder.ts (1)
EnvironmentParamSchema(26-28)apps/webapp/app/models/project.server.ts (1)
findProjectBySlug(136-147)apps/webapp/app/models/runtimeEnvironment.server.ts (1)
findEnvironmentBySlug(116-145)apps/webapp/app/db.server.ts (1)
$replica(103-106)
apps/webapp/app/components/runs/v3/RunFilters.tsx (1)
apps/webapp/app/components/runs/v3/AIFilterInput.tsx (1)
AIFilterInput(28-141)
apps/webapp/app/components/logs/LogDetailView.tsx (15)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (1)
LogEntry(56-56)apps/webapp/app/components/primitives/Spinner.tsx (1)
Spinner(8-62)apps/webapp/app/components/primitives/Headers.tsx (2)
Header2(52-70)Header3(72-90)apps/webapp/app/components/primitives/Paragraph.tsx (1)
Paragraph(88-107)apps/webapp/app/utils/pathBuilder.ts (5)
v3RunSpanPath(322-333)v3RunsPath(264-273)v3RunPath(295-304)v3BatchPath(432-439)v3DeploymentVersionPath(485-492)apps/webapp/app/utils/cn.ts (1)
cn(77-79)apps/webapp/app/components/primitives/Tabs.tsx (1)
TabContainer(32-58)packages/core/src/v3/index.ts (2)
formatDurationNanoseconds(37-37)formatDurationMilliseconds(36-36)apps/webapp/app/components/runs/v3/PacketDisplay.tsx (1)
PacketDisplay(10-74)apps/webapp/app/components/primitives/Tooltip.tsx (1)
SimpleTooltip(141-141)apps/webapp/app/components/primitives/TextLink.tsx (1)
TextLink(29-94)apps/webapp/app/components/primitives/CopyableText.tsx (1)
CopyableText(8-98)apps/webapp/app/components/environments/EnvironmentLabel.tsx (1)
EnvironmentCombo(50-76)apps/webapp/app/components/runs/v3/RunTag.tsx (1)
RunTag(10-77)apps/webapp/app/components/MachineLabelCombo.tsx (1)
MachineLabelCombo(7-24)
apps/webapp/app/components/logs/LogsLevelFilter.tsx (6)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (1)
LogLevel(21-21)apps/webapp/app/hooks/useSearchParam.ts (1)
useSearchParams(7-64)apps/webapp/app/components/runs/v3/SharedFilters.tsx (2)
FilterMenuProvider(19-45)appliedSummary(387-397)apps/webapp/app/components/primitives/Select.tsx (5)
SelectProvider(378-382)SelectPopover(611-634)ComboBox(646-665)SelectList(428-441)shortcutFromIndex(556-573)apps/webapp/app/utils/cn.ts (1)
cn(77-79)apps/webapp/app/components/primitives/AppliedFilter.tsx (1)
AppliedFilter(32-75)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsx (4)
apps/webapp/app/utils/pathBuilder.ts (1)
EnvironmentParamSchema(26-28)apps/webapp/app/models/project.server.ts (1)
findProjectBySlug(136-147)apps/webapp/app/models/runtimeEnvironment.server.ts (1)
findEnvironmentBySlug(116-145)apps/webapp/app/services/clickhouseInstance.server.ts (1)
clickhouseClient(5-5)
apps/webapp/app/services/runsRepository/postgresRunsRepository.server.ts (2)
apps/webapp/app/services/runsRepository/runsRepository.server.ts (3)
ListRunsOptions(110-110)convertRunListInputOptionsToFilterRunsOptions(383-449)FilterRunsOptions(60-62)apps/webapp/app/db.server.ts (2)
Prisma(99-99)sqlDatabaseSchema(394-394)
apps/webapp/app/presenters/v3/LogDetailPresenter.server.ts (3)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (1)
LogLevel(21-21)internal-packages/clickhouse/src/index.ts (1)
ClickHouse(87-212)apps/webapp/app/v3/eventRepository/clickhouseEventRepository.server.ts (1)
convertClickhouseDateTime64ToJsDate(2011-2055)
apps/webapp/app/components/logs/LogsSearchInput.tsx (3)
apps/webapp/app/components/primitives/Input.tsx (1)
Input(125-125)apps/webapp/app/utils/cn.ts (1)
cn(77-79)apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
ShortcutKey(38-55)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.tsx (5)
apps/webapp/app/services/session.server.ts (1)
requireUserId(25-35)apps/webapp/app/models/project.server.ts (1)
findProjectBySlug(136-147)apps/webapp/app/models/runtimeEnvironment.server.ts (1)
findEnvironmentBySlug(116-145)apps/webapp/app/presenters/v3/LogDetailPresenter.server.ts (1)
LogDetailPresenter(49-138)apps/webapp/app/db.server.ts (1)
$replica(103-106)
internal-packages/clickhouse/src/taskEvents.ts (1)
internal-packages/clickhouse/src/client/types.ts (1)
ClickhouseReader(60-199)
apps/webapp/app/components/primitives/Table.tsx (2)
apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
variants(14-19)apps/webapp/app/utils/cn.ts (1)
cn(77-79)
internal-packages/clickhouse/src/index.ts (1)
internal-packages/clickhouse/src/taskEvents.ts (2)
getLogsListQueryBuilder(258-281)getLogDetailQueryBuilder(304-327)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
| function BlankState({ isLoading }: { isLoading?: boolean }) { | ||
| if (isLoading) return <TableBlankRow colSpan={7}></TableBlankRow>; | ||
|
|
||
| return ( | ||
| <TableBlankRow colSpan={7}> | ||
| <div className="flex flex-col items-center justify-center gap-6"> | ||
| <Paragraph className="w-auto" variant="base/bright"> | ||
| No logs match your filters. Try refreshing or modifying your filters. | ||
| </Paragraph> | ||
| <div className="flex items-center gap-2"> | ||
| <Button | ||
| LeadingIcon={ArrowPathIcon} | ||
| variant="tertiary/medium" | ||
| onClick={() => { | ||
| window.location.reload(); | ||
| }} | ||
| > | ||
| Refresh | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| </TableBlankRow> | ||
| ); |
There was a problem hiding this comment.
Incorrect colSpan value in BlankState.
The table has 6 columns (Time, Run, Task, Level, Duration, Message), but BlankState uses colSpan={7}. This should be colSpan={6} for correct alignment.
🐛 Proposed fix
function BlankState({ isLoading }: { isLoading?: boolean }) {
- if (isLoading) return <TableBlankRow colSpan={7}></TableBlankRow>;
+ if (isLoading) return <TableBlankRow colSpan={6}></TableBlankRow>;
return (
- <TableBlankRow colSpan={7}>
+ <TableBlankRow colSpan={6}>
<div className="flex flex-col items-center justify-center gap-6">📝 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.
| function BlankState({ isLoading }: { isLoading?: boolean }) { | |
| if (isLoading) return <TableBlankRow colSpan={7}></TableBlankRow>; | |
| return ( | |
| <TableBlankRow colSpan={7}> | |
| <div className="flex flex-col items-center justify-center gap-6"> | |
| <Paragraph className="w-auto" variant="base/bright"> | |
| No logs match your filters. Try refreshing or modifying your filters. | |
| </Paragraph> | |
| <div className="flex items-center gap-2"> | |
| <Button | |
| LeadingIcon={ArrowPathIcon} | |
| variant="tertiary/medium" | |
| onClick={() => { | |
| window.location.reload(); | |
| }} | |
| > | |
| Refresh | |
| </Button> | |
| </div> | |
| </div> | |
| </TableBlankRow> | |
| ); | |
| function BlankState({ isLoading }: { isLoading?: boolean }) { | |
| if (isLoading) return <TableBlankRow colSpan={6}></TableBlankRow>; | |
| return ( | |
| <TableBlankRow colSpan={6}> | |
| <div className="flex flex-col items-center justify-center gap-6"> | |
| <Paragraph className="w-auto" variant="base/bright"> | |
| No logs match your filters. Try refreshing or modifying your filters. | |
| </Paragraph> | |
| <div className="flex items-center gap-2"> | |
| <Button | |
| LeadingIcon={ArrowPathIcon} | |
| variant="tertiary/medium" | |
| onClick={() => { | |
| window.location.reload(); | |
| }} | |
| > | |
| Refresh | |
| </Button> | |
| </div> | |
| </div> | |
| </TableBlankRow> | |
| ); |
🤖 Prompt for AI Agents
In @apps/webapp/app/components/logs/LogsTable.tsx around lines 267 - 289, The
BlankState component uses TableBlankRow with colSpan={7} but the table has only
6 columns; update both occurrences of TableBlankRow inside the BlankState
function to use colSpan={6} (locate the BlankState function and the two
TableBlankRow usages) so the empty-state row aligns correctly with the table
columns.
| {(isAdmin || user.isImpersonating) && ( | ||
| <SideMenuItem | ||
| name="Logs" | ||
| icon={LogsIcon} | ||
| activeIconColor="text-logs" | ||
| to={v3LogsPath(organization, project, environment)} | ||
| data-action="logs" | ||
| badge={<AlphaBadge />} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for useHasAdminAccess hook definition
rg -n "useHasAdminAccess" apps/webapp/app --type=ts --type=tsx -A 15 | head -100Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
# Search for isAdmin and isImpersonating definitions in the context
rg -n "isAdmin|isImpersonating" apps/webapp/app/components/navigation --type=ts --type=tsx -B 2 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
# Check the entire SideMenu file to see the context
cat -n apps/webapp/app/components/navigation/SideMenu.tsx | head -300Repository: triggerdotdev/trigger.dev
Length of output: 12808
🏁 Script executed:
rg -n "useHasAdminAccess" apps/webapp/app --type ts -A 10Repository: triggerdotdev/trigger.dev
Length of output: 20925
🏁 Script executed:
# Also search for where it's defined
rg -n "export.*useHasAdminAccess|function useHasAdminAccess" apps/webapp --type ts -B 2 -A 10Repository: triggerdotdev/trigger.dev
Length of output: 574
Simplify the Logs visibility condition to avoid redundancy.
useHasAdminAccess() already returns Boolean(user?.admin) || isImpersonating (from apps/webapp/app/hooks/useUser.ts), so the condition (isAdmin || user.isImpersonating) is tautological. Use {isAdmin && ( instead to avoid duplication and maintain a single source of truth.
🤖 Prompt for AI Agents
In @apps/webapp/app/components/navigation/SideMenu.tsx around lines 272 - 281,
The Logs visibility condition is redundant because useHasAdminAccess() already
accounts for impersonation; replace the current conditional (isAdmin ||
user.isImpersonating) around the SideMenuItem for "Logs" with just isAdmin to
rely on the single source of truth (symbol: isAdmin) and remove the tautological
check while keeping the same SideMenuItem (name="Logs", icon=LogsIcon,
to={v3LogsPath(...)}).
...p/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
Show resolved
Hide resolved
| const environment = await findEnvironmentBySlug(project.id, envParam, userId); | ||
| if (!environment) { | ||
| throw new Response("Environment not found", { status: 404 }); | ||
| } |
There was a problem hiding this comment.
Critical: findEnvironmentBySlug arguments are in the wrong order (likely breaks env lookup / access control).
The existing helper signature (per repo snippet) is (userId, projectId, envSlug), but this calls (projectId, envSlug, userId).
Proposed fix
- const environment = await findEnvironmentBySlug(project.id, envParam, userId);
+ const environment = await findEnvironmentBySlug(userId, project.id, envParam);📝 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.
| const environment = await findEnvironmentBySlug(project.id, envParam, userId); | |
| if (!environment) { | |
| throw new Response("Environment not found", { status: 404 }); | |
| } | |
| const environment = await findEnvironmentBySlug(userId, project.id, envParam); | |
| if (!environment) { | |
| throw new Response("Environment not found", { status: 404 }); | |
| } |
🤖 Prompt for AI Agents
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx
around lines 26 - 29, The call to findEnvironmentBySlug uses arguments in the
wrong order (project.id, envParam, userId) but the helper expects (userId,
projectId, envSlug); fix by calling findEnvironmentBySlug with userId first,
then project.id, then envParam (i.e., swap the argument order) so environment
lookup and access control work correctly, and keep the existing null check that
throws the 404 Response if no environment is returned.
| const [queryError, records] = await queryBuilder.execute(); | ||
|
|
||
| if (queryError) { | ||
| throw queryError; | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling - throwing raw error instead of Response.
When queryError occurs, it's thrown directly rather than wrapped in a Response. This differs from the pattern used elsewhere in this file (lines 42, 47, 52, 62) and could result in unexpected error formats reaching the client.
🔧 Suggested fix
if (queryError) {
- throw queryError;
+ console.error("Failed to fetch spans:", queryError);
+ throw new Response("Failed to fetch spans", { status: 500 });
}📝 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.
| const [queryError, records] = await queryBuilder.execute(); | |
| if (queryError) { | |
| throw queryError; | |
| } | |
| const [queryError, records] = await queryBuilder.execute(); | |
| if (queryError) { | |
| console.error("Failed to fetch spans:", queryError); | |
| throw new Response("Failed to fetch spans", { status: 500 }); | |
| } |
🤖 Prompt for AI Agents
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.spans.tsx
around lines 78 - 82, The code throws queryError directly which breaks the
file's Response-based error pattern; update the error handling after
queryBuilder.execute() to throw a Response instead (for example, throw new
Response(String(queryError?.message || queryError), { status: 500 })) so the
loader/route returns a consistent Response object like the other error branches;
locate the queryBuilder.execute() call and replace throw queryError with a
Response-wrapped error that includes the error message and appropriate status.
| function parseLevelsFromUrl(url: URL): LogLevel[] | undefined { | ||
| const levelParams = url.searchParams.getAll("levels").filter((v) => v.length > 0); | ||
| if (levelParams.length === 0) return undefined; | ||
| return levelParams.filter((l): l is LogLevel => validLevels.includes(l as LogLevel)); | ||
| } |
There was a problem hiding this comment.
If all provided levels are invalid, return undefined (not []).
Returning [] can accidentally mean “filter to no levels” depending on how LogsListPresenter interprets it.
Proposed fix
function parseLevelsFromUrl(url: URL): LogLevel[] | undefined {
const levelParams = url.searchParams.getAll("levels").filter((v) => v.length > 0);
if (levelParams.length === 0) return undefined;
- return levelParams.filter((l): l is LogLevel => validLevels.includes(l as LogLevel));
+ const levels = levelParams.filter((l): l is LogLevel => validLevels.includes(l as LogLevel));
+ return levels.length > 0 ? levels : undefined;
}🤖 Prompt for AI Agents
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
around lines 15 - 19, parseLevelsFromUrl currently returns an empty array when
all provided "levels" are invalid; change it to return undefined in that case so
callers like LogsListPresenter don’t treat it as an explicit empty filter. After
building levelParams and early-returning when there are none, compute the
filtered array (using validLevels / LogLevel), and if the filtered array’s
length is 0 return undefined; otherwise return the filtered array.
| const environment = await findEnvironmentBySlug(project.id, envParam, userId); | ||
| if (!environment) { | ||
| throw new Response("Environment not found", { status: 404 }); | ||
| } |
There was a problem hiding this comment.
Critical: findEnvironmentBySlug arguments are in the wrong order (same issue as the run-context loader).
Proposed fix
- const environment = await findEnvironmentBySlug(project.id, envParam, userId);
+ const environment = await findEnvironmentBySlug(userId, project.id, envParam);🤖 Prompt for AI Agents
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
around lines 30 - 33, The call to findEnvironmentBySlug has its arguments
swapped; change the invocation from findEnvironmentBySlug(project.id, envParam,
userId) to pass userId before the slug (findEnvironmentBySlug(project.id,
userId, envParam)) so the function receives (projectId, userId, envSlug) as
intended; update the same pattern in the run-context loader if present to match.
| // Build logs query settings from environment variables | ||
| const logsQuerySettings = { | ||
| list: { | ||
| max_memory_usage: env.CLICKHOUSE_LOGS_LIST_MAX_MEMORY_USAGE.toString(), | ||
| max_bytes_before_external_sort: env.CLICKHOUSE_LOGS_LIST_MAX_BYTES_BEFORE_EXTERNAL_SORT.toString(), | ||
| max_threads: env.CLICKHOUSE_LOGS_LIST_MAX_THREADS, | ||
| ...(env.CLICKHOUSE_LOGS_LIST_MAX_ROWS_TO_READ && { | ||
| max_rows_to_read: env.CLICKHOUSE_LOGS_LIST_MAX_ROWS_TO_READ.toString(), | ||
| }), | ||
| ...(env.CLICKHOUSE_LOGS_LIST_MAX_EXECUTION_TIME && { | ||
| max_execution_time: env.CLICKHOUSE_LOGS_LIST_MAX_EXECUTION_TIME, | ||
| }), | ||
| }, | ||
| detail: { | ||
| max_memory_usage: env.CLICKHOUSE_LOGS_DETAIL_MAX_MEMORY_USAGE.toString(), | ||
| max_threads: env.CLICKHOUSE_LOGS_DETAIL_MAX_THREADS, | ||
| ...(env.CLICKHOUSE_LOGS_DETAIL_MAX_EXECUTION_TIME && { | ||
| max_execution_time: env.CLICKHOUSE_LOGS_DETAIL_MAX_EXECUTION_TIME, | ||
| }), | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "ClickHouse\|logsQuerySettings" | head -20Repository: triggerdotdev/trigger.dev
Length of output: 1436
🏁 Script executed:
# Search for the clickhouseInstance file and surrounding context
find . -type f -name "clickhouseInstance.server.ts"Repository: triggerdotdev/trigger.dev
Length of output: 124
🏁 Script executed:
# Look for type definitions related to query settings
rg "max_threads|max_memory_usage|LogsQuerySettings" --type ts -B 2 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 3006
🏁 Script executed:
# Check the ClickHouse package/client to understand expected types
find . -path "*/clickhouse/*" -type f -name "*.ts" | head -20Repository: triggerdotdev/trigger.dev
Length of output: 726
🏁 Script executed:
# Get the ClickHouseSettings type definition
rg "type ClickHouseSettings" --type ts -A 20 internal-packages/Repository: triggerdotdev/trigger.dev
Length of output: 1775
🏁 Script executed:
# Check the tsql constants for complete type definition
cat -n internal-packages/tsql/src/query/constants.ts | head -50Repository: triggerdotdev/trigger.dev
Length of output: 1997
🏁 Script executed:
# Check how the settings are used in the ClickHouse client
rg "logsQuerySettings" --type ts -B 5 -A 10 internal-packages/clickhouse/src/Repository: triggerdotdev/trigger.dev
Length of output: 3563
🏁 Script executed:
# Check the complete LogsQuerySettings and ClickHouseSettings definition in index.ts
cat -n internal-packages/clickhouse/src/index.ts | grep -A 10 "export type LogsQuerySettings"Repository: triggerdotdev/trigger.dev
Length of output: 392
🏁 Script executed:
# Check how the settings are passed to the query builders
rg "getLogsListQueryBuilder|getLogDetailQueryBuilder" --type ts -B 3 -A 15 internal-packages/clickhouse/src/Repository: triggerdotdev/trigger.dev
Length of output: 5119
🏁 Script executed:
# Check the actual usage of logsQuerySettings in the query builders to see how they're used
fd "logsListQueryBuilder\|logDetailQueryBuilder" --type ts --exec grep -l "logsQuerySettings\|max_threads\|max_memory" {} \;Repository: triggerdotdev/trigger.dev
Length of output: 238
🏁 Script executed:
# Look for the implementation of these query builders
rg "getLogsListQueryBuilder|getLogDetailQueryBuilder" --type ts -B 2 -A 30Repository: triggerdotdev/trigger.dev
Length of output: 7311
🏁 Script executed:
# Check what ClickHouseSettings is from @clickhouse/client
rg "ClickHouseSettings" internal-packages/clickhouse/src/client/ -B 2 -A 10 | head -80Repository: triggerdotdev/trigger.dev
Length of output: 6889
🏁 Script executed:
# Check the package.json of clickhouse package to understand the @clickhouse/client version
cat internal-packages/clickhouse/package.json | grep -A 5 -B 5 "@clickhouse/client"Repository: triggerdotdev/trigger.dev
Length of output: 400
🏁 Script executed:
# Look for how ClickHouseSettings is actually used in queryBuilder
rg "queryBuilder|ClickHouseSettings" internal-packages/clickhouse/src/client/queryBuilder.ts -A 5 -B 2 | head -100Repository: triggerdotdev/trigger.dev
Length of output: 1488
🏁 Script executed:
# Check if there are tests that show how settings are used
find . -type f -name "*.test.ts" -exec grep -l "ClickHouseSettings\|max_threads\|max_memory" {} \;Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Look for how settings are passed to the query execution in the client
rg "settings" internal-packages/clickhouse/src/client/queryBuilder.ts -A 3 -B 1 | grep -A 5 -B 5 "this.settings"Repository: triggerdotdev/trigger.dev
Length of output: 620
🏁 Script executed:
# Check the actual client.ts to see how settings are passed to queries
rg "settings" internal-packages/clickhouse/src/client/client.ts -A 3 -B 1 | head -80Repository: triggerdotdev/trigger.dev
Length of output: 2400
🏁 Script executed:
# Check the actual clickhouseInstance.server.ts file to see the full context
cat -n apps/webapp/app/services/clickhouseInstance.server.ts | head -50Repository: triggerdotdev/trigger.dev
Length of output: 2234
🏁 Script executed:
# Search for how environment variables are defined/typed in the env.server.ts
rg "CLICKHOUSE_LOGS_LIST_MAX_THREADS|CLICKHOUSE_LOGS_DETAIL_MAX_THREADS" --type ts -B 2 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 2322
🏁 Script executed:
# Check the env.server.ts to see the types of these env variables
find . -type f -name "env.server.ts" | xargs grep -l "CLICKHOUSE"Repository: triggerdotdev/trigger.dev
Length of output: 100
🏁 Script executed:
# Check if there's a types file that shows what ClickHouseSettings expects
rg "type ClickHouseSettings\|interface ClickHouseSettings" --type ts -A 30Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Look for any type definition files in node_modules or check if it's documented
find . -path "*/node_modules/@clickhouse/client/*" -name "*.d.ts" 2>/dev/null | head -5Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Check how the settings are actually serialized/used - look at the client code more carefully
rg "clickhouse_settings" internal-packages/clickhouse/src/client/client.ts -B 5 -A 10Repository: triggerdotdev/trigger.dev
Length of output: 3356
🏁 Script executed:
# Check if there are TypeScript errors by looking for type checking in the project
find . -name "tsconfig.json" -path "*/apps/webapp/*" | head -1Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
# Look for any tests or examples that show ClickHouseSettings usage
rg "max_memory_usage|max_threads" --type ts -B 3 -A 3 apps/webapp/Repository: triggerdotdev/trigger.dev
Length of output: 1793
🏁 Script executed:
# Check the actual @clickhouse/client exports to understand ClickHouseSettings type
cat package.json | grep -A 20 "dependencies"Repository: triggerdotdev/trigger.dev
Length of output: 1048
🏁 Script executed:
# Let me look at actual ClickHouse client documentation or type signatures by examining what's actually imported
rg "from \"@clickhouse/client\"" internal-packages/clickhouse/src/ -A 5 -B 5Repository: triggerdotdev/trigger.dev
Length of output: 6057
🏁 Script executed:
# Let me check if the env variables actually parse correctly and have the right types
cat -n apps/webapp/app/env.server.ts | grep -A 20 "CLICKHOUSE_LOGS"Repository: triggerdotdev/trigger.dev
Length of output: 2167
🏁 Script executed:
# Check if there are any build/type check errors reported
rg "error|ERROR" .github/workflows/ 2>/dev/null | grep -i typescript | head -10Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Look at the actual @clickhouse/client package types by searching for type stubs
find . -name "*.d.ts" -path "*clickhouse*" -type f | head -20Repository: triggerdotdev/trigger.dev
Length of output: 51
Inconsistent type conversion for max_threads and max_execution_time — should match the pattern used for other numeric settings.
The code converts most numeric settings to strings (max_memory_usage, max_bytes_before_external_sort, max_rows_to_read all use .toString()), but max_threads (lines 20, 30) and max_execution_time (lines 25, 32) are passed as numbers without conversion. For consistency and to ensure the ClickHouse client receives uniform types, convert these to strings as well.
Settings pattern inconsistency
// Inconsistent handling of numeric settings
list: {
max_memory_usage: env.CLICKHOUSE_LOGS_LIST_MAX_MEMORY_USAGE.toString(), // string
max_bytes_before_external_sort: env.CLICKHOUSE_LOGS_LIST_MAX_BYTES_BEFORE_EXTERNAL_SORT.toString(), // string
max_threads: env.CLICKHOUSE_LOGS_LIST_MAX_THREADS, // number ← inconsistent
...(env.CLICKHOUSE_LOGS_LIST_MAX_ROWS_TO_READ && {
max_rows_to_read: env.CLICKHOUSE_LOGS_LIST_MAX_ROWS_TO_READ.toString(), // string
}),
...(env.CLICKHOUSE_LOGS_LIST_MAX_EXECUTION_TIME && {
max_execution_time: env.CLICKHOUSE_LOGS_LIST_MAX_EXECUTION_TIME, // number ← inconsistent
}),
}🤖 Prompt for AI Agents
In @apps/webapp/app/services/clickhouseInstance.server.ts around lines 15 - 35,
The logsQuerySettings object mixes string and numeric types for ClickHouse
settings; convert max_threads and max_execution_time to strings to match other
numeric entries. Update both occurrences inside logsQuerySettings.list and
logsQuerySettings.detail so that env.CLICKHOUSE_LOGS_LIST_MAX_THREADS,
env.CLICKHOUSE_LOGS_LIST_MAX_EXECUTION_TIME,
env.CLICKHOUSE_LOGS_DETAIL_MAX_THREADS, and
env.CLICKHOUSE_LOGS_DETAIL_MAX_EXECUTION_TIME are passed via .toString() (and
keep the optional spread checks intact) so all numeric settings are consistently
string-typed for the ClickHouse client.
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
reduced spinner delay removed execution time column removed metadata from side panel
PR Review: feat: tri-6738 Create aggregated logs pageThis is a substantial PR (~3,515 lines added) that introduces a new aggregated logs page feature. Overall, the implementation is well-structured and follows existing patterns in the codebase. Below are my findings: Strengths
Potential Bugs / Issues1. Missing ESLint exhaustive-deps in useEffect hooksFile: The Same issue at line ~372 in 2. Unused importFile:
3. Type safety issue with any typeFile: Using 4. Unnecessary type cast in LogsListPresenterFile: The 5. Inconsistent empty response in LogPresenterFile: Performance Considerations1. Potential N+1 query pattern in Run tabFile: Each time the Run tab is clicked, it fetches run details. If users rapidly switch between logs, this could generate many requests. Consider:
2. Large run ID list in ClickHouse queryFile:
3. No virtualization for logs tableFor very long log lists with infinite scroll, consider using virtualization (e.g., react-window) to improve rendering performance. 4. Regex in render loopFile: The search highlighting logic creates a new regex on every token during render. For large log messages with many tokens, consider memoizing the regex or using a more efficient approach. Security Considerations1. SQL/Query Injection - Properly handledThe ClickHouse queries use parameterized queries, which is correct. 2. Access Control - Properly implementedThe logs page requires admin access and properly validates project/environment access through 3. URL manipulationFile: The Test CoverageNo test files were included in this PR. Consider adding tests for:
Code Quality Suggestions1. Duplicated
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/components/code/CodeBlock.tsx (1)
392-403: MissingsearchTermprop in modal view.The main view's
HighlightCodecomponent (line 347) receives thesearchTermprop, but the modal/expanded view'sHighlightCodecomponent does not. This creates an inconsistent user experience where search highlighting disappears when the code is expanded.🔧 Proposed fix
{shouldHighlight ? ( <HighlightCode theme={theme} code={code} language={language} showLineNumbers={showLineNumbers} highlightLines={highlightLines} maxLineWidth={maxLineWidth} className="min-h-full" preClassName="text-sm" isWrapped={isWrapped} + searchTerm={searchTerm} /> ) : (
🤖 Fix all issues with AI agents
In @apps/webapp/app/components/logs/LogDetailView.tsx:
- Around line 541-546: runData.machinePreset may be null or an arbitrary string,
so don’t cast it directly to MachinePresetName; add a validation step (e.g., an
isValidMachinePreset guard or lookup against the allowed MachinePresetName
values) and only pass a known preset into MachineLabelCombo (or pass undefined/a
safe fallback like "unknown") so MachineLabelCombo never receives null/invalid
presets; update the JSX around MachineLabelCombo to use the validated value
instead of casting runData.machinePreset as MachinePresetName.
- Around line 314-335: The code in DetailsTab incorrectly checks log.status to
detect an ERROR and may pass undefined to PacketDisplay; change the conditional
to use log.level === "ERROR" and extract the error message safely from
logWithExtras.attributes?.error?.message, falling back to the original
log.message when that field is missing or falsy; update both occurrences (the
block setting message and the similar logic at lines ~346-354) to use optional
chaining and a fallback so PacketDisplay always receives a defined string.
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx:
- Around line 593-595: The form/action targets use runParam instead of the
actual displayed run's identifier, which can point actions at the wrong run when
RunBody renders a different run; update all places that build links/forms (e.g.,
calls to v3RunIdempotencyKeyResetPath and any run logs path builders around the
Reset/Logs UI) to use run.friendlyId (and validate run is defined) instead of
runParam so the reset/logs actions operate on the shown run; apply the same
replacement for the other occurrences noted (the block around lines 949-966) and
ensure any UI that falls back to runParam only does so when run.friendlyId is
not available.
🧹 Nitpick comments (4)
apps/webapp/app/components/code/CodeBlock.tsx (2)
566-605: Implementation is solid; consider extracting highlight styles.The search highlighting logic correctly escapes regex special characters, handles case-insensitive matching, and safely constructs highlighted content. However, the hardcoded inline styles (lines 586-588) may not align with the existing theme system.
♻️ Optional: Extract highlight styles to theme or constants
Consider defining highlight colors in the theme or as constants for easier customization:
+const SEARCH_HIGHLIGHT_STYLES = { + backgroundColor: "#facc15", + color: "#000000", + fontWeight: "500", +} as const; + function HighlightCode({ ... }) { ... parts.push( <span key={`match-${matchCount}`} - style={{ - backgroundColor: "#facc15", - color: "#000000", - fontWeight: "500", - }} + style={SEARCH_HIGHLIGHT_STYLES} > {match[0]} </span> );
350-371: Consider adding search highlighting to non-highlighted fallback.When
shouldHighlightis false (for files with >1000 lines), the code falls back to a plain<pre>tag without any search highlighting. While this is a performance optimization, users searching large log files would still benefit from basic search highlighting even without syntax highlighting.This affects both the main view (lines 350-371) and modal view (lines 404-413). Consider implementing a simple text-based search highlighting for these fallback cases to maintain consistency with the search feature.
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx (1)
576-578: Idempotency key wrapping improves readability, but consider keeping “copy” affordance.
If users frequently copy the idempotency key,CopyableText(or similar) might be preferable to a plain div.apps/webapp/app/components/logs/LogDetailView.tsx (1)
34-58: Avoid stringly-typedRunContextDatathat forces unsafe casts in the UI.
RunContextData.run.status/machinePresetare typed asstring/string | null, then cast toTaskRunStatus/MachinePresetNameat use sites. Consider typing them accurately (and/or validating the fetcher payload with zod) soTaskRunStatusCombo/MachineLabelCombocan’t receive invalid values at runtime. As per coding guidelines, apps/webapp should prefer zod for validation.Also applies to: 415-424, 541-546
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/components/primitives/Popover.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/components/logs/LogsTable.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Popover.tsxapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Popover.tsxapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Popover.tsxapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Popover.tsxapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Popover.tsxapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Popover.tsxapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Scope idempotency keys globally or to current run using the scope parameter
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `idempotencyKeyTTL` option to define a time window during which duplicate triggers return the original run
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
🧬 Code graph analysis (2)
apps/webapp/app/components/logs/LogDetailView.tsx (13)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (1)
LogEntry(56-56)apps/webapp/app/hooks/useProject.tsx (1)
useProject(20-24)apps/webapp/app/hooks/useEnvironment.tsx (1)
useEnvironment(19-23)apps/webapp/app/components/primitives/Headers.tsx (2)
Header2(52-70)Header3(72-90)apps/webapp/app/utils/pathBuilder.ts (3)
v3RunSpanPath(322-333)v3DeploymentVersionPath(485-492)v3RunsPath(264-273)apps/webapp/app/utils/cn.ts (1)
cn(77-79)apps/webapp/app/components/primitives/Tabs.tsx (2)
TabContainer(32-58)TabButton(164-220)apps/webapp/app/components/primitives/DateTime.tsx (1)
DateTime(54-99)apps/webapp/app/components/runs/v3/PacketDisplay.tsx (1)
PacketDisplay(10-74)apps/webapp/app/components/primitives/CopyableText.tsx (1)
CopyableText(8-98)apps/webapp/app/components/runs/v3/RunTag.tsx (1)
RunTag(10-77)apps/webapp/app/utils/numberFormatter.ts (1)
formatCurrencyAccurate(48-50)packages/core/src/v3/index.ts (1)
formatDurationMilliseconds(36-36)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx (1)
apps/webapp/app/utils/pathBuilder.ts (3)
v3RunIdempotencyKeyResetPath(344-353)v3LogsPath(458-464)v3RunDownloadLogsPath(318-320)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
apps/webapp/app/components/primitives/Popover.tsx (2)
68-68: LGTM! Clean API design with backward compatibility.The optional
openInNewTabprop with afalsedefault ensures existing usage remains unaffected while enabling new functionality.Also applies to: 82-82
107-108: Excellent security consideration!The implementation correctly includes
rel="noopener noreferrer"alongsidetarget="_blank", which prevents tabnapping attacks. Clean conditional logic that only affects Link-based navigation.apps/webapp/app/components/code/CodeBlock.tsx (2)
68-69: LGTM! Clear prop addition for search highlighting.The
searchTermprop is well-documented and properly typed as optional, ensuring backward compatibility.
245-245: Good defensive coding practice.The optional chaining and nullish coalescing operator ensure safe handling of potentially undefined
codevalues.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx (2)
70-83: Importingv3LogsPathlooks fine.
No issues with the additional pathBuilder import.
949-968: Thefromparameter is correctly formatted in milliseconds and properly handled by the logs route.The code passes
from=${new Date(run.createdAt).getTime() - 60000}(milliseconds minus 60 seconds), and the logs route'stimeFiltersfunction correctly handles numericfromvalues by passing them tonew Date(from), which interprets numbers as milliseconds since epoch. TherunIdparameter is also properly extracted and used in the query filter. No changes needed.Likely an incorrect or invalid review comment.
apps/webapp/app/components/logs/LogDetailView.tsx (2)
239-245: Remove this comment. The code is correct:log.runIdis already a friendlyId, not a database id. The LogsListPresenter fetches friendly IDs from Postgres vialistFriendlyRunIds()and uses them to filter ClickHouse queries, so therun_idfield in the returned logs contains the friendly ID.Likely an incorrect or invalid review comment.
189-203: Type mismatch between loader return andinitialLog.The loader returns
LogDetail(which includesmetadata,attributes,rawMetadata,rawAttributes), butinitialLogis typed asLogEntry(which lacks these fields). This creates a type inconsistency inconst log = fetcher.data ?? initialLog;and could cause issues if code downstream depends on these extra fields when using the fallback.
| <Property.Item> | ||
| <Property.Label>Machine</Property.Label> | ||
| <Property.Value className="-ml-0.5"> | ||
| <MachineLabelCombo preset={runData.machinePreset as MachinePresetName} /> | ||
| </Property.Value> | ||
| </Property.Item> |
There was a problem hiding this comment.
Don’t pass null/unknown presets into MachineLabelCombo.
runData.machinePreset can be null (and may be an arbitrary string). Casting bypasses safety and can crash or mis-render if the component assumes a known preset.
Proposed fix
<Property.Item>
<Property.Label>Machine</Property.Label>
<Property.Value className="-ml-0.5">
- <MachineLabelCombo preset={runData.machinePreset as MachinePresetName} />
+ {runData.machinePreset ? (
+ <MachineLabelCombo preset={runData.machinePreset as MachinePresetName} />
+ ) : (
+ "–"
+ )}
</Property.Value>
</Property.Item>📝 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.
| <Property.Item> | |
| <Property.Label>Machine</Property.Label> | |
| <Property.Value className="-ml-0.5"> | |
| <MachineLabelCombo preset={runData.machinePreset as MachinePresetName} /> | |
| </Property.Value> | |
| </Property.Item> | |
| <Property.Item> | |
| <Property.Label>Machine</Property.Label> | |
| <Property.Value className="-ml-0.5"> | |
| {runData.machinePreset ? ( | |
| <MachineLabelCombo preset={runData.machinePreset as MachinePresetName} /> | |
| ) : ( | |
| "–" | |
| )} | |
| </Property.Value> | |
| </Property.Item> |
🤖 Prompt for AI Agents
In @apps/webapp/app/components/logs/LogDetailView.tsx around lines 541 - 546,
runData.machinePreset may be null or an arbitrary string, so don’t cast it
directly to MachinePresetName; add a validation step (e.g., an
isValidMachinePreset guard or lookup against the allowed MachinePresetName
values) and only pass a known preset into MachineLabelCombo (or pass undefined/a
safe fallback like "unknown") so MachineLabelCombo never receives null/invalid
presets; update the JSX around MachineLabelCombo to use the validated value
instead of casting runData.machinePreset as MachinePresetName.
| action={v3RunIdempotencyKeyResetPath(organization, project, environment, { | ||
| friendlyId: runParam, | ||
| })} |
There was a problem hiding this comment.
Potentially targeting the wrong run: use run.friendlyId (not runParam) for reset/logs actions.
RunBody can render when run.friendlyId !== runParam (see “Focus on run / Jump to original run”), so these links/forms can operate on a different run than the one being displayed.
Proposed fix
- action={v3RunIdempotencyKeyResetPath(organization, project, environment, {
- friendlyId: runParam,
- })}
+ action={v3RunIdempotencyKeyResetPath(organization, project, environment, {
+ friendlyId: run.friendlyId,
+ })}
...
- <LinkButton
- to={`${v3LogsPath(organization, project, environment)}?runId=${runParam}&from=${new Date(run.createdAt).getTime() - 60000}`}
- variant="secondary/medium"
- >
+ <LinkButton
+ to={`${v3LogsPath(organization, project, environment)}?${new URLSearchParams({
+ runId: run.friendlyId,
+ from: String(Math.max(0, new Date(run.createdAt).getTime() - 60_000)),
+ }).toString()}`}
+ variant="secondary/medium"
+ >
View logs
</LinkButton>
<LinkButton
- to={v3RunDownloadLogsPath({ friendlyId: runParam })}
+ to={v3RunDownloadLogsPath({ friendlyId: run.friendlyId })}
LeadingIcon={CloudArrowDownIcon}
leadingIconClassName="text-indigo-400"
variant="secondary/medium"
target="_blank"
download
>
Download logs
</LinkButton>Also applies to: 949-966
🤖 Prompt for AI Agents
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
around lines 593 - 595, The form/action targets use runParam instead of the
actual displayed run's identifier, which can point actions at the wrong run when
RunBody renders a different run; update all places that build links/forms (e.g.,
calls to v3RunIdempotencyKeyResetPath and any run logs path builders around the
Reset/Logs UI) to use run.friendlyId (and validate run is defined) instead of
runParam so the reset/logs actions operate on the shown run; apply the same
replacement for the other occurrences noted (the block around lines 949-966) and
ensure any UI that falls back to runParam only does so when run.friendlyId is
not available.
PR Review: feat: tri-6738 Create aggregated logs pageThanks for this comprehensive feature addition! The Logs page is a valuable addition to the Trigger.dev dashboard. Here is my detailed review: Strengths
Suggestions for Improvement1. Duplicate Code in Search Highlighting (Medium Priority)The search highlighting logic is duplicated in multiple places:
Suggestion: Extract into a shared utility function in ~/utils/searchHighlight.ts 2. Missing Error Boundary (Medium Priority)The main logs route uses TypedAwait with an error element, but individual fetchers (like the run context fetcher) do not have error handling visible to users. Suggestion: Add error states to the RunTab component when fetcher fails. 3. Potential Memory Issue with Log Accumulation (Medium Priority)In route.tsx, logs accumulate indefinitely in state. For users scrolling through many logs, this could grow unbounded. Suggestion: Consider implementing a sliding window or virtualized list (e.g., using @tanstack/react-virtual) to cap memory usage. 4. useEffect Dependencies in LogDetailView (Low Priority)The fetcher object is not in the dependency array, which is intentional but could be flagged by ESLint. Consider adding a comment explaining why. 5. Type Safety in LogsListPresenter (Low Priority)Consider defining a proper type for error attributes to avoid using any. 6. Missing Loading State for Initial Panel Load (Low Priority)When switching between logs, there is no indication the new log is loading if the previous log data is still displayed. Suggestion: Show a subtle loading indicator when fetching a different log. 7. Newline Missing at EOF (Nit)route.tsx is missing a newline at the end of file (line 335). Security Observations
Performance Notes
Test CoverageThe PR description mentions manual testing was performed. Consider adding:
SummaryThis is a solid implementation of a complex feature. The main areas for improvement are:
Overall, this looks good to merge after addressing the medium-priority items. Nice work! |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/components/code/CodeBlock.tsx (1)
398-409: MissingsearchTermprop in modal'sHighlightCodecomponent.The modal's
HighlightCodecall doesn't passsearchTerm, but the non-highlighted path at line 416 does usehighlightSearchInText. This creates inconsistent behavior: search terms won't be highlighted in the modal when syntax highlighting is enabled (≤1000 lines), but will be highlighted when syntax highlighting is disabled.🔧 Proposed fix
<HighlightCode theme={theme} code={code} language={language} showLineNumbers={showLineNumbers} highlightLines={highlightLines} maxLineWidth={maxLineWidth} className="min-h-full" preClassName="text-sm" isWrapped={isWrapped} + searchTerm={searchTerm} />
🤖 Fix all issues with AI agents
In @apps/webapp/app/components/logs/LogDetailView.tsx:
- Around line 383-384: The code unsafely casts runData.status to TaskRunStatus
for TaskRunStatusCombo and descriptionForTaskRunStatus even though
VALID_TASK_RUN_STATUSES (used by RunContextSchema) permits values not in
TaskRunStatus; either make VALID_TASK_RUN_STATUSES match the TaskRunStatus union
or remove the direct cast and introduce a runtime-safe mapper: create a function
(e.g., normalizeTaskRunStatus) that takes runData.status, checks/guards against
the TaskRunStatus literals (or maps legacy values like
"QUEUED"/"WAITING"/"FAILURE" to corresponding TaskRunStatus entries), returns a
valid TaskRunStatus or a safe fallback (e.g., "UNKNOWN" or null) and use that
result when calling TaskRunStatusCombo and descriptionForTaskRunStatus instead
of casting.
In @apps/webapp/app/components/logs/LogsTable.tsx:
- Around line 156-157: The table's blank-row colSpan is inconsistent: update
every usage of TableBlankRow (e.g., the currently colSpan={6} and the other
occurrences with colSpan={7}) in LogsTable.tsx (and any BlankState render that
uses colSpan={7}) to use the same value that matches the header (5 data columns
+ menu = colSpan={6}); ensure all TableBlankRow components and any
NoLogs/BlankState placeholders use colSpan={6} so the empty-state spans the
correct number of columns.
In @apps/webapp/app/presenters/v3/LogsListPresenter.server.ts:
- Around line 238-241: RunsRepositoryOptions currently requires prisma:
PrismaClient but the code casts this.replica to PrismaClient; update the
RunsRepositoryOptions type to accept prisma: PrismaClientOrTransaction so you
can pass this.replica directly without casting. Modify the RunsRepositoryOptions
interface/type used by RunsRepository and ensure any constructor or call sites
(e.g., the RunsRepository instantiation in LogsListPresenter.server where new
RunsRepository({ clickhouse: this.clickhouse, prisma: this.replica })) now
type-check with PrismaClientOrTransaction, keeping existing method usage
($queryRaw, taskRun.findMany) intact.
In
@apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx:
- Around line 73-80: The route currently throws generic Error objects when
findProjectBySlug or findEnvironmentBySlug return null; replace those throws
with proper Response objects so HTTP status codes are returned (e.g., throw new
Response("Project not found", { status: 404 }) and throw new
Response("Environment not found", { status: 404 })). Update the checks around
findProjectBySlug and findEnvironmentBySlug in the route handler to throw
Response instances with appropriate status and message (or JSON body) instead of
new Error to ensure consistent HTTP error handling.
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx:
- Around line 31-33: The schema currently uses status:
z.enum(VALID_TASK_RUN_STATUSES).catch((ctx) => { throw new Error(`Invalid
TaskRunStatus: ${ctx.input}`); }), which misuses .catch() (it should provide a
fallback) and causes parse to always fail; either remove the .catch() so
z.enum(VALID_TASK_RUN_STATUSES) naturally rejects invalid values, or replace the
.catch() with a proper refinement such as using .refine(value =>
VALID_TASK_RUN_STATUSES.includes(value), { message: `Invalid TaskRunStatus` })
to enforce validation with a custom error message; update the symbol "status"
and reference VALID_TASK_RUN_STATUSES accordingly.
🧹 Nitpick comments (16)
apps/webapp/app/components/code/CodeBlock.tsx (1)
429-463: Consider extracting shared highlighting logic to reduce duplication.This function duplicates the regex-based highlighting logic found in
HighlightCode(lines 612-642). Extracting the core matching logic into a shared helper would improve maintainability.♻️ Suggested refactor
/** * Splits text by search term matches, returning an array of parts * where matches are wrapped in highlighted spans. */ function highlightMatches( text: string, searchTerm: string | undefined, keyPrefix: string = "match" ): React.ReactNode { if (!searchTerm || searchTerm.trim() === "") { return text; } const escapedSearch = searchTerm.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); const regex = new RegExp(escapedSearch, "gi"); const parts: React.ReactNode[] = []; let lastIndex = 0; let match; let matchCount = 0; while ((match = regex.exec(text)) !== null) { if (match.index > lastIndex) { parts.push(text.substring(lastIndex, match.index)); } parts.push( <span key={`${keyPrefix}-${matchCount}`} style={SEARCH_HIGHLIGHT_STYLES}> {match[0]} </span> ); lastIndex = regex.lastIndex; matchCount++; } if (lastIndex < text.length) { parts.push(text.substring(lastIndex)); } return parts.length > 0 ? parts : text; }Then use it in both
highlightSearchInTextand within the token loop inHighlightCode.apps/webapp/app/components/logs/LogDetailView.tsx (4)
153-159: Missing dependencies in useEffect for fetcher.The
fetcherobject is omitted from the dependency array. Whilefetcher.loadis stable, the pattern can cause stale closure issues or ESLint warnings. Either addfetcherto dependencies or extract the load function.♻️ Suggested fix
useEffect(() => { if (!logId) return; fetcher.load( `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug}/logs/${encodeURIComponent(logId)}` ); - }, [organization.slug, project.slug, environment.slug, logId]); + }, [organization.slug, project.slug, environment.slug, logId, fetcher]);
340-348: Missing dependencies in useEffect for fetcher in RunTab.Similar to the parent component,
fetcheris missing from the dependency array. This can lead to stale closures or linting warnings.♻️ Suggested fix
useEffect(() => { if (!log.runId) return; setRequested(true); fetcher.load( `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug}/logs/${encodeURIComponent(log.id)}/run?runId=${encodeURIComponent(log.runId)}` ); - }, [organization.slug, project.slug, environment.slug, log.id, log.runId]); + }, [organization.slug, project.slug, environment.slug, log.id, log.runId, fetcher]);
274-296: Type assertion withas anybypasses type safety.Line 292 uses
(logWithExtras.attributes as any)?.error?.messagewhich bypasses TypeScript's type checking. Consider using a type guard or narrowing to safely access nested properties.♻️ Suggested fix
// Determine message to show let message = log.message ?? ""; if (log.level === "ERROR") { - const maybeErrorMessage = (logWithExtras.attributes as any)?.error?.message; + const attrs = logWithExtras.attributes; + const maybeErrorMessage = + attrs && + typeof attrs === "object" && + "error" in attrs && + attrs.error && + typeof attrs.error === "object" && + "message" in attrs.error && + typeof attrs.error.message === "string" + ? attrs.error.message + : undefined; if (typeof maybeErrorMessage === "string" && maybeErrorMessage.length > 0) { message = maybeErrorMessage; } }
97-142: Potential infinite loop with zero-length regex match.If
escapedSearchproduces a regex that can match zero-length strings (edge case),regex.exec(json)could loop infinitely sincelastIndexwouldn't advance. Consider adding a guard.♻️ Suggested guard
while ((match = regex.exec(json)) !== null) { + // Prevent infinite loop on zero-length matches + if (match[0].length === 0) { + regex.lastIndex++; + continue; + } // Add text before match if (match.index > lastIndex) {apps/webapp/app/components/logs/LogsTable.tsx (2)
29-41: Unusedfiltersandvariantprops in LogsTableProps.The
filtersandvariantprops are defined inLogsTablePropsbut never used in the component. Remove them or implement their usage to avoid confusion.♻️ Remove unused props
type LogsTableProps = { logs: LogEntry[]; hasFilters: boolean; - filters: LogsListAppliedFilters; searchTerm?: string; isLoading?: boolean; isLoadingMore?: boolean; hasMore?: boolean; onLoadMore?: () => void; - variant?: TableVariant; selectedLogId?: string; onLogSelect?: (logId: string) => void; };And update the destructuring:
export function LogsTable({ logs, hasFilters, - filters, searchTerm, isLoading = false, isLoadingMore = false, hasMore = false, onLoadMore, - variant, selectedLogId, onLogSelect, }: LogsTableProps) {Also applies to: 86-96
61-84:highlightTextonly highlights the first match.Unlike
highlightJsonWithSearchin LogDetailView.tsx which highlights all occurrences, this function only highlights the first match. Consider making the behavior consistent across components.♻️ Highlight all occurrences
function highlightText(text: string, searchTerm: string | undefined): ReactNode { if (!searchTerm || searchTerm.trim() === "") { return text; } - const lowerText = text.toLowerCase(); - const lowerSearch = searchTerm.toLowerCase(); - const index = lowerText.indexOf(lowerSearch); - - if (index === -1) { - return text; - } - - return ( - <> - {text.slice(0, index)} - <mark className="rounded px-0.5 bg-yellow-400 text-black font-medium"> - {text.slice(index, index + searchTerm.length)} - </mark> - {text.slice(index + searchTerm.length)} - </> - ); + const escapedSearch = searchTerm.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const regex = new RegExp(`(${escapedSearch})`, "gi"); + const parts = text.split(regex); + + if (parts.length === 1) { + return text; + } + + return parts.map((part, i) => + regex.test(part) ? ( + <mark key={i} className="rounded px-0.5 bg-yellow-400 text-black font-medium"> + {part} + </mark> + ) : ( + part + ) + ); }apps/webapp/app/presenters/v3/LogDetailPresenter.server.ts (2)
18-22: Unusedreplicaconstructor parameter.The
replica(PrismaClientOrTransaction) parameter is accepted in the constructor but never used in thecallmethod. Either remove it or add a comment explaining future usage.♻️ Remove unused parameter
export class LogDetailPresenter { constructor( - private readonly replica: PrismaClientOrTransaction, private readonly clickhouse: ClickHouse ) {}And update instantiation sites accordingly.
63-84: Silent failure on JSON parse errors may hide issues.Swallowing parse errors with empty catch blocks could hide malformed data issues. Consider logging a warning for debugging purposes.
♻️ Add debug logging
try { if (log.metadata) { parsedMetadata = JSON.parse(log.metadata) as Record<string, unknown>; } } catch { - // Ignore parse errors + // Metadata parse failed - continue with empty object + // Could add: this.logger?.warn("Failed to parse log metadata", { logId: options.spanId }); }apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (4)
61-68: Admin-only redirect may expose route existence.Using
redirect("/")for non-admin users reveals that the route exists. Consider using a 404 or 403 response instead for security through obscurity.♻️ Use 404 for non-admin access
if (!isAdmin) { - throw redirect("/"); + throw new Response("Not Found", { status: 404 }); }
219-225: Stale closure risk in fetcher effect.The
existingIdsmemo depends onaccumulatedLogs, but the effect that appends logs uses bothexistingIdsand updatesaccumulatedLogs. This could lead to stale closure issues whereexistingIdsdoesn't reflect the latest accumulated logs during rapid fetches.♻️ Use functional state update
// Append new logs when fetcher completes (with deduplication) useEffect(() => { if (fetcher.data && fetcher.state === "idle") { - const newLogs = fetcher.data.logs.filter((log) => !existingIds.has(log.id)); - setAccumulatedLogs((prev) => [...prev, ...newLogs]); + setAccumulatedLogs((prev) => { + const existingSet = new Set(prev.map((log) => log.id)); + const newLogs = fetcher.data.logs.filter((log) => !existingSet.has(log.id)); + return [...prev, ...newLogs]; + }); setNextCursor(fetcher.data.pagination.next); } - }, [fetcher.data, fetcher.state, existingIds]); + }, [fetcher.data, fetcher.state]);
42-42: Unused import:getUserById.The
getUserByIdfunction is imported but never used in this file.♻️ Remove unused import
-import { getUserById } from "~/models/user.server";
196-207: Full page reload for debug toggle is disruptive.Using
window.location.href = url.toString()causes a full page reload, losing scroll position and accumulated logs. Consider using Remix navigation or the fetcher pattern for a smoother UX.♻️ Use Remix navigation
+import { useNavigate } from "@remix-run/react"; + function LogsList({ ... }) { + const navigate = useNavigate(); // ... const handleDebugToggle = useCallback( (checked: boolean) => { const url = new URL(window.location.href); if (checked) { url.searchParams.set("showDebug", "true"); } else { url.searchParams.delete("showDebug"); } - window.location.href = url.toString(); + navigate(url.pathname + url.search, { replace: true }); }, - [] + [navigate] );apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (3)
101-104: Duplicate utility functionconvertDateToNanoseconds.This function duplicates the one in
apps/webapp/app/v3/eventRepository/common.server.ts(lines 52-54). Consider importing from the existing module to maintain DRY principles.♻️ Import existing utility
+import { convertDateToNanoseconds } from "~/v3/eventRepository/common.server"; -function convertDateToNanoseconds(date: Date): bigint { - return BigInt(date.getTime()) * 1_000_000n; -}
461-471: Inconsistent error handling in attributes parsing.The try-catch block for parsing attributes has an unused variable declaration on line 463 (
let attributes). Also, theattributesvariable shadows thelog.attributesproperty which may cause confusion.♻️ Clean up variable declaration
if (log.status === "ERROR" && log.attributes) { try { - let attributes = log.attributes as Record<string, any>; + const attrs = log.attributes as Record<string, any>; - if (attributes?.error?.message && typeof attributes.error.message === 'string') { - displayMessage = attributes.error.message; - } - } catch { + if (attrs?.error?.message && typeof attrs.error.message === "string") { + displayMessage = attrs.error.message; + } + } catch { // If attributes parsing fails, use the regular message } }
243-246:clampToNowfunction could be extracted.The nested
clampToNowfunction is defined insidecall(). Since it's a pure utility, consider extracting it to module scope for reusability and testability.♻️ Extract to module scope
+function clampToNow(date: Date): Date { + const now = new Date(); + return date > now ? now : date; +} + export class LogsListPresenter { // ... public async call(...) { // ... - function clampToNow(date: Date): Date { - const now = new Date(); - return date > now ? now : date; - } - runIds = await runsRepository.listFriendlyRunIds({ // ... to: effectiveTo ? clampToNow(effectiveTo).getTime() : undefined,
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsxapps/webapp/app/utils/logUtils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/components/logs/LogDetailView.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/components/logs/LogDetailView.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/components/logs/LogDetailView.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/components/logs/LogDetailView.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/components/logs/LogDetailView.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/code/CodeBlock.tsxapps/webapp/app/components/logs/LogDetailView.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/utils/logUtils.tsapps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
🧠 Learnings (14)
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure log level in trigger.config.ts using `logLevel` property
Applied to files:
apps/webapp/app/utils/logUtils.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure OpenTelemetry instrumentations and exporters in trigger.config.ts for enhanced logging
Applied to files:
apps/webapp/app/utils/logUtils.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use logger methods (debug, log, info, warn, error) from `trigger.dev/sdk/v3` for structured logging in tasks
Applied to files:
apps/webapp/app/utils/logUtils.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.{ts,tsx} : Organize presenters in the webapp following the pattern `app/v3/presenters/*/*.server.ts` to move complex loader code into classes
Applied to files:
apps/webapp/app/presenters/v3/LogDetailPresenter.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: The webapp at apps/webapp is a Remix 2.1 application using Node.js v20
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Use the Run Engine 2.0 from `internal/run-engine` for new run lifecycle code in the webapp instead of the legacy run engine
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx
📚 Learning: 2025-07-12T18:06:04.133Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogDetailView.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2024-10-18T15:41:52.352Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 1418
File: packages/core/src/v3/errors.ts:364-371
Timestamp: 2024-10-18T15:41:52.352Z
Learning: In `packages/core/src/v3/errors.ts`, within the `taskRunErrorEnhancer` function, `error.message` is always defined, so it's safe to directly call `error.message.includes("SIGTERM")` without additional checks.
Applied to files:
apps/webapp/app/components/logs/LogDetailView.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Use `useRun`, `useRealtimeRun` and other SWR/realtime hooks from `trigger.dev/react-hooks` for data fetching
Applied to files:
apps/webapp/app/components/logs/LogDetailView.tsx
📚 Learning: 2025-01-13T18:31:48.160Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 1608
File: apps/webapp/app/v3/services/triggerTask.server.ts:418-418
Timestamp: 2025-01-13T18:31:48.160Z
Learning: The `MachinePresetName` schema is used to validate machine preset values in the trigger.dev codebase, ensuring type safety and validation of machine preset options.
Applied to files:
apps/webapp/app/components/logs/LogDetailView.tsx
🧬 Code graph analysis (2)
apps/webapp/app/utils/logUtils.ts (1)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (1)
LogLevel(22-22)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (6)
apps/webapp/app/utils/logUtils.ts (2)
LogLevel(1-1)kindToLevel(4-30)apps/webapp/app/v3/eventRepository/common.server.ts (1)
convertDateToNanoseconds(53-55)apps/webapp/app/components/runs/v3/SharedFilters.tsx (1)
timeFilters(106-163)apps/webapp/app/models/task.server.ts (1)
getAllTaskIdentifiers(10-21)apps/webapp/app/models/runtimeEnvironment.server.ts (2)
displayableEnvironment(257-277)findDisplayableEnvironment(279-310)apps/webapp/app/v3/eventRepository/clickhouseEventRepository.server.ts (1)
convertDateToClickhouseDateTime(1950-1953)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
apps/webapp/app/components/code/CodeBlock.tsx (4)
67-79: LGTM!The new
searchTermprop andSEARCH_HIGHLIGHT_STYLESconstant are well-defined. Theas constassertion is appropriate for ensuring immutability of the style object.
251-251: LGTM!Good defensive handling for potentially undefined
codevalues.
608-656: LGTM!The token-level search highlighting logic correctly handles regex escaping and match highlighting. The implementation properly preserves the original token styling while overlaying the search highlight.
342-377: LGTM!The
searchTermprop is correctly propagated to both theHighlightCodecomponent and the fallback plain-text rendering path, ensuring consistent highlighting behavior regardless of which code path is taken.apps/webapp/app/components/logs/LogDetailView.tsx (1)
145-272: Well-structured component with good UX patterns.The LogDetailView component implements several good practices:
- Escape key handling for panel dismissal
- Loading states with spinner
- Graceful "not found" handling
- Clean tab navigation with keyboard shortcuts
apps/webapp/app/components/logs/LogsTable.tsx (1)
117-140: IntersectionObserver implementation is correct.The infinite scroll implementation properly:
- Checks prerequisites before creating the observer
- Stores
currentRefbefore cleanup to avoid stale reference issues- Cleans up the observer in the effect's return function
apps/webapp/app/utils/logUtils.ts (1)
1-50: Clean utility implementation following coding guidelines.The file correctly uses a string union type for
LogLevelinstead of an enum (per coding guidelines). Both functions are pure, well-documented, and provide exhaustive handling with sensible defaults.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx (2)
73-102: Good validation and error handling in loader.The loader properly:
- Authenticates the user
- Validates required params with Zod schema
- Returns appropriate HTTP status codes for missing resources
- Uses
$replicafor read operations
12-24: Replace hardcodedVALID_TASK_RUN_STATUSESwithObject.values(TaskRunStatus)from the properly maintained enum.The hardcoded array contains stale and incorrect values that no longer match the actual
TaskRunStatusenum inapps/webapp/app/database-types.ts. It includes invalid values (QUEUED,WAITING_FOR_EXECUTION,WAITING,FAILURE) and is missing current valid statuses (CRASHED,DELAYED,EXPIRED,TIMED_OUT, and others). This causes the Zod schema to reject valid run statuses and accept invalid ones, risking validation failures at runtime.Use the pattern already established in the codebase:
z.enum(Object.values(TaskRunStatus) as [TaskRunStatus, ...TaskRunStatus[]])(seeapps/webapp/app/services/runsRepository/runsRepository.server.ts:25). ImportTaskRunStatusfrom~/database-types.tsinstead of maintaining a separate hardcoded list.⛔ Skipped due to learnings
Learnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/writing-tasks.mdc:0-0 Timestamp: 2025-11-27T16:27:35.304Z Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validationLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-27T16:26:37.432Z Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : Use zod for validation in packages/core and apps/webappLearnt from: matt-aitken Repo: triggerdotdev/trigger.dev PR: 2264 File: apps/webapp/app/services/runsRepository.server.ts:172-174 Timestamp: 2025-07-12T18:06:04.133Z Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.apps/webapp/app/presenters/v3/LogDetailPresenter.server.ts (1)
86-105: Well-structured return object with consistent ID format.The return object correctly:
- Uses
::separator matching LogsListPresenter format- Converts dates to ISO strings
- Handles optional fields with proper nullish coalescing
- Derives display level using shared
kindToLevelutilityapps/webapp/app/presenters/v3/LogsListPresenter.server.ts (2)
365-402: Complex level filtering logic is well-implemented.The
levelToKindsAndStatusesmapping and the dynamic query building for level filters correctly handles:
- Multiple levels with OR conditions
- Special TRACE handling to exclude error/cancelled when not explicitly selected
- Proper parameterization for ClickHouse queries
352-362: No SQL injection vulnerability present.The code uses the official
@clickhouse/clientlibrary's parameterized query system (query_paramswith{paramName: Type}syntax), which performs server-side binding. The parameter value—including the wildcard characters—is sent as a literal string to ClickHouse and is safely interpreted. Including%and_wildcards directly in bound parameters is the recommended and safe approach for pattern matching in ClickHouse ILIKE queries.Likely an incorrect or invalid review comment.
| button={<TaskRunStatusCombo status={runData.status as TaskRunStatus} />} | ||
| content={descriptionForTaskRunStatus(runData.status as TaskRunStatus)} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the LogDetailView.tsx file
find . -name "LogDetailView.tsx" -type f 2>/dev/nullRepository: triggerdotdev/trigger.dev
Length of output: 120
🏁 Script executed:
# Also search for RunContextSchema definition
rg "RunContextSchema" --type ts -A 5 | head -100Repository: triggerdotdev/trigger.dev
Length of output: 2732
🏁 Script executed:
# Search for TaskRunStatus enum definition
rg "enum TaskRunStatus|type TaskRunStatus" --type ts -A 10 | head -150Repository: triggerdotdev/trigger.dev
Length of output: 5855
🏁 Script executed:
# Find VALID_TASK_RUN_STATUSES definition
rg "VALID_TASK_RUN_STATUSES" --type ts -B 2 -A 20 | head -100Repository: triggerdotdev/trigger.dev
Length of output: 5776
🏁 Script executed:
# Get more context on TaskRunStatus from database package
rg "export.*TaskRunStatus|const.*TaskRunStatus" --type ts packages/core/src -B 2 -A 15 | head -200Repository: triggerdotdev/trigger.dev
Length of output: 2709
🏁 Script executed:
# Read LogDetailView.tsx around lines 383-384
sed -n '375,390p' apps/webapp/app/components/logs/LogDetailView.tsxRepository: triggerdotdev/trigger.dev
Length of output: 648
Type mismatch between validation schema and target type creates unsafe assertion.
The cast runData.status as TaskRunStatus is unsafe because VALID_TASK_RUN_STATUSES (used by RunContextSchema validation) includes values like "QUEUED", "WAITING_FOR_EXECUTION", "WAITING", and "FAILURE" that don't exist in the TaskRunStatus type from @trigger.dev/database. At runtime, the fetcher could provide a status value that passes schema validation but doesn't match TaskRunStatus literals, causing a hidden type safety violation.
Align VALID_TASK_RUN_STATUSES with TaskRunStatus values, or handle the type incompatibility without a direct cast.
🤖 Prompt for AI Agents
In @apps/webapp/app/components/logs/LogDetailView.tsx around lines 383 - 384,
The code unsafely casts runData.status to TaskRunStatus for TaskRunStatusCombo
and descriptionForTaskRunStatus even though VALID_TASK_RUN_STATUSES (used by
RunContextSchema) permits values not in TaskRunStatus; either make
VALID_TASK_RUN_STATUSES match the TaskRunStatus union or remove the direct cast
and introduce a runtime-safe mapper: create a function (e.g.,
normalizeTaskRunStatus) that takes runData.status, checks/guards against the
TaskRunStatus literals (or maps legacy values like "QUEUED"/"WAITING"/"FAILURE"
to corresponding TaskRunStatus entries), returns a valid TaskRunStatus or a safe
fallback (e.g., "UNKNOWN" or null) and use that result when calling
TaskRunStatusCombo and descriptionForTaskRunStatus instead of casting.
| <TableBlankRow colSpan={6}> | ||
| {!isLoading && <NoLogs title="No logs found" />} |
There was a problem hiding this comment.
Mismatched colSpan values across table states.
TableBlankRow uses colSpan={6} on line 156 but colSpan={7} on lines 252 and 255. The table header defines 5 columns (Time, Run, Task, Level, Message) plus the menu cell. Ensure consistent colSpan values.
🔧 Fix colSpan consistency
{logs.length === 0 && !hasFilters ? (
- <TableBlankRow colSpan={6}>
+ <TableBlankRow colSpan={6}>
{!isLoading && <NoLogs title="No logs found" />}
</TableBlankRow>And in BlankState:
-function BlankState({ isLoading }: { isLoading?: boolean }) {
- if (isLoading) return <TableBlankRow colSpan={7}></TableBlankRow>;
+function BlankState({ isLoading }: { isLoading?: boolean }) {
+ if (isLoading) return <TableBlankRow colSpan={6}></TableBlankRow>;
return (
- <TableBlankRow colSpan={7}>
+ <TableBlankRow colSpan={6}>🤖 Prompt for AI Agents
In @apps/webapp/app/components/logs/LogsTable.tsx around lines 156 - 157, The
table's blank-row colSpan is inconsistent: update every usage of TableBlankRow
(e.g., the currently colSpan={6} and the other occurrences with colSpan={7}) in
LogsTable.tsx (and any BlankState render that uses colSpan={7}) to use the same
value that matches the header (5 data columns + menu = colSpan={6}); ensure all
TableBlankRow components and any NoLogs/BlankState placeholders use colSpan={6}
so the empty-state spans the correct number of columns.
| const runsRepository = new RunsRepository({ | ||
| clickhouse: this.clickhouse, | ||
| prisma: this.replica as PrismaClient, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find RunsRepository class definition
fd "runsRepository" --type f | head -10Repository: triggerdotdev/trigger.dev
Length of output: 194
🏁 Script executed:
# Search for RunsRepository class definition with context
rg "class RunsRepository" --type ts -A 15Repository: triggerdotdev/trigger.dev
Length of output: 1806
🏁 Script executed:
# Check the LogsListPresenter file around the mentioned lines
fd "LogsListPresenter" --type fRepository: triggerdotdev/trigger.dev
Length of output: 126
🏁 Script executed:
# Find RunsRepositoryOptions definition
rg "interface RunsRepositoryOptions|type RunsRepositoryOptions" --type ts -A 10Repository: triggerdotdev/trigger.dev
Length of output: 3377
🏁 Script executed:
# Get the LogsListPresenter context around lines 238-241
rg "class LogsListPresenter" --type ts -B 5 -A 50 | head -80Repository: triggerdotdev/trigger.dev
Length of output: 4422
🏁 Script executed:
# Search for where this.replica is defined in LogsListPresenter
rg "this\.replica" --type ts -B 3 -A 3 apps/webapp/app/presenters/Repository: triggerdotdev/trigger.dev
Length of output: 8859
🏁 Script executed:
# Find PrismaClientOrTransaction type definition
rg "type PrismaClientOrTransaction|interface PrismaClientOrTransaction" --type ts -A 3Repository: triggerdotdev/trigger.dev
Length of output: 7033
🏁 Script executed:
# Check what Prisma methods RunsRepository actually uses
rg "this\.options\.prisma\." --type ts apps/webapp/app/services/runsRepository/ | head -20Repository: triggerdotdev/trigger.dev
Length of output: 1094
🏁 Script executed:
# Check if PrismaTransactionClient supports $queryRaw and model delegates
rg "PrismaTransactionClient" --type ts -B 2 -A 10 internal-packages/database/Repository: triggerdotdev/trigger.dev
Length of output: 2881
🏁 Script executed:
# Look at the actual RunsRepository instantiation context in LogsListPresenter
sed -n '235,245p' apps/webapp/app/presenters/v3/LogsListPresenter.server.tsRepository: triggerdotdev/trigger.dev
Length of output: 487
Update RunsRepositoryOptions to accept PrismaClientOrTransaction instead of casting.
The RunsRepositoryOptions type definition requires prisma: PrismaClient, but this.replica is typed as PrismaClientOrTransaction. Since RunsRepository only uses methods available on both types (e.g., $queryRaw, taskRun.findMany()), the cast is unnecessary. Update the type to prisma: PrismaClientOrTransaction to eliminate the cast.
🤖 Prompt for AI Agents
In @apps/webapp/app/presenters/v3/LogsListPresenter.server.ts around lines 238 -
241, RunsRepositoryOptions currently requires prisma: PrismaClient but the code
casts this.replica to PrismaClient; update the RunsRepositoryOptions type to
accept prisma: PrismaClientOrTransaction so you can pass this.replica directly
without casting. Modify the RunsRepositoryOptions interface/type used by
RunsRepository and ensure any constructor or call sites (e.g., the
RunsRepository instantiation in LogsListPresenter.server where new
RunsRepository({ clickhouse: this.clickhouse, prisma: this.replica })) now
type-check with PrismaClientOrTransaction, keeping existing method usage
($queryRaw, taskRun.findMany) intact.
| if (!project) { | ||
| throw new Error("Project not found"); | ||
| } | ||
|
|
||
| const environment = await findEnvironmentBySlug(project.id, envParam, userId); | ||
| if (!environment) { | ||
| throw new Error("Environment not found"); | ||
| } |
There was a problem hiding this comment.
Missing error response types for findProjectBySlug and findEnvironmentBySlug.
Throwing generic Error instead of Response objects means these errors won't result in proper HTTP status codes. Use Response for consistent error handling.
🔧 Use Response objects for HTTP errors
const project = await findProjectBySlug(organizationSlug, projectParam, userId);
if (!project) {
- throw new Error("Project not found");
+ throw new Response("Project not found", { status: 404 });
}
const environment = await findEnvironmentBySlug(project.id, envParam, userId);
if (!environment) {
- throw new Error("Environment not found");
+ throw new Response("Environment not found", { status: 404 });
}🤖 Prompt for AI Agents
In
@apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
around lines 73 - 80, The route currently throws generic Error objects when
findProjectBySlug or findEnvironmentBySlug return null; replace those throws
with proper Response objects so HTTP status codes are returned (e.g., throw new
Response("Project not found", { status: 404 }) and throw new
Response("Environment not found", { status: 404 })). Update the checks around
findProjectBySlug and findEnvironmentBySlug in the route handler to throw
Response instances with appropriate status and message (or JSON body) instead of
new Error to ensure consistent HTTP error handling.
| status: z.enum(VALID_TASK_RUN_STATUSES).catch((ctx) => { | ||
| throw new Error(`Invalid TaskRunStatus: ${ctx.input}`); | ||
| }), |
There was a problem hiding this comment.
Schema .catch() throws instead of providing fallback.
Using .catch() with a function that throws defeats the purpose of .catch(), which is to provide a fallback value. This will cause the entire schema parse to fail. If that's the intent, use .refine() or simply omit .catch().
🔧 Use refine or remove catch
If you want validation to fail on invalid status:
- status: z.enum(VALID_TASK_RUN_STATUSES).catch((ctx) => {
- throw new Error(`Invalid TaskRunStatus: ${ctx.input}`);
- }),
+ status: z.enum(VALID_TASK_RUN_STATUSES),Or if you want a fallback (not recommended for status):
- status: z.enum(VALID_TASK_RUN_STATUSES).catch((ctx) => {
- throw new Error(`Invalid TaskRunStatus: ${ctx.input}`);
- }),
+ status: z.enum(VALID_TASK_RUN_STATUSES).catch("PENDING"),📝 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.
| status: z.enum(VALID_TASK_RUN_STATUSES).catch((ctx) => { | |
| throw new Error(`Invalid TaskRunStatus: ${ctx.input}`); | |
| }), | |
| status: z.enum(VALID_TASK_RUN_STATUSES), |
🤖 Prompt for AI Agents
In
@apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx
around lines 31 - 33, The schema currently uses status:
z.enum(VALID_TASK_RUN_STATUSES).catch((ctx) => { throw new Error(`Invalid
TaskRunStatus: ${ctx.input}`); }), which misuses .catch() (it should provide a
fallback) and causes parse to always fail; either remove the .catch() so
z.enum(VALID_TASK_RUN_STATUSES) naturally rejects invalid values, or replace the
.catch() with a proper refinement such as using .refine(value =>
VALID_TASK_RUN_STATUSES.includes(value), { message: `Invalid TaskRunStatus` })
to enforce validation with a custom error message; update the symbol "status"
and reference VALID_TASK_RUN_STATUSES accordingly.
Closes #
✅ Checklist
Testing
Changelog
Created new Logs page.
The information shown is gathered from the spans from each run.
The feature supports all run filters with two new filters for level and logs text search.
Screenshots
[Screenshots]
💯