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
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds end-to-end logs support across frontend and backend. Frontend: new UI components (LogsIcon, LogsTable, LogDetailView, LogsSearchInput, LogsLevelFilter, LogsRunIdFilter), search-term highlighting utilities and propagation to CodeBlock and PacketDisplay, table variant updates, Popover openInNewTab, SideMenu entry and v3LogsPath, and Tailwind color alias. Backend: new Remix routes/loaders for logs, presenters (LogsListPresenter, LogDetailPresenter), ClickHouse V2 query builders and PREWHERE support, ClickHouse logsQuerySettings wiring, environment config entries, repository methods for friendly run IDs, and related service/utility updates for fetching and formatting logs. 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. 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)
| {(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.
| 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.
...p/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
Show resolved
Hide resolved
...es/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx
Outdated
Show resolved
Hide resolved
extracted common logic to logUtils improved codeBlock highlight adjusted CLICKHOUSE LOGS values
PR Review: feat: tri-6738 Create aggregated logs pageThis is a well-structured PR that adds a new Logs page for viewing aggregated logs across runs. Overall, the implementation is solid and follows the codebase patterns. Here is my detailed feedback: Code Quality and Best PracticesStrengths:
Areas for improvement:
Potential Bugs or Issues
Performance Considerations
Security Concerns
Test CoverageMissing tests for:
The codebase uses vitest and testcontainers per CLAUDE.md. Id recommend adding at least basic tests for the presenter logic. Additional Observations
SummaryThis is a solid PR that implements a useful feature following established patterns. The main areas to address before merging:
The security model is sound with proper parameterized queries and environment scoping. Performance considerations for ClickHouse are well thought out. |
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)
393-404: Missing searchTerm prop in modal HighlightCode.The modal/expanded view doesn't forward the
searchTermprop toHighlightCode, causing inconsistent highlighting behavior between the inline and expanded views.🔧 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/LogsTable.tsx:
- Around line 1-2: The import formatDurationNanoseconds in LogsTable.tsx is
unused; remove it from the import statement (so only import ArrowPathIcon and
ArrowTopRightOnSquareIcon) to clean up the module and avoid linter errors, or if
you intended to use it, add its usage where durations are formatted (e.g.,
inside the LogsTable component) and ensure the symbol name matches exactly.
- Around line 62-72: The LogsTable component currently destructures a filters
prop that is not used; either remove filters from the LogsTable parameter list
and update the LogsTableProps type to delete the filters field, or (if filters
were intended) use the filters value inside LogsTable where filtering logic is
applied (e.g., pass it into the data/rows computation or effect that
fetches/filters logs) and ensure LogsTableProps still declares filters;
reference the LogsTable function and LogsTableProps type to locate and apply the
change.
In @apps/webapp/app/presenters/v3/LogsListPresenter.server.ts:
- Around line 109-111: The current convertDateToNanoseconds(date) -> BigInt
conversion is fine but downstream string slicing assumes a 19-digit nanosecond
string which breaks for dates before ~1970-04-26; add a helper
formatNanosecondsForClickhouse(ns: bigint): string that converts the bigint to a
decimal string and left-pads with zeros to 19 digits (or truncates to 19 if
longer), then replace any direct string-slicing uses with
formatNanosecondsForClickhouse(convertDateToNanoseconds(date)) in the query
builder calls (references: convertDateToNanoseconds,
formatNanosecondsForClickhouse, and the query construction locations in
LogsListPresenter.server.ts).
In
@apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx:
- Line 42: The import getUserById is unused in this route module; remove the
import statement for getUserById (or if it was intended to be used, replace the
unused import by actually calling getUserById where needed). Locate the import
line that references getUserById and delete it (or implement its usage) to
resolve the unused import.
🧹 Nitpick comments (6)
apps/webapp/app/utils/logUtils.ts (1)
23-60: ReDoS concern is mitigated by proper escaping, but consider input length limits.The static analysis warning about ReDoS is a false positive in this case since line 33 escapes all regex special characters before constructing the RegExp. However, for defense-in-depth against very long search inputs causing performance issues, consider adding a maximum length check.
🛡️ Optional: Add input length validation
export function highlightSearchText( text: string, searchTerm?: string, style: React.CSSProperties = DEFAULT_HIGHLIGHT_STYLES ): ReactNode { - if (!searchTerm || searchTerm.trim() === "") { + if (!searchTerm || searchTerm.trim() === "" || searchTerm.length > 500) { return text; }apps/webapp/app/components/logs/LogDetailView.tsx (3)
106-112: Missingfetcherin useEffect dependency array.The
fetcher.loadcall uses thefetcherobject, but it's not listed in the dependency array. While Remix'suseFetcherreturns a stable reference, ESLint rules and React best practices recommend including it.🔧 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]);
227-249: Type assertions for dynamic attributes are acceptable but could be safer.The
asassertions forLogEntry & { attributes }andas anyfor error message extraction work but are fragile. Consider using optional chaining more defensively.♻️ Optional: Safer attribute access
- const logWithExtras = log as LogEntry & { - attributes?: Record<string, unknown>; - }; + const logWithExtras = log as LogEntry & { + attributes?: Record<string, unknown>; + }; // Determine message to show let message = log.message ?? ""; if (log.level === "ERROR") { - const maybeErrorMessage = (logWithExtras.attributes as any)?.error?.message; + const maybeErrorMessage = + typeof logWithExtras.attributes === 'object' && logWithExtras.attributes !== null + ? (logWithExtras.attributes as { error?: { message?: unknown } })?.error?.message + : undefined; if (typeof maybeErrorMessage === "string" && maybeErrorMessage.length > 0) { message = maybeErrorMessage; } }
286-312: Similar missingfetcherdependency in RunTab.Same issue as the main component -
fetcher.loadis called butfetcheris not in the dependency array.🔧 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]);apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (2)
214-223: Potential stale closure or unnecessary re-renders.Including
accumulatedLogsin the dependency array while also callingsetAccumulatedLogsinside the effect could cause unnecessary re-renders. Consider using a ref or functional update to avoid this.♻️ Suggested improvement using ref
+ const accumulatedLogsRef = useRef<LogEntry[]>(list.logs); + + useEffect(() => { + accumulatedLogsRef.current = accumulatedLogs; + }, [accumulatedLogs]); // Append new logs when fetcher completes (with deduplication) useEffect(() => { if (fetcher.data && fetcher.state === "idle") { - const existingIds = new Set(accumulatedLogs.map((log) => log.id)); + const existingIds = new Set(accumulatedLogsRef.current.map((log) => log.id)); const newLogs = fetcher.data.logs.filter((log) => !existingIds.has(log.id)); if (newLogs.length > 0) { setAccumulatedLogs((prev) => [...prev, ...newLogs]); setNextCursor(fetcher.data.pagination.next); } } - }, [fetcher.data, fetcher.state, accumulatedLogs]); + }, [fetcher.data, fetcher.state]);
193-204: Debug toggle causes full page reload.Using
window.location.hrefassignment causes a full page navigation, losing accumulated state. Consider usinguseSearchParamsorfetcher.submitfor a smoother UX.♻️ Alternative using navigate
+import { useNavigate } from "@remix-run/react"; ... + 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] );
📜 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/env.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/services/runsRepository/runsRepository.server.tsapps/webapp/app/utils/logUtils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.$logId.run.tsx
- apps/webapp/app/env.server.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{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 insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/code/CodeBlock.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/services/runsRepository/runsRepository.server.tsapps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/code/CodeBlock.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/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.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/utils/logUtils.tsapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/code/CodeBlock.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/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/utils/logUtils.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/code/CodeBlock.tsx
🧠 Learnings (13)
📚 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/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2026-01-12T11:01:34.777Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:01:34.777Z
Learning: Applies to internal-packages/database/prisma/migrations/**/*.sql : Edit database schema in internal-packages/database/prisma/schema.prisma; remove extraneous lines from migrations related to BackgroundWorker, TaskRun tags, waitpoints, and SecretStore indexes
Applied to files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/presenters/v3/LogsListPresenter.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: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.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 log level in trigger.config.ts using `logLevel` property
Applied to files:
apps/webapp/app/utils/logUtils.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-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
📚 Learning: 2025-09-03T14:34:41.781Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2464
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx:357-371
Timestamp: 2025-09-03T14:34:41.781Z
Learning: When using Zod's safeParse, the .data property is undefined when parsing fails, but TypeScript may still complain about accessing .data without checking .success first. The suggested approach of checking .success before accessing .data improves type safety and code clarity.
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: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
Applied to files:
apps/webapp/app/components/logs/LogDetailView.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/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/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-08-14T10:09:02.528Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2390
File: internal-packages/run-engine/src/engine/index.ts:466-467
Timestamp: 2025-08-14T10:09:02.528Z
Learning: In the triggerdotdev/trigger.dev codebase, it's acceptable to pass `string | undefined` types directly to Prisma operations (both create and update). The codebase consistently uses this pattern and the team is comfortable with how Prisma handles undefined values.
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 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.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
🧬 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/components/code/CodeBlock.tsx (1)
apps/webapp/app/utils/logUtils.ts (1)
highlightSearchText(23-60)
🪛 ast-grep (0.40.4)
apps/webapp/app/utils/logUtils.ts
[warning] 33-33: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(escapedSearch, "gi")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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 (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- 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 (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 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 (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (20)
apps/webapp/app/utils/logUtils.ts (3)
1-12: LGTM on type and constants definition.Using a string union type for
LogLevelaligns with coding guidelines (prefer string unions over enums). Theas constassertion onDEFAULT_HIGHLIGHT_STYLESis appropriate for ensuring immutability.
62-89: LGTM on kindToLevel mapping.The function correctly maps ClickHouse event kinds and statuses to display levels with appropriate precedence (CANCELLED status takes priority, then ERROR from kind or status).
91-109: LGTM on getLevelColor utility.The level-to-color mapping is comprehensive and covers all defined LogLevel values with a sensible default fallback.
apps/webapp/app/components/code/CodeBlock.tsx (3)
68-71: LGTM on new searchTerm prop.The prop addition is well-documented and follows the existing pattern for optional props.
246-246: Good defensive null check.The change from
code.trim()tocode?.trim() ?? ""properly handles potentially undefined code input.
567-581: LGTM on token-level search highlighting.The integration of
highlightSearchTextwithin the token rendering loop correctly applies highlighting while preserving the original token styling.apps/webapp/app/components/logs/LogDetailView.tsx (2)
51-63: LGTM on badge styling helpers.The
getKindColorandgetKindLabelfunctions provide clear, maintainable mappings for UI presentation.
322-504: LGTM on RunTab property display.The property table is comprehensive, handles optional fields well, and provides appropriate formatting and navigation for run metadata.
apps/webapp/app/components/logs/LogsTable.tsx (2)
93-116: LGTM on IntersectionObserver implementation.The infinite scroll implementation properly handles cleanup, guards against double-loading with state checks, and uses appropriate threshold.
150-199: LGTM on table row rendering.The row rendering properly handles selection state, level-based styling, and provides appropriate action handlers. The
TableCellMenuwith hidden "View Run" action is a good UX pattern.apps/webapp/app/services/runsRepository/runsRepository.server.ts (3)
130-131: Good documentation.The JSDoc comment clearly explains the purpose of
listFriendlyRunIdsand its use case for ClickHouse queries.
228-268: LGTM on listFriendlyRunIds implementation.The method follows the established pattern for repository methods with proper tracing, error handling, and ClickHouse-to-Postgres fallback. The implementation is consistent with
listRunIds,listRuns, andcountRuns.
17-23: LGTM on type update.Changing
prismafromPrismaClienttoPrismaClientOrTransactionallows the repository to work within existing transactions, improving flexibility.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (2)
61-68: LGTM on admin-only access control.The loader properly validates admin access and redirects non-admin users. The check includes impersonation support.
276-336: LGTM on resizable panel layout.The two-panel layout with conditional detail panel rendering, Suspense boundaries, and proper state management provides a good UX for the logs viewer.
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (5)
1-21: LGTM on imports.Imports from
@trigger.dev/databaseuse proper package paths rather than root imports. The separation of concerns with ClickHouse and Prisma is well-organized.
314-345: Good use of PREWHERE for partition pruning.Using
prewhereforenvironment_idandinserted_atfilters enables ClickHouse to efficiently prune partitions before applying other filters. This is a valuable performance optimization.
359-369: LGTM on search implementation.The search uses parameterized queries (
{searchPattern: String}) which prevents SQL injection. The case-insensitiveilikesearch across message, attributes, and status fields provides flexible filtering.
372-409: Complex but correct level filtering logic.The level-to-kinds/statuses mapping with conditional exclusion of ERROR/CANCELLED traces (unless explicitly selected) handles the edge cases well. The dynamic parameter building is correct.
462-494: LGTM on log transformation.The ID composition using
::separator, error message extraction from attributes, and proper type conversions produce a well-structuredLogEntryoutput.
...p/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
Outdated
Show resolved
Hide resolved
PR Review: feat: tri-6738 Create aggregated logs pageSummaryThis PR adds a new aggregated logs page that displays spans and logs from task runs, with search and level filtering capabilities. This is a significant feature addition (3,500+ lines) that integrates with ClickHouse for efficient log querying. Strengths
Issues and Concerns1. Potential wildcard injection in ClickHouse LIKE queries (Medium Priority)In LogsListPresenter.server.ts:376-380, the searchTerm is not being escaped for ClickHouse LIKE pattern characters (percent, underscore). While the parameterized query protects against SQL injection, users could craft searches with wildcards that match unintended data. Consider escaping these characters in the search term. 2. Missing test coverage (High Priority)Per CLAUDE.md: We use vitest exclusively - but no tests are included for LogsListPresenter, LogDetailPresenter, logUtils.ts functions, or the new ClickHouse query builders. Please add tests, especially for the presenter logic and the highlightSearchText function. 3. Duplicate color styling functions (Low Priority)getLevelBadgeColor is defined in multiple files: LogsLevelFilter.tsx:36-48, LogsTable.tsx:43-58 (as getLevelBorderColor), and logUtils.ts:92-109 (as getLevelColor). Consider consolidating these into a single utility in logUtils.ts. 4. eslint-disable comments (Low Priority)In LogDetailView.tsx:112 and LogDetailView.tsx:277, the eslint-disable-next-line comments suppress dependency warnings in useEffect. The fetcher and other dependencies should be added to the dependency array, or consider using useCallback to stabilize the load function. 5. Memory concern with accumulated logs (Medium Priority)In the logs route (route.tsx:185-186), logs accumulate infinitely in state with no upper bound. For long-running sessions with heavy usage, this could lead to memory issues. Consider adding a maximum log count limit or using virtualization for the table. 6. Unused filters parameter (Low Priority)In LogsTable.tsx:31, the filters prop is received but not used in the component. 7. Missing changeset (Question)Per CLAUDE.md: When modifying any public package, add a changeset. This PR modifies internal-packages/clickhouse which appears to be internal, but please confirm whether a changeset is needed. 8. Reference project seed/ committed (Minor)The PR adds a new references/seed/ project. Is this intentional for this feature, or should it be in a separate PR? Performance Considerations
Security
Suggestions
SummaryThis is a well-implemented feature that follows the existing codebase patterns. The main concerns are:
Once tests are added and the memory concern is addressed, this looks good to merge. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @apps/webapp/app/components/logs/LogDetailView.tsx:
- Around line 297-311: The loading logic can flash "Run not found" because
requested is set true synchronously before fetcher.state becomes "loading";
update the isLoading check to rely on fetcher state and/or presence of data
instead of the requested flag: replace the current isLoading = !requested ||
fetcher.state === "loading" with a condition such as fetcher.state !== "idle" ||
!fetcher.data (or fetcher.state === "loading" || fetcher.state === "submitting"
|| !fetcher.data) so the UI remains in loading state until the fetcher actually
has results; adjust any UI branches that use isLoading accordingly and remove or
repurpose the requested state (used with fetcher.load in the useEffect) to avoid
the transient false state.
In @apps/webapp/app/components/logs/LogsTable.tsx:
- Around line 166-168: The Run cell (TableCell containing
TruncatedCopyableValue) is missing the click handler and hasAction flag—make it
behave like the other cells by adding the same onClick callback used to select a
log row (e.g., the component's select/log click handler such as onRowClick,
onSelectLog, or handleSelect) to that TableCell and set hasAction={true} so
clicks on the runId select the row; ensure you call the existing selection
function with the current log (or log.id) and preserve any event
handling/stopPropagation logic used elsewhere.
In
@apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx:
- Around line 212-221: The effect that appends fetched logs is suffering from a
stale-closure / unnecessary re-run due to including accumulatedLogs in the
dependency array; change the logic in the useEffect that reads
fetcher.data/fetcher.state so it does not depend on accumulatedLogs: when
fetcher.data && fetcher.state === "idle", call setAccumulatedLogs with a
functional updater (prev => { const existingIds = new Set(prev.map(l => l.id));
const newLogs = fetcher.data.logs.filter(l => !existingIds.has(l.id)); return
newLogs.length ? [...prev, ...newLogs] : prev; }) and then call
setNextCursor(fetcher.data.pagination.next); finally remove accumulatedLogs from
the effect dependency array so the effect only depends on fetcher.data and
fetcher.state (alternatively use a ref for existing IDs if you prefer).
🧹 Nitpick comments (5)
apps/webapp/app/components/logs/LogDetailView.tsx (1)
58-95: Helper functions are well-implemented.The
getKindColorandgetKindLabelfunctions provide clear mappings for log kinds. Consider extracting these tologUtils.tsalongsidegetLevelColorfor consistency and reusability across the codebase.apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (2)
31-58: Consider adding Zod validation for LogsListOptions.Per coding guidelines, validation in
apps/webappshould use Zod. TheLogsListOptionstype defines the input contract but doesn't have runtime validation. Consider adding a Zod schema for validation at the entry points (route loaders).
387-387: Avoid usinganytype.The
paramsvariable usesRecord<string, any>. Consider using a more specific type orRecord<string, string | string[]>to match the expected parameter types.Suggested fix
- const params: Record<string, any> = {}; + const params: Record<string, string | string[]> = {};apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (1)
191-202: Full page reload for debug toggle may impact UX.Using
window.location.href = url.toString()causes a full page reload, losing accumulated logs state. Consider using Remix'suseNavigateoruseSearchParamsfor a smoother experience that preserves client state.Suggested improvement
+ import { useSearchParams } from "@remix-run/react"; // In LogsList component: + const [searchParams, setSearchParams] = useSearchParams(); 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(); + setSearchParams((prev) => { + if (checked) { + prev.set("showDebug", "true"); + } else { + prev.delete("showDebug"); + } + return prev; + }); }, - [] + [setSearchParams] );apps/webapp/app/components/logs/LogsTable.tsx (1)
28-40: Unusedfiltersprop in LogsTableProps.The
filtersprop is defined inLogsTableProps(Line 31) but is not used anywhere in the component. If it's not needed, consider removing it to keep the interface clean.Suggested fix
type LogsTableProps = { logs: LogEntry[]; hasFilters: boolean; - filters: LogsListAppliedFilters; searchTerm?: string; // ...rest }; export function LogsTable({ logs, hasFilters, - filters, searchTerm, // ...rest }: LogsTableProps) {
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/app/components/navigation/SideMenu.tsx
- apps/webapp/app/components/logs/LogsSearchInput.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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 insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.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/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.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/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.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/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.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/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.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/presenters/v3/LogsListPresenter.server.ts
🧠 Learnings (11)
📚 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-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/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 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
📚 Learning: 2025-09-03T14:34:41.781Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2464
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx:357-371
Timestamp: 2025-09-03T14:34:41.781Z
Learning: When using Zod's safeParse, the .data property is undefined when parsing fails, but TypeScript may still complain about accessing .data without checking .success first. The suggested approach of checking .success before accessing .data improves type safety and code clarity.
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: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
Applied to files:
apps/webapp/app/components/logs/LogDetailView.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/**/*.{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.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.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: 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/LogsListPresenter.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: 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: 2025-08-14T10:09:02.528Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2390
File: internal-packages/run-engine/src/engine/index.ts:466-467
Timestamp: 2025-08-14T10:09:02.528Z
Learning: In the triggerdotdev/trigger.dev codebase, it's acceptable to pass `string | undefined` types directly to Prisma operations (both create and update). The codebase consistently uses this pattern and the team is comfortable with how Prisma handles undefined values.
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/components/logs/LogsTable.tsx (7)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (2)
LogEntry(64-64)LogsListAppliedFilters(65-65)apps/webapp/app/utils/pathBuilder.ts (1)
v3RunSpanPath(322-333)apps/webapp/app/utils/cn.ts (1)
cn(77-79)apps/webapp/app/components/primitives/DateTime.tsx (1)
DateTime(54-99)apps/webapp/app/components/primitives/TruncatedCopyableValue.tsx (1)
TruncatedCopyableValue(5-26)apps/webapp/app/utils/logUtils.ts (2)
getLevelColor(92-109)highlightSearchText(23-60)apps/webapp/app/components/primitives/Spinner.tsx (1)
Spinner(8-62)
⏰ 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 / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: typecheck / typecheck
- 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 (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (17)
apps/webapp/app/components/logs/LogDetailView.tsx (4)
112-119: Fetcher included in effect but eslint rule disabled.The
fetcherobject is intentionally excluded from the dependency array to avoid infinite re-fetching loops, which is a known pattern with Remix fetchers. The eslint-disable comment is appropriate here.
167-231: Main component structure looks good.The component properly handles loading states, error cases (log not found), keyboard navigation (Escape to close), and tab switching. The header with badges and the tab navigation with keyboard shortcuts (d/r) provide a good UX.
234-291: DetailsTab implementation is clean.The error message extraction logic for ERROR level logs (Lines 250-256) and the conditional rendering of attributes are well-handled. The use of
PacketDisplaywithsearchTermenables consistent search highlighting.
330-510: RunTab displays comprehensive run information.The property table covers all relevant run details (status, task, version, costs, etc.) with appropriate formatting and navigation links. The conditional rendering for optional fields (rootRun, batch, tags) is handled correctly.
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (5)
385-421: Level filtering logic is well-structured.The
levelToKindsAndStatusesmapping and the dynamic query building for level filters handle the complex relationship between display levels and ClickHouse kinds/statuses correctly. The TRACE exclusion logic for error/cancelled statuses (Lines 399-402) is a thoughtful edge case handling.
372-382: Search query uses parameterized values - safe from SQL injection.The search implementation correctly uses ClickHouse parameterized queries (
{searchPattern: String},{statusTerm: String}), preventing SQL injection. The ILIKE pattern matching with%${searchTerm}%is properly escaped through the query builder.
256-324: Run-level filter handling with early return is efficient.The pattern of fetching matching run IDs from Postgres first and returning early when no matches are found (Lines 293-324) avoids unnecessary ClickHouse queries. The
MAX_RUN_IDS = 5000limit is a reasonable safeguard.
477-507: Log transformation handles edge cases well.The error message extraction from attributes (Lines 481-491) and the composite ID generation using
::separator (Line 494) are implemented correctly. The duration conversion handles both number and bigint types.
75-86: Cursor encoding/decoding is straightforward.The base64-encoded JSON cursor pattern is a standard approach for cursor-based pagination. The
decodeCursorfunction safely handles parse errors by returningnull.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (4)
59-66: Admin-only access is enforced correctly.The loader properly checks for admin status and redirects non-admin users. This aligns with the PR description stating this is an admin feature.
117-165: Page component with Suspense boundary is well-structured.The use of
SuspensewithTypedAwaitfor streaming deferred data provides a good loading experience. The error boundary with a user-friendly message handles failures gracefully.
274-334: Resizable panel layout provides good UX.The two-panel layout with
ResizablePanelGroupallows users to adjust the logs list and detail panel sizes. The conditional rendering of the detail panel when a log is selected is clean.
88-96: Presenter instantiation and call look correct.The
LogsListPresenteris properly instantiated with$replicaandclickhouseClient, and the call passes all required filters including the admin-controlledincludeDebugLogsflag.apps/webapp/app/components/logs/LogsTable.tsx (4)
92-115: IntersectionObserver implementation is correct.The observer is properly set up with cleanup, and the threshold of 0.1 triggers loading when the sentinel is barely visible. The guards for
hasMore,isLoadingMore, andonLoadMoreprevent unnecessary observations.
78-90: Delayed loading spinner prevents UI flicker.The 200ms delay before showing the "load more" spinner is a good UX pattern that prevents brief flashes for fast loads while still providing feedback for longer operations.
137-200: Table row rendering is well-implemented.Each row correctly handles selection state, click events, level-based styling, and search highlighting. The use of
TruncatedCopyableValuefor run IDs and the action menu for "View Run" provide useful functionality.
226-248: BlankState provides good user guidance.The empty state message and refresh button help users understand there are no matching logs and provide a clear action. The conditional rendering based on
isLoadingprevents showing the message during initial load.
| const fetcher = useTypedFetcher<RunContextData>(); | ||
| const [requested, setRequested] = useState(false); | ||
|
|
||
| // Fetch run details when tab is active | ||
| 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)}` | ||
| ); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [organization.slug, project.slug, environment.slug, log.id, log.runId]); | ||
|
|
||
| const isLoading = !requested || fetcher.state === "loading"; |
There was a problem hiding this comment.
Potential loading state logic issue in RunTab.
The isLoading check at Line 311 uses !requested || fetcher.state === "loading", but requested is set to true synchronously before the fetch starts (Line 304). This means isLoading will briefly be false between setting requested = true and the fetcher transitioning to the "loading" state, potentially causing a flash of the "Run not found" message.
Consider using fetcher.state !== "idle" || !fetcher.data pattern, or checking if data exists:
Suggested fix
- const isLoading = !requested || fetcher.state === "loading";
+ const isLoading = fetcher.state === "loading" || (!fetcher.data && fetcher.state === "idle" && !requested);Or alternatively, check fetcher.state more comprehensively:
- const isLoading = !requested || fetcher.state === "loading";
+ const isLoading = fetcher.state !== "idle" || !fetcher.data;📝 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 fetcher = useTypedFetcher<RunContextData>(); | |
| const [requested, setRequested] = useState(false); | |
| // Fetch run details when tab is active | |
| 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)}` | |
| ); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [organization.slug, project.slug, environment.slug, log.id, log.runId]); | |
| const isLoading = !requested || fetcher.state === "loading"; | |
| const fetcher = useTypedFetcher<RunContextData>(); | |
| const [requested, setRequested] = useState(false); | |
| // Fetch run details when tab is active | |
| 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)}` | |
| ); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [organization.slug, project.slug, environment.slug, log.id, log.runId]); | |
| const isLoading = fetcher.state === "loading" || (!fetcher.data && fetcher.state === "idle" && !requested); |
| const fetcher = useTypedFetcher<RunContextData>(); | |
| const [requested, setRequested] = useState(false); | |
| // Fetch run details when tab is active | |
| 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)}` | |
| ); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [organization.slug, project.slug, environment.slug, log.id, log.runId]); | |
| const isLoading = !requested || fetcher.state === "loading"; | |
| const fetcher = useTypedFetcher<RunContextData>(); | |
| const [requested, setRequested] = useState(false); | |
| // Fetch run details when tab is active | |
| 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)}` | |
| ); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [organization.slug, project.slug, environment.slug, log.id, log.runId]); | |
| const isLoading = fetcher.state !== "idle" || !fetcher.data; |
🤖 Prompt for AI Agents
In @apps/webapp/app/components/logs/LogDetailView.tsx around lines 297 - 311,
The loading logic can flash "Run not found" because requested is set true
synchronously before fetcher.state becomes "loading"; update the isLoading check
to rely on fetcher state and/or presence of data instead of the requested flag:
replace the current isLoading = !requested || fetcher.state === "loading" with a
condition such as fetcher.state !== "idle" || !fetcher.data (or fetcher.state
=== "loading" || fetcher.state === "submitting" || !fetcher.data) so the UI
remains in loading state until the fetcher actually has results; adjust any UI
branches that use isLoading accordingly and remove or repurpose the requested
state (used with fetcher.load in the useEffect) to avoid the transient false
state.
| <TableCell className="min-w-24"> | ||
| <TruncatedCopyableValue value={log.runId} /> | ||
| </TableCell> |
There was a problem hiding this comment.
Run cell missing onClick handler.
The Run column cell (Lines 166-168) doesn't have an onClick handler or hasAction prop, unlike other cells. This means clicking on the run ID won't select the log row. Consider adding the handler for consistency:
Suggested fix
- <TableCell className="min-w-24">
+ <TableCell className="min-w-24" onClick={handleRowClick} hasAction>
<TruncatedCopyableValue value={log.runId} />
</TableCell>🤖 Prompt for AI Agents
In @apps/webapp/app/components/logs/LogsTable.tsx around lines 166 - 168, The
Run cell (TableCell containing TruncatedCopyableValue) is missing the click
handler and hasAction flag—make it behave like the other cells by adding the
same onClick callback used to select a log row (e.g., the component's select/log
click handler such as onRowClick, onSelectLog, or handleSelect) to that
TableCell and set hasAction={true} so clicks on the runId select the row; ensure
you call the existing selection function with the current log (or log.id) and
preserve any event handling/stopPropagation logic used elsewhere.
| useEffect(() => { | ||
| if (fetcher.data && fetcher.state === "idle") { | ||
| const existingIds = new Set(accumulatedLogs.map((log) => log.id)); | ||
| const newLogs = fetcher.data.logs.filter((log) => !existingIds.has(log.id)); | ||
| if (newLogs.length > 0) { | ||
| setAccumulatedLogs((prev) => [...prev, ...newLogs]); | ||
| setNextCursor(fetcher.data.pagination.next); | ||
| } | ||
| } | ||
| }, [fetcher.data, fetcher.state, accumulatedLogs]); |
There was a problem hiding this comment.
Potential stale closure with accumulatedLogs in dependency array.
Including accumulatedLogs in the dependency array of the effect that appends new logs could cause the effect to re-run unnecessarily or miss updates. The deduplication check on Line 214 creates a new Set on every run.
Consider using a functional update or ref to avoid the dependency:
Suggested fix
useEffect(() => {
if (fetcher.data && fetcher.state === "idle") {
- const existingIds = new Set(accumulatedLogs.map((log) => log.id));
- const newLogs = fetcher.data.logs.filter((log) => !existingIds.has(log.id));
- if (newLogs.length > 0) {
- setAccumulatedLogs((prev) => [...prev, ...newLogs]);
- setNextCursor(fetcher.data.pagination.next);
- }
+ setAccumulatedLogs((prev) => {
+ const existingIds = new Set(prev.map((log) => log.id));
+ const newLogs = fetcher.data.logs.filter((log) => !existingIds.has(log.id));
+ if (newLogs.length > 0) {
+ return [...prev, ...newLogs];
+ }
+ return prev;
+ });
+ setNextCursor(fetcher.data.pagination.next);
}
- }, [fetcher.data, fetcher.state, accumulatedLogs]);
+ }, [fetcher.data, fetcher.state]);📝 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.
| useEffect(() => { | |
| if (fetcher.data && fetcher.state === "idle") { | |
| const existingIds = new Set(accumulatedLogs.map((log) => log.id)); | |
| const newLogs = fetcher.data.logs.filter((log) => !existingIds.has(log.id)); | |
| if (newLogs.length > 0) { | |
| setAccumulatedLogs((prev) => [...prev, ...newLogs]); | |
| setNextCursor(fetcher.data.pagination.next); | |
| } | |
| } | |
| }, [fetcher.data, fetcher.state, accumulatedLogs]); | |
| useEffect(() => { | |
| if (fetcher.data && fetcher.state === "idle") { | |
| setAccumulatedLogs((prev) => { | |
| const existingIds = new Set(prev.map((log) => log.id)); | |
| const newLogs = fetcher.data.logs.filter((log) => !existingIds.has(log.id)); | |
| if (newLogs.length > 0) { | |
| return [...prev, ...newLogs]; | |
| } | |
| return prev; | |
| }); | |
| setNextCursor(fetcher.data.pagination.next); | |
| } | |
| }, [fetcher.data, fetcher.state]); |
🤖 Prompt for AI Agents
In
@apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
around lines 212 - 221, The effect that appends fetched logs is suffering from a
stale-closure / unnecessary re-run due to including accumulatedLogs in the
dependency array; change the logic in the useEffect that reads
fetcher.data/fetcher.state so it does not depend on accumulatedLogs: when
fetcher.data && fetcher.state === "idle", call setAccumulatedLogs with a
functional updater (prev => { const existingIds = new Set(prev.map(l => l.id));
const newLogs = fetcher.data.logs.filter(l => !existingIds.has(l.id)); return
newLogs.length ? [...prev, ...newLogs] : prev; }) and then call
setNextCursor(fetcher.data.pagination.next); finally remove accumulatedLogs from
the effect dependency array so the effect only depends on fetcher.data and
fetcher.state (alternatively use a ref for existing IDs if you prefer).
PR Review: feat: tri-6738 Create aggregated logs pageI have reviewed this PR that adds a new aggregated logs page. Overall, this is a solid implementation with good architecture choices. Here is my detailed feedback: What is Good
Suggestions for Improvement1. SQL Injection Consideration (Low Risk)In 2. Unused Import and ParameterIn 3. Potential Memory Issue with Log AccumulationIn 4. React Hook Dependency WarningIn 5. Duplicate Level Badge Color Functions
6. Error Handling in JSON ParsingIn Potential Bugs1.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/webapp/app/presenters/v3/LogsListPresenter.server.ts:
- Around line 84-85: The machines Zod validation currently uses
z.array(z.string()).optional(); change it to validate against the
MachinePresetName enum/schema by replacing z.array(z.string()) with
z.array(MachinePresetName), i.e. machines:
z.array(MachinePresetName).optional(), so the runtime schema matches the
TypeScript MachinePresetName[] type used by LogsListPresenter.server.ts and
prevents invalid preset names.
🧹 Nitpick comments (10)
apps/webapp/app/components/logs/LogsTable.tsx (2)
165-167: Consider addingonClickhandler to the Run cell for consistent row selection.The Run cell doesn't have
onClick={handleRowClick}andhasAction, unlike other cells. This means clicking on the run ID area won't select the row. If this is intentional (to allow copying without selecting), consider adding a comment to clarify.
225-247: Consider extracting the page reload into a callback.Using
window.location.reload()directly works but couples the component to browser globals. For better testability, consider accepting an optionalonRefreshprop.apps/webapp/app/components/logs/LogDetailView.tsx (3)
72-79: Consider adding error handling for the fetch.The fetcher loads log details but there's no explicit error handling if the fetch fails. While
fetcher.data ?? initialLogprovides a fallback, consider checkingfetcher.datafor error responses or adding error state handling.♻️ Suggested improvement
const isLoading = fetcher.state === "loading"; - const log = fetcher.data ?? initialLog; + const fetchError = fetcher.data === null && fetcher.state === "idle" && !initialLog; + const log = fetcher.data ?? initialLog;Then handle
fetchErrorin the render logic if needed.
261-272: Potential race condition withrequestedstate.The
requestedflag is set totruebeforefetcher.load()completes. Iflog.idorlog.runIdchanges rapidly, the loading state logic at line 271 (!requested || fetcher.state === "loading") may briefly show incorrect states.Consider relying solely on
fetcher.statefor loading detection:♻️ Suggested simplification
- const [requested, setRequested] = useState(false); - // Fetch run details when tab is active 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)}` ); // eslint-disable-next-line react-hooks/exhaustive-deps }, [organization.slug, project.slug, environment.slug, log.id, log.runId]); - const isLoading = !requested || fetcher.state === "loading"; + const isLoading = fetcher.state === "loading" || fetcher.state === "submitting";
391-398: Redundant null check onenvironment.The
environmentvariable comes fromuseEnvironment()hook at line 256 and is used unconditionally earlier in the component (e.g., line 266). The conditional check here is unnecessary.♻️ Remove redundant check
- {environment && ( - <Property.Item> - <Property.Label>Environment</Property.Label> - <Property.Value> - <EnvironmentCombo environment={environment} /> - </Property.Value> - </Property.Item> - )} + <Property.Item> + <Property.Label>Environment</Property.Label> + <Property.Value> + <EnvironmentCombo environment={environment} /> + </Property.Value> + </Property.Item>apps/webapp/app/utils/logUtils.ts (2)
23-60: ReDoS risk is mitigated but consider input length limits.The regex special characters are properly escaped (line 33), which mitigates most ReDoS attack vectors. However, for defense in depth, consider adding an early return for extremely long search terms or text inputs, especially since this is user-facing.
🛡️ Optional: Add length guard
export function highlightSearchText( text: string, searchTerm?: string, style: React.CSSProperties = DEFAULT_HIGHLIGHT_STYLES ): ReactNode { if (!searchTerm || searchTerm.trim() === "") { return text; } + + // Skip highlighting for very long inputs to prevent performance issues + if (text.length > 100_000 || searchTerm.length > 1000) { + return text; + } // Escape special regex characters in search term
126-149: Consider consolidating repeated "Log" return values.Multiple cases return the same
"Log"string. While the current form is readable, you could simplify using a fallthrough pattern or a Set lookup.♻️ Alternative implementation
export function getKindLabel(kind: string): string { + const logKinds = new Set(["LOG_DEBUG", "LOG_INFO", "LOG_WARN", "LOG_ERROR", "LOG_LOG"]); + if (logKinds.has(kind)) return "Log"; + switch (kind) { case "SPAN": return "Span"; case "SPAN_EVENT": return "Event"; - case "LOG_DEBUG": - return "Log"; - case "LOG_INFO": - return "Log"; - case "LOG_WARN": - return "Log"; - case "LOG_ERROR": - return "Log"; - case "LOG_LOG": - return "Log"; case "DEBUG_EVENT": return "Debug"; case "ANCESTOR_OVERRIDE": return "Override"; default: return kind; } }apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (3)
12-16: Type assertion may cause runtime issues.The
as unknown as z.ZodSchema<TaskRunStatus[]>cast bypasses TypeScript's type checking. IfTaskRunStatusvalues change, this won't catch the mismatch at compile time.♻️ Consider using z.nativeEnum instead
-// Create a schema that validates TaskRunStatus enum values -const TaskRunStatusSchema = z.array(z.string()).refine( - (val) => val.every((v) => Object.values(TaskRunStatusEnum).includes(v)), - { message: "Invalid TaskRunStatus values" } -) as unknown as z.ZodSchema<TaskRunStatus[]>; +// Create a schema that validates TaskRunStatus enum values +const TaskRunStatusSchema = z.array(z.nativeEnum(TaskRunStatusEnum));This provides proper type inference without unsafe casts.
419-455: Improve type safety for dynamic params object.The
paramsobject is typed asRecord<string, any>and later cast withas any. Consider defining a proper type or using a type-safe builder pattern.♻️ Type-safe alternative
const conditions: string[] = []; - const params: Record<string, any> = {}; + const params: Record<string, string | string[]> = {}; const hasErrorOrCancelledLevel = levels.includes("ERROR") || levels.includes("CANCELLED");This provides better type safety while still allowing dynamic key names.
113-120: Consider validating cursor structure after decoding.The
decodeCursorfunction parses JSON and casts toLogCursorwithout validation. Malformed cursors could cause issues downstream.♻️ Add validation with zod
+const LogCursorSchema = z.object({ + startTime: z.string(), + traceId: z.string(), + spanId: z.string(), + runId: z.string(), +}); + function decodeCursor(cursor: string): LogCursor | null { try { const decoded = Buffer.from(cursor, "base64").toString("utf-8"); - return JSON.parse(decoded) as LogCursor; + const parsed = JSON.parse(decoded); + const result = LogCursorSchema.safeParse(parsed); + return result.success ? result.data : null; } catch { return null; } }As per coding guidelines, use zod for validation in apps/webapp.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/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.logs.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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 insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/components/logs/LogsTable.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/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/components/logs/LogsTable.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/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/components/logs/LogsTable.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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/components/logs/LogsTable.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/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/components/logs/LogsTable.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/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/components/logs/LogsTable.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/presenters/RunFilters.server.tsapps/webapp/app/components/logs/LogsTable.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/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/utils/logUtils.tsapps/webapp/app/presenters/RunFilters.server.ts
🧠 Learnings (19)
📚 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-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/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/presenters/RunFilters.server.ts
📚 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
📚 Learning: 2025-09-03T14:34:41.781Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2464
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx:357-371
Timestamp: 2025-09-03T14:34:41.781Z
Learning: When using Zod's safeParse, the .data property is undefined when parsing fails, but TypeScript may still complain about accessing .data without checking .success first. The suggested approach of checking .success before accessing .data improves type safety and code clarity.
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: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
Applied to files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/RunFilters.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/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/LogsListPresenter.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: 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: 2025-08-14T10:09:02.528Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2390
File: internal-packages/run-engine/src/engine/index.ts:466-467
Timestamp: 2025-08-14T10:09:02.528Z
Learning: In the triggerdotdev/trigger.dev codebase, it's acceptable to pass `string | undefined` types directly to Prisma operations (both create and update). The codebase consistently uses this pattern and the team is comfortable with how Prisma handles undefined values.
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2026-01-12T11:01:34.777Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:01:34.777Z
Learning: Applies to internal-packages/database/prisma/migrations/**/*.sql : Edit database schema in internal-packages/database/prisma/schema.prisma; remove extraneous lines from migrations related to BackgroundWorker, TaskRun tags, waitpoints, and SecretStore indexes
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.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 `tasks.trigger()` with type-only imports to trigger tasks from backend code without importing the task implementation
Applied to files:
apps/webapp/app/presenters/RunFilters.server.ts
📚 Learning: 2026-01-12T11:01:34.777Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:01:34.777Z
Learning: Applies to **/*.{ts,tsx} : Every Trigger.dev task must be exported; use task() function with unique id and run async function
Applied to files:
apps/webapp/app/presenters/RunFilters.server.ts
📚 Learning: 2025-08-14T20:37:30.914Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2390
File: apps/webapp/app/runEngine/services/batchTrigger.server.ts:787-795
Timestamp: 2025-08-14T20:37:30.914Z
Learning: The webapp uses ts-reset which provides better TypeScript type narrowing for common patterns like filter(Boolean), making explicit type guards unnecessary in many cases.
Applied to files:
apps/webapp/app/presenters/RunFilters.server.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 the `task()` function from `trigger.dev/sdk/v3` to define tasks with id and run properties
Applied to files:
apps/webapp/app/presenters/RunFilters.server.ts
📚 Learning: 2026-01-12T11:01:34.777Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:01:34.777Z
Learning: Applies to **/*.{ts,tsx,js} : Always import task definitions from trigger.dev/sdk, never from trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Applied to files:
apps/webapp/app/presenters/RunFilters.server.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:
apps/webapp/app/presenters/RunFilters.server.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} : Subscribe to run updates using `runs.subscribeToRun()` for realtime monitoring of task execution
Applied to files:
apps/webapp/app/presenters/RunFilters.server.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:
apps/webapp/app/presenters/RunFilters.server.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:
apps/webapp/app/presenters/RunFilters.server.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 `yourTask.batchTrigger()` to trigger multiple runs of a task from inside another task
Applied to files:
apps/webapp/app/presenters/RunFilters.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/components/primitives/Table.tsx (1)
apps/webapp/app/utils/cn.ts (1)
cn(77-79)
🪛 ast-grep (0.40.4)
apps/webapp/app/utils/logUtils.ts
[warning] 33-33: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(escapedSearch, "gi")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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 / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- 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)
🔇 Additional comments (7)
apps/webapp/app/components/primitives/Table.tsx (2)
10-58: LGTM! Well-structured variant system.The variant-driven styling approach with a centralized
variantsobject and context propagation is clean and maintainable. The new"compact/mono"variant properly defines all required style keys consistent with other variants.
69-90: Good use of React Context for variant propagation.The
TableContextpattern cleanly propagates the variant to all descendant components without prop drilling. The default value of"dimmed"ensures backward compatibility.apps/webapp/app/components/logs/LogsTable.tsx (1)
91-114: LGTM! Proper IntersectionObserver cleanup.The infinite scroll implementation correctly captures
loadMoreRef.currentin a local variable before cleanup, preventing potential issues with stale refs during unmount.apps/webapp/app/utils/logUtils.ts (1)
3-3: Good use of string union type instead of enum.The
LogLeveltype correctly uses a string union per the coding guidelines.apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (1)
160-164: LGTM! Clean presenter structure.The
LogsListPresenterclass follows the established presenter pattern with dependency injection ofreplicaandclickhouseclients.apps/webapp/app/presenters/RunFilters.server.ts (2)
1-1: LGTM!Correct use of type-only import syntax for the database type.
43-43: Type assertion is unnecessary; alignment is already enforced at compile time.The
allTaskRunStatusesarray is explicitly constrained withsatisfies Readonly<Array<TaskRunStatus>>, ensuring compile-time validation. Any divergence between the array values andTaskRunStatuswould cause a TypeScript compilation error. The Zod schema already usesz.enum(allTaskRunStatuses), which properly derives the enum from the constrained array. After parsing withTaskRunListSearchFilters.parse(), the inferred type is already correct, making the type assertion redundant. You can safely remove the assertion and rely on Zod's type inference.
| queues: z.array(z.string()).optional(), | ||
| machines: z.array(z.string()).optional(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the LogsListPresenter.server.ts file to understand the context
head -100 apps/webapp/app/presenters/v3/LogsListPresenter.server.ts | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 4050
🏁 Script executed:
# Search for MachinePresetName definition in the codebase
fd -t f -e ts -e tsx | xargs rg "type MachinePresetName|export.*MachinePresetName" -A 3 | head -50Repository: triggerdotdev/trigger.dev
Length of output: 2179
🏁 Script executed:
# Look for how MachinePresetName is used in validation contexts
rg "MachinePresetName" -B 2 -A 2 --type ts --type tsx | head -80Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
# Check the imports in the LogsListPresenter.server.ts file
sed -n '1,30p' apps/webapp/app/presenters/v3/LogsListPresenter.server.ts | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 1608
Use MachinePresetName Zod schema for machines validation.
The machines field should validate against the MachinePresetName enum schema to match the TypeScript type definition (MachinePresetName[]) and prevent invalid machine preset names at runtime. Update to machines: z.array(MachinePresetName).optional().
🤖 Prompt for AI Agents
In @apps/webapp/app/presenters/v3/LogsListPresenter.server.ts around lines 84 -
85, The machines Zod validation currently uses z.array(z.string()).optional();
change it to validate against the MachinePresetName enum/schema by replacing
z.array(z.string()) with z.array(MachinePresetName), i.e. machines:
z.array(MachinePresetName).optional(), so the runtime schema matches the
TypeScript MachinePresetName[] type used by LogsListPresenter.server.ts and
prevents invalid preset names.
Code Review: PR #2862 - Aggregated Logs PageThanks for this comprehensive new feature! The aggregated logs page is a solid addition to the dashboard. I have reviewed the code and have feedback organized by category. Overall AssessmentThis is a well-structured feature implementation with good separation of concerns. The ClickHouse query builder patterns are well-designed for performance, and the UI components follow existing patterns in the codebase. However, there are a few areas that need attention. Critical Issues1. Missing ChangesetPer the repository CLAUDE.md guidelines, any modifications to public packages require a changeset:
The pnpm run changeset:add2. No Test CoverageThe CLAUDE.md explicitly states: "We use vitest exclusively. Never mock anything - use testcontainers instead." This PR adds significant new functionality including:
At minimum, the utility functions in Security Considerations3. SQL Injection Protection - Good!The search implementation properly uses parameterized queries in 4. Admin-Only Access - EnforcedGood to see the page is protected to admin users only ( Code Quality Issues5. Unused
|
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
💯