Fix/tri 7032 logs page feedback #2947
Conversation
Added ellipsis to logs loading text Changed log detail View full run variant to minimal Normalized Log level between table column and Logs detail view Fixed Logs table sidebar showing on the left of the header Fixed table header disappearing on scroll Removed search from logs filter Changed Icon to logs filter Side panel closes when the filters change Made the loading segment taller and stopped the resizing when it appears Showing a message when there are no more logs to show that includes the number of logs Fixed bug where logs from the previous search remained in the table
added debug level (logger.debug) by default fixed issue where when changing filters the table would not scroll up removed internal logs from list fixed ServiceValidationError not being sent to FE Removed run filters removed v1 support for logs
|
WalkthroughRemoves TRACE and CANCELLED log levels from schemas and utils; consolidates ClickHouse log query builders to V2 and adds full‑text indexes for task events. Introduces plan‑aware retentionLimitDays into logs loader/presenter, changes cursor shape to { environmentId, unixTimestamp }, and simplifies runId handling. Adds UI components LogLevelTooltipInfo and LogsTaskFilter, animates LogsSearchInput, changes level highlighting to box‑shadow, updates LogsLevelFilter and LogsTable APIs (including Table.showTopBorder and TableCell.style), adjusts LogsRunIdFilter shortcut, and adds useCanViewLogsPage plus a loader route to gate log UI. Routes and list UI refactored to URL‑driven filters with deduplication and retention notices. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
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/presenters/v3/LogsListPresenter.server.ts (1)
296-305: Unused variablestatusTermin search query parameters.The
statusTermvariable is computed but never used in the query template. This appears to be leftover code from a previous implementation.♻️ Proposed fix
if (search && search.trim() !== "") { const searchTerm = search.trim(); queryBuilder.where( "(lower(message) like {searchPattern: String} OR lower(attributes_text) like {searchPattern: String})", { searchPattern: `%${searchTerm.toLowerCase()}%`, - statusTerm: searchTerm.toUpperCase(), } ); }
🤖 Fix all issues with AI agents
In `@apps/webapp/app/components/logs/LogDetailView.tsx`:
- Line 25: When building the URL for a log's run/span, don’t call v3RunSpanPath
with an empty spanId (which yields a trailing `span=`); instead, conditionally
use v3RunSpanPath(runId, spanId) only when spanId is truthy and fall back to
v3RunsPath(runId) when spanId is missing. Update all occurrences (e.g., the
import usage around v3RunSpanPath, v3RunsPath and the code blocks referencing
spanId) so links omit the empty span param.
- Around line 105-116: The global key handler in useEffect (handleKeyDown)
triggers while the user is typing; update handleKeyDown to early-return when the
event target is an editable element by checking if e.target is an HTMLElement
and if its tagName is INPUT, TEXTAREA, SELECT or it has isContentEditable true
(or other editor-specific classes), so the Escape and "v" shortcut only run when
focus is not inside an input-like element; keep references to onClose, runPath,
isLoading and log unchanged and only add this guard at the top of handleKeyDown.
In `@apps/webapp/app/presenters/v3/LogsListPresenter.server.ts`:
- Around line 240-244: The code incorrectly throws ServiceValidationError for
CLICKHOUSE in addition to POSTGRES, blocking all log retrievals; update the
conditional logic in LogsListPresenter.server.ts (around the EVENT_STORE_TYPES
checks) so that only POSTGRES (or the unsupported store) raises
ServiceValidationError and ensure CLICKHOUSE is treated as the supported path
(remove or adjust the throw for EVENT_STORE_TYPES.CLICKHOUSE), keeping the
existing ServiceValidationError usage and message intact for the unsupported
store branch.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx:
- Around line 322-326: The clear-filters control currently renders a <Form>
around a <Button> but has no action/submit handler so clicks do nothing; update
both occurrences (the hasFilters blocks around Form/Button) to either a) add an
explicit Form action that triggers a server action (or POST handler) which
redirects to the same route without query params so filters are cleared and
ensure the Button is type="submit", or b) convert the Button to a plain button
with an onClick handler that uses the router/navigation (e.g., navigate/replace)
to reload the current route with query params removed; reference the existing
Form and Button in the hasFilters blocks and ensure Button has type="submit"
when using Form action or an onClick when not.
In
`@apps/webapp/app/routes/resources.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts:
- Around line 51-54: The parsed timestamps fromStr/toStr using parseInt can
produce NaN; update the parsing for from and to so after parseInt you validate
with Number.isFinite or !Number.isNaN and set the variables to undefined when
invalid (e.g., const from = fromStr ? parseInt(fromStr, 10) : undefined; if
(Number.isNaN(from)) from = undefined), or alternatively return a 400 response
when invalid; apply the same fix for to—ensure you reference the fromStr, toStr,
parseInt, from, and to identifiers so the presenter never receives NaN values.
- Around line 56-70: The code references isAdmin but never defines it; change
the auth call to use requireUser (not just requireUserId), derive userId from
the returned user object and compute isAdmin from that user before calling
LogsListOptionsSchema.parse. Specifically, update imports to import requireUser,
call const user = await requireUser(request), set const userId = user.id
(replace existing requireUserId usage) and set const isAdmin =
Boolean(user.isAdmin) (or compute from user.roles if your user model uses roles)
and then pass isAdmin into the options passed to LogsListOptionsSchema.parse.
In `@internal-packages/clickhouse/schema/014_add_task_runs_v2_serch_indexes.sql`:
- Around line 4-20: The Down migration uses the wrong index name for the message
index and should mirror the Up; change the DROP INDEX statement for the message
index to drop message_text_search (not idx_message_text_search) and add IF
EXISTS to both DROP INDEX statements for symmetry and safe rollback against
trigger_dev.task_events_v2 (ensure the names match the Up-created indexes:
idx_attributes_text_search and message_text_search).
🧹 Nitpick comments (7)
apps/webapp/app/components/logs/LogsTaskFilter.tsx (1)
25-27: Prefer a type alias for props.
This file is TS/TSX and the repo standard is to use type aliases for props.As per coding guidelines, use type aliases over interfaces in TS/TSX.♻️ Suggested change
-interface LogsTaskFilterProps { - possibleTasks: TaskOption[]; -} +type LogsTaskFilterProps = { + possibleTasks: TaskOption[]; +};apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (2)
352-360: Remove commented-out dead code.These commented lines appear to be debugging artifacts or incomplete refactoring. They should be removed to maintain code cleanliness.
♻️ Proposed fix
queryBuilder.where("kind NOT IN {debugSpans: Array(String)}", { debugSpans: ["SPAN", "ANCESTOR_OVERRIDE", "SPAN_EVENT"], }); - // kindCondition += ` `; - // params["excluded_statuses"] = ["SPAN", "ANCESTOR_OVERRIDE", "SPAN_EVENT"]; - queryBuilder.where("NOT (kind = 'SPAN' AND status = 'PARTIAL')");
257-268: Inconsistent indentation onqueryBuilder.wherecall.Line 261 has extra indentation that doesn't match the surrounding code style.
♻️ Proposed fix
if (effectiveFrom) { const fromNs = convertDateToNanoseconds(effectiveFrom); - queryBuilder.where("inserted_at >= {insertedAtStart: DateTime64(3)}", { - insertedAtStart: convertDateToClickhouseDateTime(effectiveFrom), - }); + queryBuilder.where("inserted_at >= {insertedAtStart: DateTime64(3)}", { + insertedAtStart: convertDateToClickhouseDateTime(effectiveFrom), + }); queryBuilder.where("start_time >= {fromTime: String}", { fromTime: formatNanosecondsForClickhouse(fromNs), }); }apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts (1)
70-70: Unnecessaryas anycast after Zod validation.The
as anycast undermines the type safety that Zod validation provides. The comment suggests it's validated at runtime, but the types should align without needing to escape toany.♻️ Consider removing the cast
If the types don't align, consider using
z.infer<typeof LogsListOptionsSchema>to derive the type, or adjustLogsListOptionsto match the schema output.- }) as any; // Validated by LogsListOptionsSchema at runtime + });apps/webapp/app/components/logs/LogsTable.tsx (1)
42-58: Dead code: CANCELLED and TRACE cases will never be reached.The
CANCELLEDandTRACElog levels have been removed from the system (seeLogsLevelFilter.tsxandlogUtils.tschanges). These switch cases are now unreachable and should be removed for consistency.♻️ Proposed fix
function getLevelBoxShadow(level: LogEntry["level"]): string { switch (level) { case "ERROR": return "inset 2px 0 0 0 rgb(239, 68, 68)"; case "WARN": return "inset 2px 0 0 0 rgb(234, 179, 8)"; case "INFO": return "inset 2px 0 0 0 rgb(59, 130, 246)"; - case "CANCELLED": - return "inset 2px 0 0 0 rgb(65, 65, 75)"; case "DEBUG": - case "TRACE": default: return "none"; } }apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (2)
315-341: Duplicate JSX in FiltersBar conditional branches.The filter UI is duplicated between the
listand no-list branches. This can be simplified by extracting the common JSX.♻️ Proposed fix
return ( <div className="flex items-start justify-between gap-x-2 border-b border-grid-bright p-2"> <div className="flex flex-row flex-wrap items-center gap-1"> - {list ? ( - <> - <LogsTaskFilter possibleTasks={list.possibleTasks} /> - <LogsRunIdFilter /> - <TimeFilter defaultPeriod={defaultPeriod} /> - <LogsLevelFilter/> - <LogsSearchInput /> - {hasFilters && ( - <Form className="h-6"> - <Button variant="secondary/small" LeadingIcon={XMarkIcon} tooltip="Clear all filters" /> - </Form> - )} - </> - ) : ( - <> - <LogsTaskFilter possibleTasks={[]} /> - <LogsRunIdFilter /> - <TimeFilter defaultPeriod={defaultPeriod} /> - <LogsLevelFilter/> - <LogsSearchInput /> - {hasFilters && ( - <Form className="h-6"> - <Button variant="secondary/small" LeadingIcon={XMarkIcon} tooltip="Clear all filters" /> - </Form> - )} - </> - )} + <LogsTaskFilter possibleTasks={list?.possibleTasks ?? []} /> + <LogsRunIdFilter /> + <TimeFilter defaultPeriod={defaultPeriod} /> + <LogsLevelFilter /> + <LogsSearchInput /> + {hasFilters && ( + <Form className="h-6"> + <Button variant="secondary/small" LeadingIcon={XMarkIcon} tooltip="Clear all filters" /> + </Form> + )} </div>
302-310: Consider using Remix navigation instead of full page reload for debug toggle.Using
window.location.hrefcauses a full page reload. Consider using Remix'suseNavigateoruseSearchParamsfor a smoother experience.♻️ Alternative using useSearchParams
import { useSearchParams } from "@remix-run/react"; // In FiltersBar: const [searchParams, setSearchParams] = useSearchParams(); const handleDebugToggle = useCallback((checked: boolean) => { setSearchParams((prev) => { if (checked) { prev.set("showDebug", "true"); } else { prev.delete("showDebug"); } return prev; }); }, [setSearchParams]);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/webapp/app/components/LogLevelTooltipInfo.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsLevelFilter.tsxapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/components/primitives/Table.tsxapps/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.tsapps/webapp/app/utils/logUtils.tsinternal-packages/clickhouse/schema/014_add_task_runs_v2_serch_indexes.sqlinternal-packages/clickhouse/src/index.tsinternal-packages/clickhouse/src/taskEvents.ts
💤 Files with no reviewable changes (1)
- internal-packages/clickhouse/src/index.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 instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
internal-packages/clickhouse/src/taskEvents.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/LogLevelTooltipInfo.tsxapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsLevelFilter.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
internal-packages/clickhouse/src/taskEvents.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/LogLevelTooltipInfo.tsxapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsLevelFilter.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:
internal-packages/clickhouse/src/taskEvents.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/utils/logUtils.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
internal-packages/clickhouse/src/taskEvents.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/LogLevelTooltipInfo.tsxapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsLevelFilter.tsx
internal-packages/clickhouse/schema/**/*.sql
📄 CodeRabbit inference engine (CLAUDE.md)
internal-packages/clickhouse/schema/**/*.sql: ClickHouse migrations must use Goose format with-- +goose Upand-- +goose Downmarkers
Follow ClickHouse naming conventions:raw_prefix for input tables,_v1,_v2suffixes for versioning,_mv_v1suffix for materialized views
Files:
internal-packages/clickhouse/schema/014_add_task_runs_v2_serch_indexes.sql
{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/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/LogLevelTooltipInfo.tsxapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsLevelFilter.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/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/LogLevelTooltipInfo.tsxapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsLevelFilter.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 webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/LogLevelTooltipInfo.tsxapps/webapp/app/components/logs/LogsSearchInput.tsxapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsLevelFilter.tsx
🧠 Learnings (24)
📚 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:
internal-packages/clickhouse/src/taskEvents.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to internal-packages/database/prisma/migrations/**/*.sql : When editing the Prisma schema, remove extraneous migration lines related to specific tables: `_BackgroundWorkerToBackgroundWorkerFile`, `_BackgroundWorkerToTaskQueue`, `_TaskRunToTaskRunTag`, `_WaitpointRunConnections`, `_completedWaitpoints`, `SecretStore_key_idx`, and unrelated `TaskRun` indexes
Applied to files:
internal-packages/clickhouse/src/taskEvents.tsinternal-packages/clickhouse/schema/014_add_task_runs_v2_serch_indexes.sql
📚 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.tsapps/webapp/app/components/LogLevelTooltipInfo.tsxapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to internal-packages/clickhouse/schema/**/*.sql : ClickHouse migrations must use Goose format with `-- +goose Up` and `-- +goose Down` markers
Applied to files:
internal-packages/clickhouse/schema/014_add_task_runs_v2_serch_indexes.sql
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to internal-packages/clickhouse/schema/**/*.sql : Follow ClickHouse naming conventions: `raw_` prefix for input tables, `_v1`, `_v2` suffixes for versioning, `_mv_v1` suffix for materialized views
Applied to files:
internal-packages/clickhouse/schema/014_add_task_runs_v2_serch_indexes.sql
📚 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/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.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/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/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/components/LogLevelTooltipInfo.tsxapps/webapp/app/utils/logUtils.tsapps/webapp/app/components/logs/LogsLevelFilter.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} : Export tasks with unique IDs within the project to enable proper task discovery and execution
Applied to files:
apps/webapp/app/components/logs/LogsTaskFilter.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} : Export every task, including subtasks
Applied to files:
apps/webapp/app/components/logs/LogsTaskFilter.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 `schedules.task()` for scheduled/cron tasks instead of regular `task()`
Applied to files:
apps/webapp/app/components/logs/LogsTaskFilter.tsxapps/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 the `task()` function from `trigger.dev/sdk/v3` to define tasks with id and run properties
Applied to files:
apps/webapp/app/components/logs/LogsTaskFilter.tsxapps/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/components/logs/LogsTaskFilter.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to **/*.{ts,tsx} : Always import tasks from `trigger.dev/sdk`, never use `trigger.dev/sdk/v3` or deprecated `client.defineJob` pattern
Applied to files:
apps/webapp/app/components/logs/LogsTaskFilter.tsxapps/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} : Generate example payloads for tasks when possible
Applied to files:
apps/webapp/app/components/logs/LogsTaskFilter.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 `.withStreams()` to subscribe to realtime streams from task metadata in addition to run changes
Applied to files:
apps/webapp/app/components/logs/LogsTaskFilter.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} : 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/components/logs/LogsTaskFilter.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/utils/logUtils.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/**/*.{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-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: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/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 `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
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} : Subscribe to run updates using `runs.subscribeToRun()` for realtime monitoring of task execution
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2025-07-21T12:52:44.342Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2284
File: apps/webapp/app/services/realtimeClient.server.ts:111-127
Timestamp: 2025-07-21T12:52:44.342Z
Learning: Electric (the database service used in the realtimeClient) has built-in SQL injection protection and safely handles whereClause parameters passed via URL parameters, so direct string interpolation of runId values into SQL where clauses is safe when using Electric.
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
🧬 Code graph analysis (7)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts (2)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (2)
LogLevel(21-21)LogsListOptionsSchema(51-67)apps/webapp/app/utils/logUtils.ts (1)
LogLevel(5-5)
apps/webapp/app/components/LogLevelTooltipInfo.tsx (2)
apps/webapp/app/components/primitives/Paragraph.tsx (1)
Paragraph(88-107)apps/webapp/app/components/primitives/Buttons.tsx (1)
LinkButton(335-403)
apps/webapp/app/components/logs/LogsTaskFilter.tsx (5)
apps/webapp/app/hooks/useSearchParam.ts (1)
useSearchParams(7-64)apps/webapp/app/components/runs/v3/SharedFilters.tsx (2)
FilterMenuProvider(37-63)appliedSummary(852-862)apps/webapp/app/components/runs/v3/TaskTriggerSource.tsx (1)
TaskTriggerSourceIcon(6-23)apps/webapp/app/components/primitives/AppliedFilter.tsx (1)
AppliedFilter(32-75)apps/webapp/app/components/primitives/Select.tsx (4)
SelectProvider(378-382)SelectPopover(611-634)ComboBox(646-665)SelectList(428-441)
apps/webapp/app/components/logs/LogsTable.tsx (3)
apps/webapp/app/components/primitives/Table.tsx (3)
Table(73-102)TableHeader(109-125)TableHeaderCell(181-233)apps/webapp/app/components/LogLevelTooltipInfo.tsx (1)
LogLevelTooltipInfo(6-63)apps/webapp/app/utils/cn.ts (1)
cn(77-79)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (3)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (2)
LogsListPresenter(137-469)LogsList(71-71)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts (1)
loader(21-79)apps/webapp/app/components/runs/v3/SharedFilters.tsx (1)
TimeFilter(297-346)
apps/webapp/app/components/logs/LogDetailView.tsx (4)
apps/webapp/app/utils/pathBuilder.ts (1)
v3RunSpanPath(322-333)apps/webapp/app/utils/cn.ts (1)
cn(77-79)apps/webapp/app/utils/logUtils.ts (1)
getLevelColor(98-111)apps/webapp/app/components/primitives/Buttons.tsx (1)
Button(296-329)
apps/webapp/app/components/primitives/Table.tsx (2)
internal-packages/tsql/src/query/models.ts (1)
Table(41-49)apps/webapp/app/utils/cn.ts (1)
cn(77-79)
⏰ 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). (14)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 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 (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (27)
internal-packages/clickhouse/src/taskEvents.ts (1)
301-323: LGTM for the V2 log detail query builder.No concerns in this segment.
apps/webapp/app/components/logs/LogsSearchInput.tsx (1)
3-3: LGTM — animated search width feels smooth.
Nice touch wrapping the input with motion without altering submit/clear behavior.Also applies to: 56-65, 93-93
apps/webapp/app/components/LogLevelTooltipInfo.tsx (1)
1-61: LGTM — clear, compact tooltip content.
Good reuse of primitives and straightforward structure.apps/webapp/app/components/primitives/Table.tsx (2)
62-68: LGTM —showTopBorderadds helpful layout flexibility.
Defaulting totruekeeps existing behavior while enabling overrides.Also applies to: 73-91
235-246: LGTM —stylepassthrough enables targeted cell tweaks.
Forwarding to<td>is the right surface for this.Also applies to: 262-309
apps/webapp/app/utils/logUtils.ts (1)
4-8: LGTM — simplified level set is consistent.
Defaulting unknown kinds to INFO aligns with the trimmed schema.Also applies to: 72-94
apps/webapp/app/components/logs/LogDetailView.tsx (1)
24-24: LGTM — simplified header and action styling read well.
The level-only badge and minimal button variant are clear and consistent.Also applies to: 145-153, 180-182
apps/webapp/app/components/logs/LogsTaskFilter.tsx (1)
1-24: LGTM — solid URL-synced task filtering UX.
The dropdown + applied filter behavior looks coherent and easy to use.Also applies to: 29-143
apps/webapp/app/components/logs/LogsLevelFilter.tsx (5)
18-28: LGTM - Clean level definitions and simplified availability function.The removal of TRACE and CANCELLED levels aligns with the PR objective to normalize log level display. The
getAvailableLevelsfunction is now straightforward while leaving room for future extension as noted in the comment.
30-43: LGTM - Comprehensive badge color mapping with safe default.The
getLevelBadgeColorfunction properly handles all defined log levels with distinct visual styles and includes a sensible default fallback for any unexpected values.
47-70: LGTM - Simplified component with clean conditional rendering.The component now has a cleaner structure with the conditional rendering based on whether levels are applied.
72-110: LGTM - Streamlined dropdown component.The
LevelDropdownnow focuses solely on its core responsibility of rendering the selection UI. ThehandleChangeproperly resets cursor and direction when levels change, which aligns with the PR objective to fix table scroll position when changing filters.
112-135: LGTM - Applied filter with proper cleanup.The
AppliedLevelFiltercorrectly integrates with Ariakit's Select component and properly removes related pagination params (cursor,direction) when the filter is cleared.apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (3)
183-192: LGTM - Retention clamping logic.The retention-based time clamping is well-implemented, properly tracking whether the time window was adjusted due to retention limits.
76-86: LGTM - Updated cursor schema.The cursor structure change to use
environmentIdandunixTimestampaligns with the updated ordering in the query.
463-467: LGTM - Retention info in response.Properly exposing retention information in the response enables the UI to show retention notices when logs were clamped.
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts (1)
35-40: LGTM - Plan-based retention limit.Good use of
getCurrentPlanto derive retention limits from the user's subscription, with a sensible 30-day default fallback.apps/webapp/app/components/logs/LogsTable.tsx (5)
118-119: LGTM - Improved table container styling.Good addition of the top border on the container and removing it from the inner Table to avoid double borders. The scrollbar styling maintains consistency.
125-130: LGTM - Level header tooltip integration.Good UX enhancement using the new
LogLevelTooltipInfocomponent to provide contextual help about log levels.
159-166: LGTM - Box-shadow based level indicator.Using inline
boxShadowstyle for level highlighting is a valid approach for better scroll performance compared to border-based solutions.
193-198: LGTM - Improved "View run" action.Wrapping the button in a Link with
target="_blank"properly opens the run in a new tab, which is better UX for keeping the logs view intact.
207-226: LGTM - Improved infinite scroll UX.Good improvements:
- Increased padding (
py-12) gives more visual breathing room- Conditional spinner visibility prevents layout shift
- "Showing all N logs" message provides clear feedback when all logs are loaded
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (5)
387-396: LGTM - Proper handling of stale fetcher data.Using
fetcherFilterStateRefto track which filter state the fetcher request corresponds to is a good pattern to prevent race conditions when filters change during a pending load-more request.
256-278: LGTM - RetentionNotice component.Clean component with proper pluralization handling for the retention days message. The upgrade link is appropriately styled.
143-163: LGTM - ServiceValidationError handling.Good error handling pattern that catches
ServiceValidationErrorand returns it as a typed error object, allowing the UI to display a meaningful message instead of crashing.
485-495: LGTM - LogsTable with location.search key.Using
location.searchas thekeyprop ensures the table remounts when filters change, which resets scroll position - addressing the PR objective of fixing table scroll position when changing filters.
390-396: The empty state flash is already prevented by the existing loading state mechanism.When
location.searchchanges, Remix automatically triggers a navigation, settingnavigation.stateto "loading" before the new data arrives. TheisLoadingprop (derived fromnavigation.state !== "idle") is passed toLogsTable, which displays a loading placeholder (BlankStatewithTableBlankRow) instead of rendering the cleared logs. Users see the loading state immediately, not an empty list, so the current implementation already achieves the desired UX without needing changes.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
...p/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
Show resolved
Hide resolved
...app/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
Outdated
Show resolved
Hide resolved
...app/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
Show resolved
Hide resolved
internal-packages/clickhouse/schema/014_add_task_runs_v2_serch_indexes.sql
Show resolved
Hide resolved
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 120-121: In LogDetailView.tsx the useEffect keyboard handler
contains an explicit case for e.key === "v" that calls window.open(runPath,
"_blank"), but the same "v" shortcut is also registered via the Button/Link prop
shortcut={{ key: "v" }}, causing two tabs to open; remove the manual "v" branch
from the useEffect keyboard handler (the condition checking (e.key === "v" ||
e.key === "V") && !e.ctrlKey && ... && !isLoading && log) so that only the
Button/Link's shortcut triggers opening the run page in a single tab.
In `@apps/webapp/app/presenters/v3/LogsListPresenter.server.ts`:
- Around line 75-85: The debug-filtering and cursor uniqueness are incorrect:
update the filtering logic that checks includeDebugLogs (refer to
includeDebugLogs and the enum values DEBUG_EVENT and LOG_DEBUG) to exclude both
DEBUG_EVENT and LOG_DEBUG when includeDebugLogs === false, and change the cursor
representation (LogCursor type and LogCursorSchema) and pagination keys to use
millisecond precision and a stable tiebreaker by replacing unixTimestamp with a
millisecond timestamp produced by toUnixTimestamp64Milli(start_time) and adding
span_id to the cursor tuple (use span_id where span_id is referenced near line
420) so pagination keys are (environmentId, unixTimestampMillis, traceId,
spanId) and ensure the schema and encoding/decoding logic are updated
accordingly.
- Around line 345-348: The filter that hides debug logs only excludes
"DEBUG_EVENT" but also needs to exclude "LOG_DEBUG" when includeDebugLogs ===
false; update the query in the includeDebugLogs branch (the queryBuilder.where
call) to add "LOG_DEBUG" to the debugKinds array so both DEBUG_EVENT and
LOG_DEBUG are excluded (this aligns with levelToKindsAndStatuses() which treats
both as DEBUG-level). Ensure the change is applied where includeDebugLogs is
checked and the queryBuilder.where("kind NOT IN {debugKinds: Array(String)}", {
debugKinds: [...] }) call is updated to include both kinds.
🧹 Nitpick comments (2)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts (1)
3-36: Avoid double session lookups by derivinguserIdfromrequireUser.You already call
requireUserfor admin gating; reuse it to avoid two session reads.♻️ Proposed refactor
-import { requireUser, requireUserId } from "~/services/session.server"; +import { requireUser } from "~/services/session.server"; export const loader = async ({ request, params }: LoaderFunctionArgs) => { - const userId = await requireUserId(request); + const user = await requireUser(request); + const userId = user.id; const { projectParam, organizationSlug, envParam } = EnvironmentParamSchema.parse(params); @@ - const user = await requireUser(request); const isAdmin = user?.admin || user?.isImpersonating;apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (1)
355-357: Remove commented-out debug artifact.The commented-out
kindCondition/excluded_statuseslines appear to be leftover debug code. Consider removing for clarity.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsinternal-packages/clickhouse/schema/014_add_task_runs_v2_serch_indexes.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- internal-packages/clickhouse/schema/014_add_task_runs_v2_serch_indexes.sql
🧰 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
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.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/components/logs/LogDetailView.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
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/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/presenters/v3/LogsListPresenter.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 webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
**/*.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/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
🧠 Learnings (12)
📚 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/components/logs/LogDetailView.tsxapps/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/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
📚 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/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.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/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
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 the `task()` function from `trigger.dev/sdk/v3` to define tasks with id and run properties
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 `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
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/v3/LogsListPresenter.server.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to **/*.{ts,tsx} : Always import tasks from `trigger.dev/sdk`, never use `trigger.dev/sdk/v3` or deprecated `client.defineJob` pattern
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 `schedules.task()` for scheduled/cron tasks instead of regular `task()`
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} : Subscribe to run updates using `runs.subscribeToRun()` for realtime monitoring of task execution
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 packages/trigger-sdk/**/*.{ts,tsx} : In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2025-07-21T12:52:44.342Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2284
File: apps/webapp/app/services/realtimeClient.server.ts:111-127
Timestamp: 2025-07-21T12:52:44.342Z
Learning: Electric (the database service used in the realtimeClient) has built-in SQL injection protection and safely handles whereClause parameters passed via URL parameters, so direct string interpolation of runId values into SQL where clauses is safe when using Electric.
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
🧬 Code graph analysis (2)
apps/webapp/app/components/logs/LogDetailView.tsx (3)
apps/webapp/app/utils/pathBuilder.ts (1)
v3RunSpanPath(322-333)apps/webapp/app/utils/logUtils.ts (1)
getLevelColor(98-111)apps/webapp/app/components/primitives/Buttons.tsx (1)
Button(296-329)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts (2)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (2)
LogLevel(21-21)LogsListOptionsSchema(51-67)apps/webapp/app/utils/logUtils.ts (1)
LogLevel(5-5)
⏰ 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). (20)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 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 (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 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 (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (4)
apps/webapp/app/components/logs/LogsRunIdFilter.tsx (1)
17-17: LGTM!The shortcut key change from "r" to "i" is a sensible UX improvement—"i" serves as a clear mnemonic for "ID" in "Run ID".
apps/webapp/app/components/logs/LogDetailView.tsx (3)
24-24: LGTM!Import simplified to match the UI changes removing the kind badge.
105-126: LGTM!The input guard correctly prevents shortcuts from firing while typing in form controls, and the dependency array is complete.
155-167: LGTM!The simplified header with the level badge styling looks clean and consistent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🔴 1 issue in files not directly in the diff
🔴 LogDetailPresenter will crash for ClickHouse v1 event store users (apps/webapp/app/presenters/v3/LogDetailPresenter.server.ts:39-42)
The LogDetailPresenter still tries to access this.clickhouse.taskEvents.logDetailQueryBuilder() for non-v2 ClickHouse stores, but this method was removed from the taskEvents API in the PR changes.
Click to expand
Root Cause
In LogDetailPresenter.server.ts:39-42, the code still expects the v1 API:
const isClickhouseV2 = store === "clickhouse_v2";
const queryBuilder = isClickhouseV2
? this.clickhouse.taskEventsV2.logDetailQueryBuilder()
: this.clickhouse.taskEvents.logDetailQueryBuilder();However, in internal-packages/clickhouse/src/index.ts:210-218, the logDetailQueryBuilder method was removed from taskEvents (v1):
get taskEvents() {
return {
insert: insertTaskEvents(this.writer),
traceSummaryQueryBuilder: getTraceSummaryQueryBuilder(this.reader),
traceDetailedSummaryQueryBuilder: getTraceDetailedSummaryQueryBuilder(this.reader),
spanDetailsQueryBuilder: getSpanDetailsQueryBuilder(this.reader),
// logDetailQueryBuilder REMOVED
};
}Impact
While LogsListPresenter.server.ts:240-244 correctly throws errors for both POSTGRES and CLICKHOUSE (v1) stores, LogDetailPresenter only checks for postgres (line 33-37). Users with store === "clickhouse" (v1) will get a runtime error when trying to view log details.
Recommendation: Add a check for the v1 ClickHouse store similar to what's done in LogsListPresenter:
if (store === "clickhouse") {
throw new ServiceValidationError(
"Log details are not available for ClickHouse v1 event store. Please contact support."
);
}View issue and 11 additional flags in Devin Review.
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`:
- Line 373: The current orderBy call on queryBuilder uses "environment_id,
toUnixTimestamp(start_time), trace_id DESC" which applies DESC only to trace_id
and causes mixed ordering; change the orderBy to explicitly sort newest-first
for both timestamp and tie-breaker (e.g. "environment_id,
toUnixTimestamp(start_time) DESC, trace_id DESC") and update the cursor
comparison used in pagination (the comparison around the cursor check) from ">"
to "<" so the cursor logic matches descending order; ensure you update both the
orderBy string in queryBuilder.orderBy and the cursor comparison used when
building the WHERE clause.
🧹 Nitpick comments (3)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (3)
354-356: Remove commented-out dead code.These commented lines appear to be leftover from refactoring and should be removed:
- // kindCondition += ` `; - // params["excluded_statuses"] = ["SPAN", "ANCESTOR_OVERRIDE", "SPAN_EVENT"]; -
257-267: Minor: Inconsistent indentation.Line 261 has extra indentation compared to surrounding code:
- queryBuilder.where("inserted_at >= {insertedAtStart: DateTime64(3)}", { + queryBuilder.where("inserted_at >= {insertedAtStart: DateTime64(3)}", { insertedAtStart: convertDateToClickhouseDateTime(effectiveFrom), });
240-244: Clarify the error message to indicate ClickHouse V2 requirement.The code intentionally rejects both
POSTGRESandCLICKHOUSE(v1) stores because logs functionality only supportsCLICKHOUSE_V2(as evidenced by the V2 query builder at line 246). To provide clearer guidance to users, improve the error message:Suggested change
if (store === EVENT_STORE_TYPES.CLICKHOUSE) { throw new ServiceValidationError( - "Logs are not available for ClickHouse event store. Please contact support." + "Logs require ClickHouse V2 event store. Please contact support to migrate." ); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.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 (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
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/logs/LogDetailView.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/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
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/logs/LogDetailView.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 webapp
Files:
apps/webapp/app/components/logs/LogDetailView.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 webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/logs/LogsTable.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/logs/LogDetailView.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 (13)
📚 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/components/logs/LogDetailView.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 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.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: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/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2025-08-06T14:25:20.438Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2354
File: apps/webapp/app/presenters/v3/RegionsPresenter.server.ts:117-131
Timestamp: 2025-08-06T14:25:20.438Z
Learning: In RegionsPresenter.server.ts, the final sort by name that removes the "default first" ordering is intentional behavior as a way to prefer the default, as clarified by matt-aitken.
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 `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
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 the `task()` function from `trigger.dev/sdk/v3` to define tasks with id and run properties
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 `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
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/v3/LogsListPresenter.server.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to **/*.{ts,tsx} : Always import tasks from `trigger.dev/sdk`, never use `trigger.dev/sdk/v3` or deprecated `client.defineJob` pattern
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 `schedules.task()` for scheduled/cron tasks instead of regular `task()`
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} : Subscribe to run updates using `runs.subscribeToRun()` for realtime monitoring of task execution
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 packages/trigger-sdk/**/*.{ts,tsx} : In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
🧬 Code graph analysis (2)
apps/webapp/app/components/logs/LogDetailView.tsx (3)
apps/webapp/app/utils/pathBuilder.ts (1)
v3RunSpanPath(322-333)apps/webapp/app/utils/logUtils.ts (1)
getLevelColor(98-111)apps/webapp/app/components/primitives/Buttons.tsx (1)
Button(296-329)
apps/webapp/app/components/logs/LogsTable.tsx (3)
apps/webapp/app/components/primitives/Table.tsx (4)
Table(73-102)TableHeader(109-125)TableRow(149-167)TableHeaderCell(181-233)apps/webapp/app/components/LogLevelTooltipInfo.tsx (1)
LogLevelTooltipInfo(6-63)apps/webapp/app/utils/cn.ts (1)
cn(77-79)
⏰ 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). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (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 (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 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 (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 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 (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: Analyze (javascript-typescript)
🔇 Additional comments (13)
apps/webapp/app/components/logs/LogsTable.tsx (4)
42-55: LGTM! Box-shadow approach for level highlighting.Using
box-shadowfor the level indicator is a good performance optimization overborder-left, as it avoids layout recalculations during scrolling. The color mappings align with theLogLevelTooltipInfocomponent (blue for INFO, yellow/amber for WARN, red for ERROR).
122-127: LGTM! Level header tooltip integration.The tooltip prop usage aligns with the
TableHeaderCellcomponent API and provides helpful context about log levels directly in the table header.
190-194: LGTM! Run link opens in new tab with proper security attributes.The
rel="noopener noreferrer"is correctly applied for external navigation security.
204-222: LGTM! Improved infinite scroll UX.The increased padding (
py-12) provides a better trigger area for intersection observer, and theinvisibleclass approach preserves layout while hiding the spinner. The "Showing all N logs" message provides good feedback.apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (5)
75-86: Cursor pagination may skip logs with same-second timestamps.The cursor uses second-precision (
toUnixTimestamp) which can cause pagination to skip logs when multiple entries share the same second andtrace_id. This issue was flagged in a previous review and remains unaddressed.Consider using millisecond precision and adding
span_idas a stable tiebreaker:🐛 Proposed fix
type LogCursor = { environmentId: string; - unixTimestamp: number; + unixTimestampMs: number; traceId: string; + spanId: string; }; const LogCursorSchema = z.object({ environmentId: z.string(), - unixTimestamp: z.number(), + unixTimestampMs: z.number(), traceId: z.string(), + spanId: z.string(), });The query builder and cursor encoding at lines 363-370 and 391-397 would also need updates to use
toUnixTimestamp64Milliand includespan_id.[duplicate_comment, raise_major_issue]
344-347: Debug filter incomplete:LOG_DEBUGnot excluded.When
includeDebugLogs === false, onlyDEBUG_EVENTis filtered out, butlevelToKindsAndStatuses()at line 110 shows that DEBUG level includes bothDEBUG_EVENTandLOG_DEBUG. This allowsLOG_DEBUGentries to remain visible when they should be hidden.🐛 Proposed fix
if (includeDebugLogs === false) { queryBuilder.where("kind NOT IN {debugKinds: Array(String)}", { - debugKinds: ["DEBUG_EVENT"], + debugKinds: ["DEBUG_EVENT", "LOG_DEBUG"], }); }[duplicate_comment, raise_major_issue]
183-192: LGTM! Retention limit clamping logic.The retention limit correctly clamps
effectiveFromto the cutoff date and tracks whether clamping occurred for UI feedback.
306-341: Level filtering logic may incorrectly exclude ERROR-level logs.The code adds
status NOT IN {excluded_statuses: Array(String)}with["ERROR", "CANCELLED"]to every kind condition (lines 318-320). However, for the ERROR level,levelToKindsAndStatuses("ERROR")returns{ kinds: ["LOG_ERROR"], statuses: ["ERROR"] }(line 116).The issue: when filtering for ERROR level, the kind condition
kind IN ["LOG_ERROR"] AND status NOT IN ["ERROR", "CANCELLED"]would exclude logs withstatus = "ERROR", while the separatestatus IN ["ERROR"]condition (lines 327-330) would include them. These are OR'd together (line 334), so ERROR logs are still included.However, for non-ERROR levels like INFO or WARN, this unnecessarily excludes logs that happen to have
status = "ERROR"or"CANCELLED". If this is intentional (to prevent ERROR/CANCELLED status logs from appearing under INFO/WARN levels), consider adding a comment explaining this behavior.
461-464: LGTM! Retention info in response.The conditional retention object provides the UI with both the limit and whether clamping was applied, enabling appropriate user feedback.
apps/webapp/app/components/logs/LogDetailView.tsx (4)
24-24: Narrowed log utils import looks good.
Keeps the dependency surface tight and matches the simplified level handling.
108-116: Keyboard shortcut guard is solid.
Nice UX win to avoid global shortcuts while typing in inputs.Also applies to: 124-124
153-161: Header simplification is clean and consistent.
Level badge styling and layout tweaks look good.
188-189: “View full run” action styling + shortcut looks good.
Minimal variant fits the header and the shortcut is discoverable.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Flagged the ui for Download Logs Solved issue with infinite fetching when the table is scrolled to the end Fixed logs query order by
Flagged the ui for Download Logs Solved issue with infinite fetching when the table is scrolled to the end Fixed logs query order by
|
|
||
| kindCondition += ` AND status NOT IN {excluded_statuses: Array(String)}`; | ||
| params["excluded_statuses"] = ["ERROR", "CANCELLED"]; |
There was a problem hiding this comment.
🔴 Level filter for ERROR incorrectly excludes ERROR status logs
When filtering for ERROR level logs, the query incorrectly excludes logs with status = 'ERROR' due to a blanket exclusion applied to all level filters.
Click to expand
How it gets triggered
When a user selects the ERROR level filter from the logs page UI.
The bug
At LogsListPresenter.server.ts:318-320, the code unconditionally adds:
kindCondition += ` AND status NOT IN {excluded_statuses: Array(String)}`;
params["excluded_statuses"] = ["ERROR", "CANCELLED"];This is applied to ALL levels including ERROR. But for ERROR level, levelToKindsAndStatuses at line 116 returns:
case "ERROR":
return { kinds: ["LOG_ERROR"], statuses: ["ERROR"] };The resulting condition becomes:
(kind IN ['LOG_ERROR'] AND status NOT IN ['ERROR', 'CANCELLED']) OR (status IN ['ERROR'])The first part kind IN ['LOG_ERROR'] AND status NOT IN ['ERROR', 'CANCELLED'] would exclude LOG_ERROR events that have status=ERROR.
Actual vs Expected
- Actual: Error logs with
kind=LOG_ERRORandstatus=ERRORare excluded by the first condition. - Expected: The
excluded_statusesfilter should only apply to non-error levels (INFO, WARN, DEBUG) to prevent showing error traces when filtering for those levels.
Impact
Some ERROR level logs may not appear when filtering by ERROR level, leading to incomplete error visibility.
Recommendation: Only apply the excluded_statuses filter to non-ERROR levels:
if (filter.kinds && filter.kinds.length > 0) {
const kindsKey = `kinds_${level}`;
let kindCondition = `kind IN {${kindsKey}: Array(String)}`;
// Only exclude ERROR/CANCELLED status for non-error levels
if (level !== "ERROR") {
kindCondition += ` AND status NOT IN {excluded_statuses: Array(String)}`;
params["excluded_statuses"] = ["ERROR", "CANCELLED"];
}
levelConditions.push(kindCondition);
params[kindsKey] = filter.kinds;
}
Was this helpful? React with 👍 or 👎 to provide feedback.
escaped "/" character in search for logs fixed cursor ordering
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }; | ||
|
|
||
| function escapeClickHouseString(val: string): string { | ||
| return val.replace(/\//g, "\\/"); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 11 hours ago
In general, when writing a custom escaping/encoding function, you must ensure that all meta-characters that can affect the target syntax are escaped consistently, including the backslash itself. If you escape / by inserting a backslash, you also need to ensure backslashes in the input are escaped so they cannot form unintended escape sequences when combined with characters that follow.
For this specific function, the minimal fix that preserves existing behavior for / while making the escaping self-consistent is:
- First escape all existing backslashes by doubling them (
\→\\). - Then escape forward slashes (
/→\/).
The order matters: escaping / first would introduce new backslashes that you might then re-escape if you subsequently replace backslashes, changing current behavior. Doing backslashes first ensures that any existing backslash cannot escape the / you add, and leaves the /-escaping behavior (for inputs without backslashes) unchanged except for making the output safer when a backslash precedes a slash.
Concretely:
- Edit
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts. - In
escapeClickHouseString, change the implementation to:- Call
val.replace(/\\/g, "\\\\")to escape all backslashes. - Then call
.replace(/\//g, "\\/")on the result to escape all slashes.
- Call
- No new imports or helper methods are required.
| @@ -28,7 +28,7 @@ | ||
| }; | ||
|
|
||
| function escapeClickHouseString(val: string): string { | ||
| return val.replace(/\//g, "\\/"); | ||
| return val.replace(/\\/g, "\\\\").replace(/\//g, "\\/"); | ||
| } | ||
|
|
||
|
|
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 30-32: The escapeClickHouseString function currently only escapes
forward slashes causing potential ClickHouse injection via unescaped
backslashes; update escapeClickHouseString to first escape backslashes (replace
"\" with "\\") and then escape forward slashes (replace "/" with "\/") so
backslashes aren’t consumed by subsequent replacements; locate and modify the
escapeClickHouseString function to perform these replacements in that order (and
ensure the function still returns a string).
🧹 Nitpick comments (2)
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (2)
359-361: Remove commented-out code.These lines appear to be development leftovers and should be removed for code cleanliness.
♻️ Proposed fix
- // kindCondition += ` `; - // params["excluded_statuses"] = ["SPAN", "ANCESTOR_OVERRIDE", "SPAN_EVENT"]; -
323-326: Clarify level filtering logic for ERROR level.The
excluded_statusesfilter is applied to all kind conditions, but for ERROR level this makes the kind condition (kind IN ["LOG_ERROR"] AND status NOT IN ["ERROR"]) effectively dead code since ERROR status logs would fail this condition. They're only captured by the separatestatus IN ["ERROR"]condition.While functionally correct, this logic is confusing. Consider conditionally applying the status exclusion only to non-ERROR levels.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/Shortcuts.tsxapps/webapp/app/presenters/v3/LogsListPresenter.server.ts
🧰 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
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/Shortcuts.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/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/Shortcuts.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/Shortcuts.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/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/Shortcuts.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 webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/Shortcuts.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
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/components/Shortcuts.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/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/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2025-08-06T14:25:20.438Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2354
File: apps/webapp/app/presenters/v3/RegionsPresenter.server.ts:117-131
Timestamp: 2025-08-06T14:25:20.438Z
Learning: In RegionsPresenter.server.ts, the final sort by name that removes the "default first" ordering is intentional behavior as a way to prefer the default, as clarified by matt-aitken.
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 `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
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 the `task()` function from `trigger.dev/sdk/v3` to define tasks with id and run properties
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 `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
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/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 logger methods (debug, log, info, warn, error) from `trigger.dev/sdk/v3` for structured logging in tasks
Applied to files:
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to **/*.{ts,tsx} : Always import tasks from `trigger.dev/sdk`, never use `trigger.dev/sdk/v3` or deprecated `client.defineJob` pattern
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 `schedules.task()` for scheduled/cron tasks instead of regular `task()`
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} : Subscribe to run updates using `runs.subscribeToRun()` for realtime monitoring of task execution
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 packages/trigger-sdk/**/*.{ts,tsx} : In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
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/presenters/v3/LogsListPresenter.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/components/Shortcuts.tsx (3)
apps/webapp/app/hooks/useShortcutKeys.tsx (1)
Shortcut(7-12)apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
ShortcutKey(38-55)apps/webapp/app/components/primitives/Paragraph.tsx (1)
Paragraph(88-107)
🪛 GitHub Check: CodeQL
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
[failure] 31-31: Incomplete string escaping or encoding
This does not escape backslash characters in the input.
⏰ 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). (21)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- 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 (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 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 (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 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 (7, 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 (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- 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 - npm)
🔇 Additional comments (2)
apps/webapp/app/components/Shortcuts.tsx (1)
164-194: LGTM!The new "Logs page" shortcuts section is well-structured and follows the established patterns in this file. The key assignments are appropriate for page-specific context, and the 1-4 range for log level selection aligns with the simplified log levels (DEBUG, INFO, WARN, ERROR) mentioned in the PR objectives.
apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (1)
368-378: Pagination ordering is now consistent.The sort order uses DESC for all columns and the cursor comparison correctly uses
<for forward pagination through descending results. This addresses the previously noted inconsistency.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| function escapeClickHouseString(val: string): string { | ||
| return val.replace(/\//g, "\\/"); | ||
| } |
There was a problem hiding this comment.
Incomplete string escaping allows potential ClickHouse injection.
The function escapes forward slashes but not backslashes. In ClickHouse LIKE patterns, backslash is the escape character. A malicious or unexpected input containing backslashes could alter query behavior or cause errors.
🐛 Proposed fix
function escapeClickHouseString(val: string): string {
- return val.replace(/\//g, "\\/");
+ // Escape backslash first, then forward slash
+ return val.replace(/\\/g, "\\\\").replace(/\//g, "\\/");
}Based on static analysis hint from CodeQL.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function escapeClickHouseString(val: string): string { | |
| return val.replace(/\//g, "\\/"); | |
| } | |
| function escapeClickHouseString(val: string): string { | |
| // Escape backslash first, then forward slash | |
| return val.replace(/\\/g, "\\\\").replace(/\//g, "\\/"); | |
| } |
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 31-31: Incomplete string escaping or encoding
This does not escape backslash characters in the input.
🤖 Prompt for AI Agents
In `@apps/webapp/app/presenters/v3/LogsListPresenter.server.ts` around lines 30 -
32, The escapeClickHouseString function currently only escapes forward slashes
causing potential ClickHouse injection via unescaped backslashes; update
escapeClickHouseString to first escape backslashes (replace "\" with "\\") and
then escape forward slashes (replace "/" with "\/") so backslashes aren’t
consumed by subsequent replacements; locate and modify the
escapeClickHouseString function to perform these replacements in that order (and
ensure the function still returns a string).
| function escapeClickHouseString(val: string): string { | ||
| return val.replace(/\//g, "\\/"); | ||
| } |
There was a problem hiding this comment.
🟡 Incomplete escaping of LIKE pattern wildcards in search term allows unexpected matching
The escapeClickHouseString function in the LogsListPresenter only escapes forward slashes (/), but fails to escape ClickHouse LIKE pattern wildcards % and _.
Click to expand
The problematic code
At apps/webapp/app/presenters/v3/LogsListPresenter.server.ts:30-32:
function escapeClickHouseString(val: string): string {
return val.replace(/\//g, "\\/");
}This escaped string is then used in a LIKE pattern at lines 302-307:
const searchTerm = escapeClickHouseString(search.trim()).toLowerCase();
queryBuilder.where(
"(lower(message) like {searchPattern: String} OR lower(attributes_text) like {searchPattern: String})",
{
searchPattern: `%${searchTerm}%`
}
);Actual vs Expected Behavior
Actual: If a user searches for test%data or test_data, the % is interpreted as "match any characters" and _ as "match any single character", returning far more results than intended.
Expected: The search should find logs containing the literal strings test%data or test_data.
Impact
Users cannot search for log messages containing % or _ characters literally. These searches will return incorrect/unexpected results.
Recommendation: Escape LIKE wildcards by replacing % with \% and _ with \_:
function escapeClickHouseString(val: string): string {
return val
.replace(/\\/g, '\\\\') // escape backslash first
.replace(/%/g, '\\%')
.replace(/_/g, '\\_')
.replace(/\//g, '\\/');
}
Was this helpful? React with 👍 or 👎 to provide feedback.
✨ Changes
UI & UX
Filtering & Search
Backend
014_add_task_runs_v2_search_indexes.sql)