-
-
Notifications
You must be signed in to change notification settings - Fork 957
fix(webapp): reconcile trace with run lifecycle to handle ClickHouse lag #2875
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(webapp): reconcile trace with run lifecycle to handle ClickHouse lag #2875
Conversation
⚠️ No Changeset found
Latest commit: 960b23f
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
WalkthroughThe PR adds a trace reconciliation module and integrates it into the run presenter to derive root-span status, events, and duration from run lifecycle data. The RunPresenter now includes run.createdAt in its payload and delegates root-span reconciliation to the new reconcileTraceWithRunLifecycle function. Throttle utility behavior is changed to execute immediately on first call and coalesce the last call per window. Tests for the reconciliation logic are added and Vitest config expanded to discover tests under app/**. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Summary of changesapps/webapp/app/presenters/v3/RunPresenter.server.ts
apps/webapp/app/presenters/v3/reconcileTrace.server.ts (new file)
apps/webapp/app/presenters/v3/reconcileTrace.server.test.ts (new file)
apps/webapp/app/utils/throttle.ts
apps/webapp/vitest.config.ts
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/webapp/app/presenters/v3/RunPresenter.server.ts:
- Around line 288-294: The currentStatus calculation incorrectly assumes the
root span is events[0]; instead, use the actual root span ID
(traceSummary.rootSpan.id or the existing rootSpanId variable) to locate the
root event in the events array and derive the status from that event's
data.isError / data.isPartial fields; update the logic that sets currentStatus
to find the event whose spanId equals rootSpanId (similar to the reconciliation
logic around line ~310) and base the "failed"|"completed"|"executing" result on
that found event rather than events[0].
🧹 Nitpick comments (2)
apps/webapp/test/RunPresenter.test.ts (2)
1-28: Consider test file location and mock usage.Per coding guidelines, test files should be placed next to source files with
.test.tsnaming. This file should be atapps/webapp/app/presenters/v3/RunPresenter.test.ts.The mocks work around indirect imports of
env.server.ts, but the guidelines suggest "pass configuration through options instead". Consider extractingreconcileTraceWithRunLifecycleto a separate pure utility file (e.g.,reconcileTrace.ts) without dependencies, which would eliminate the need for mocks entirely.
35-41: Prefer proper types overany.Using
anyreduces type safety. Consider importing theRuntype and creating properly typed test data:import type { Run, RunEvent } from "../app/presenters/v3/RunPresenter.server"; const runData: Parameters<typeof reconcileTraceWithRunLifecycle>[0] = { isFinished: true, status: "COMPLETED_SUCCESSFULLY" as Run["status"], createdAt, completedAt, rootTaskRun: null, };
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/test/RunPresenter.test.ts
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.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/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
Files:
apps/webapp/test/RunPresenter.test.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Test files should only import classes and functions from
app/**/*.tsfiles and should not importenv.server.tsdirectly or indirectly; pass configuration through options instead
Files:
apps/webapp/test/RunPresenter.test.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx,js,jsx}: Test files should live beside the files under test and use descriptivedescribeanditblocks
Avoid mocks or stubs in tests; use helpers from@internal/testcontainerswhen Redis or Postgres are needed
Use vitest for unit tests
Files:
apps/webapp/test/RunPresenter.test.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/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use vitest exclusively for testing and never mock anything; use testcontainers instead
Files:
apps/webapp/test/RunPresenter.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place test files next to source files with .test.ts naming convention (e.g., MyService.ts → MyService.test.ts)
Use testcontainers helpers (redisTest, postgresTest, containerTest from @internal/testcontainers) for Redis/PostgreSQL testing instead of mocks
Files:
apps/webapp/test/RunPresenter.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025年07月12日T18: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.
📚 Learning: 2025年11月27日T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025年11月27日T16:26:37.432Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Use vitest for all tests in the Trigger.dev repository
Applied to files:
apps/webapp/test/RunPresenter.test.ts
📚 Learning: 2025年07月12日T18: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月12日T18: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/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.ts
🧬 Code graph analysis (1)
apps/webapp/test/RunPresenter.test.ts (3)
packages/core/src/v3/index.ts (1)
millisecondsToNanoseconds(38-38)packages/core/src/v3/workers/taskExecutor.ts (1)
result(1285-1332)apps/webapp/app/presenters/v3/RunPresenter.server.ts (1)
reconcileTraceWithRunLifecycle(272-329)
🔇 Additional comments (5)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (4)
9-9: LGTM!The import addition for
isFailedRunStatusis correct and necessary for the new reconciliation function to properly detect failed run states.
214-236: LGTM!The formatting changes improve readability while maintaining the existing logic.
242-254: LGTM!The reconciliation function is cleanly integrated into the presenter flow, and the reconciled values are properly used for the trace output.
267-329: Well-documented reconciliation logic for ClickHouse lag.The function correctly addresses the issue where ClickHouse telemetry can lag behind Postgres run status. The implementation properly:
- Uses
Math.maxto preserve longer durations from ClickHouse when available- Only modifies partial root spans (completed spans are trusted)
- Correctly propagates error status for failed runs
Based on learnings, this UI-layer workaround for ClickHouse data delays is consistent with existing patterns in the codebase.
apps/webapp/test/RunPresenter.test.ts (1)
62-140: Comprehensive test coverage.The test cases effectively cover the key scenarios:
- Lagging partial telemetry reconciliation
- Preserving longer ClickHouse durations
- Unfinished run passthrough
- Failed run status propagation
rootTaskRun.createdAtusage for duration calculation- Missing root span handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
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/RunPresenter.server.ts (1)
117-138: MissingcreatedAtinrunDataobject.The
reconcileTraceWithRunLifecyclefunction expectscreatedAtin therunDataparameter (line 276), but therunDataobject constructed at lines 117-138 does not include it. The function usesrunData.createdAtat line 304 as a fallback whenrootTaskRun?.createdAtis not available.🐛 Proposed fix to add createdAt to runData
const runData = { id: run.id, + createdAt: run.createdAt, number: run.number, friendlyId: run.friendlyId, traceId: run.traceId, spanId: run.spanId, status: run.status, isFinished: isFinalRunStatus(run.status),Also applies to: 242-247
🧹 Nitpick comments (1)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (1)
267-330: Well-designed reconciliation logic.The function correctly handles the eventual consistency issue between ClickHouse and Postgres:
- Early return for unfinished runs preserves real-time telemetry data
- Uses Postgres timing as source of truth for finished runs
- Only updates partial spans to avoid overwriting valid ClickHouse data
- Returns status derived from Postgres regardless of event state
The documentation comment clearly explains the rationale for this UI-layer tradeoff. Based on learnings, this pattern aligns with the project's approach of handling ClickHouse delays through in-memory reconciliation.
Consider extracting the status type for reusability:
♻️ Optional: Extract RootSpanStatus type
+type RootSpanStatus = "executing" | "completed" | "failed"; + export function reconcileTraceWithRunLifecycle( runData: { isFinished: boolean; status: Run["status"]; createdAt: Date; completedAt: Date | null; rootTaskRun: { createdAt: Date } | null; }, rootSpanId: string, events: RunEvent[], totalDuration: number ): { events: RunEvent[]; totalDuration: number; - rootSpanStatus: "executing" | "completed" | "failed"; + rootSpanStatus: RootSpanStatus; } {
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/app/presenters/v3/RunPresenter.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/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/presenters/v3/RunPresenter.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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/presenters/v3/RunPresenter.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/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025年07月12日T18: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.
📚 Learning: 2025年11月27日T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.ts
📚 Learning: 2025年07月12日T18: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月12日T18: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/RunPresenter.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (2)
apps/webapp/app/utils/timelineSpanEvents.ts (1)
createTimelineSpanEventsFromSpanEvents(26-131)apps/webapp/app/v3/taskStatus.ts (1)
isFailedRunStatus(88-90)
🔇 Additional comments (3)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (3)
9-9: LGTM!Import addition is appropriate for the new reconciliation logic.
214-237: LGTM!Event mapping logic correctly calculates offsets, handles debug events, and sets partial node durations to null for proper UI rendering.
249-263: LGTM!Clean integration of reconciled trace data into the response object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a big risk if there are 50k events or possibly more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ericallam — good call.
I’ve refactored this in the latest commits to address that concern:
Removed the events.find lookup entirely.
The root span is now accessed via index 0 (flattened tree guarantees order), making it O(1).
The reconciliation logic is merged into the existing trace construction pass, so there’s no additional iteration even for large event sets.
I also fixed a missing createdAt edge case flagged by CodeRabbit.
Latest commit: 4c3f0e8. Let me know if you’d like me to split anything further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/webapp/app/presenters/v3/RunPresenter.server.ts:
- Around line 298-305: The call to reconcileTraceWithRunLifecycle is missing the
required createdAt field on the runData object; reconcileTraceWithRunLifecycle
reads runData.createdAt when rootTaskRun is null, so update the runData
construction (the object passed into reconcileTraceWithRunLifecycle) to include
createdAt (a Date) taken from the run entity (e.g., run.createdAt or the
appropriate created timestamp) so the function receives a valid Date and avoids
a runtime error.
🧹 Nitpick comments (1)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (1)
222-268: Duplicate reconciliation logic creates maintenance burden.The inline reconciliation at lines 235-239 modifies root span properties (
nIsPartial,nDuration,nIsError) beforereconcileTraceWithRunLifecycleis called. Since the inline code already setsisPartial: falsefor finished runs, the check at line 340 inreconcileTraceWithRunLifecyclewill always be false for these cases, making that branch effectively dead code.Consider either:
- Removing the inline reconciliation (lines 235-239) and relying solely on
reconcileTraceWithRunLifecycle, or- Removing the event mutation from
reconcileTraceWithRunLifecyclesince inline already handles itThis would reduce duplication and make the reconciliation logic easier to maintain. Based on the retrieved learnings, this pattern of UI-layer reconciliation for ClickHouse delays is intentional, so consolidating the logic in one place would help.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/app/utils/throttle.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/app/utils/throttle.tsapps/webapp/app/presenters/v3/RunPresenter.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/utils/throttle.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/utils/throttle.tsapps/webapp/app/presenters/v3/RunPresenter.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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/utils/throttle.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/utils/throttle.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/utils/throttle.tsapps/webapp/app/presenters/v3/RunPresenter.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/utils/throttle.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/app/utils/throttle.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025年07月12日T18: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.
📚 Learning: 2025年11月27日T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.ts
📚 Learning: 2025年07月12日T18: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月12日T18: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/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (2)
packages/core/src/v3/index.ts (1)
millisecondsToNanoseconds(38-38)apps/webapp/app/v3/taskStatus.ts (1)
isFailedRunStatus(88-90)
🔇 Additional comments (2)
apps/webapp/app/utils/throttle.ts (1)
1-28: LGTM! Clean implementation of leading + trailing edge throttle.The logic correctly implements the described behavior: immediate first invocation, coalescing subsequent calls during the window, and firing the last call at the trailing edge. The recursive
wrapped(...argsToUse)call elegantly chains windows when continuous calls occur.One consideration: there's no exposed cancellation mechanism. If the component using this throttle unmounts while a timeout is pending, the callback will still fire. This is typically fine for server-side presenters but worth noting if this utility is used client-side.
apps/webapp/app/presenters/v3/RunPresenter.server.ts (1)
273-280: Usage of reconciled values looks correct.The trace object is properly constructed using the reconciled values. This ensures the UI displays consistent state regardless of ClickHouse lag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (2)
236-240: Duplicated reconciliation logic withreconcileTraceWithRunLifecycle.The root span reconciliation here (lines 236-240) duplicates the logic in
reconcileTraceWithRunLifecycle(lines 341-351). SincereconcileTraceWithRunLifecycleis called at line 274 with the events produced by this map, the root span is processed twice.While the second pass becomes a no-op (because
isPartialis alreadyfalseafter the first pass), this duplication is confusing and may drift over time. Consider removing the inline reconciliation and relying solely onreconcileTraceWithRunLifecycle.♻️ Suggested refactor
- // NOTE: Clickhouse trace ingestion is eventually consistent. - // When a run is marked finished in Postgres, we reconcile the - // root span to reflect completion even if telemetry is still partial. - // This is a deliberate UI-layer tradeoff to prevent stale or "stuck" - // run states in the dashboard. - if (isRoot && runData.isFinished && nIsPartial) { - nIsPartial = false; - nDuration = Math.max(nDuration ?? 0, postgresRunDuration); - nIsError = isFailedRunStatus(runData.status); - } + // Root span reconciliation is handled by reconcileTraceWithRunLifecycle below //only let non-debug events extend the total duration if (!n.data.isDebug) { - totalDuration = Math.max(totalDuration, offset + (nIsPartial ? 0 : nDuration)); + totalDuration = Math.max(totalDuration, offset + (n.data.isPartial ? 0 : n.data.duration)); } return { ...n, data: { ...n.data, timelineEvents: createTimelineSpanEventsFromSpanEvents( n.data.events, user?.admin ?? false, treeRootStartTimeMs ), //set partial nodes to null duration - duration: nIsPartial ? null : nDuration, - isPartial: nIsPartial, - isError: nIsError, + duration: n.data.isPartial ? null : n.data.duration, + isPartial: n.data.isPartial, + isError: n.data.isError, offset, isRoot, }, };
267-269: RedundanttotalDurationupdate.This update is also performed inside
reconcileTraceWithRunLifecycleat line 338. Since the reconciledtotalDurationis what gets returned (line 281), this pre-update is redundant.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/app/presenters/v3/RunPresenter.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/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/presenters/v3/RunPresenter.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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/presenters/v3/RunPresenter.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/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025年07月12日T18: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.
📚 Learning: 2026年01月12日T11:01:34.777Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026年01月12日T11:01:34.777Z
Learning: Applies to internal-packages/database/prisma/migrations/**/*.sql : Edit database schema in internal-packages/database/prisma/schema.prisma; remove extraneous lines from migrations related to BackgroundWorker, TaskRun tags, waitpoints, and SecretStore indexes
Applied to files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
📚 Learning: 2025年07月12日T18: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月12日T18: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/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025年11月27日T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Attach metadata to task runs using the metadata option when triggering, and access/update it inside runs using metadata functions
Applied to files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (3)
packages/core/src/v3/index.ts (1)
millisecondsToNanoseconds(38-38)apps/webapp/app/v3/taskStatus.ts (1)
isFailedRunStatus(88-90)apps/webapp/app/utils/timelineSpanEvents.ts (1)
createTimelineSpanEventsFromSpanEvents(26-131)
🔇 Additional comments (5)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (5)
9-9: LGTM!The import correctly adds
isFailedRunStatuswhich is used in the reconciliation logic to determine if a run has failed.
127-127: LGTM!The
createdAtfield is correctly added torunDatato support accurate duration calculation in the reconciliation logic.
212-219: LGTM!The
postgresRunDurationcalculation correctly computes the run lifetime in nanoseconds, preferringrootTaskRun.createdAtwhen available for accurate duration across child runs.
274-281: LGTM!The reconciliation function is correctly invoked and its return values are properly integrated into the response structure.
299-365: Well-designed reconciliation function.The function is pure, self-contained, and testable. A few observations:
Assumption on root at index 0: Line 315 assumes the root event is always first. This is valid given how
flattenTreeworks, and theisActualRootcheck at line 316 provides defensive validation.Self-contained duration calculation: The function recalculates
postgresRunDurationinternally (lines 331-336) rather than accepting it as a parameter. This is a reasonable trade-off for testability and independence, though it does duplicate the calculation from lines 213-219.Type handling: The
duration ?? 0at line 348 correctly handles the nullable duration from the event data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this throttle change is here now, just curious why you keep adding this to your PRs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the throttle change: It was originally in PR #2874, but after your feedback that it was unrelated to the SDK fix, I've isolated it here. In this PR (#2875), it's one of the two main components of the fix for the dashboard refresh bug (#2798). The original throttle could drop final status updates; this trailing-edge version ensures the completion events are always delivered to the UI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather the reconcileTraceWithRunLifecycle be moved to a different file that doesn't need all these mocks and makes testing it a bit easier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ericallam, great suggestion! I've moved the reconciliation logic to its own utility file reconcileTrace.server.ts. This allowed me to simplify the tests significantly by removing the heavy presenter/repository mocks. All tests are passing independently now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
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/RunPresenter.server.ts (1)
1-11: Missing import fromreconcileTrace.server.ts— duplicate function exists.The commit message indicates reconciliation logic was moved to a separate file for testability, and
reconcileTrace.server.tsexportsreconcileTraceWithRunLifecycle. However, this file defines the same function inline (lines 272-330) without importing from the extracted module.This creates:
- Code duplication — two implementations to maintain
- Behavioral divergence — the extracted module uses
events[0]while this inline version usesevents.find()- Test coverage gap — tests may cover the extracted module while production uses this inline version
Please import from the extracted module instead:
import { isFailedRunStatus, isFinalRunStatus } from "~/v3/taskStatus"; +import { reconcileTraceWithRunLifecycle } from "./reconcileTrace.server"; import { env } from "~/env.server";Then remove the inline function definition at lines 266-330.
🤖 Fix all issues with AI agents
In @apps/webapp/app/presenters/v3/RunPresenter.server.ts:
- Around line 266-330: Remove the duplicate inline function and import the
canonical implementation from reconcileTrace.server.ts: update
reconcileTrace.server.ts to use this exact logic (use events.find to locate
rootEvent, compute currentStatus, calculate postgresRunDuration with
runData.completedAt and runData.rootTaskRun?.createdAt, compute
updatedTotalDuration, map events to clear isPartial and set duration/isError for
the root span, and set rootSpanStatus based on isFailedRunStatus), export
reconcileTraceWithRunLifecycle from that module, then replace the inline
reconcileTraceWithRunLifecycle in RunPresenter.server.ts with an import and
re-export/call to the shared function so there is a single authoritative
implementation.
🧹 Nitpick comments (1)
apps/webapp/app/presenters/v3/reconcileTrace.server.ts (1)
22-26: Consider using stronger typing foreventsarray.The
any[]type loses type safety. SinceReconcileEventis already defined, consider using it:export type ReconcileResult = { - events: any[]; + events: ReconcileEvent[]; totalDuration: number; rootSpanStatus: "executing" | "completed" | "failed"; };Similarly, the function parameter at line 36 could use
ReconcileEvent[]instead ofany[].
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.server.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.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/presenters/v3/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.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/presenters/v3/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.server.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.server.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025年07月12日T18: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.
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16: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
📚 Learning: 2025年11月27日T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.server.ts
📚 Learning: 2025年07月12日T18: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月12日T18: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/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025年11月27日T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure OpenTelemetry instrumentations and exporters in trigger.config.ts for enhanced logging
Applied to files:
apps/webapp/app/presenters/v3/reconcileTrace.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (3)
packages/core/src/v3/index.ts (1)
millisecondsToNanoseconds(38-38)apps/webapp/app/presenters/v3/reconcileTrace.server.ts (1)
reconcileTraceWithRunLifecycle(33-89)apps/webapp/app/v3/taskStatus.ts (1)
isFailedRunStatus(88-90)
🔇 Additional comments (4)
apps/webapp/app/presenters/v3/reconcileTrace.server.ts (2)
33-89: Function logic looks sound for its intended purpose.The reconciliation correctly handles:
- Early return when run isn't finished (line 51-53)
- Duration calculation using Postgres timestamps with appropriate fallbacks
- Updating partial root spans to completed state with correct duration
- Deriving
rootSpanStatusfrom the authoritative Postgres statusThe approach of preferring
Math.maxfor durations is a good defensive measure.
38-49: Implementations use different patterns, but both are safe since flattenTree guarantees root is first.The function assumes
events[0]is the root span and validates withisActualRoot. While this works (theflattenTreefunction does return root as the first element via depth-first traversal), it differs from the implementation inRunPresenter.server.ts(lines 288-323), which usesevents.find((e) => e.id === rootSpanId)andevents.map().For consistency with the other presenter and better defensive coding, consider aligning the patterns.
apps/webapp/app/presenters/v3/RunPresenter.server.ts (2)
212-237: Event mapping logic is well-structured.The offset calculation and duration handling correctly:
- Computes offsets relative to the tree root's start time
- Excludes debug events from extending total duration (prevents debugging artifacts from skewing the timeline)
- Sets partial node durations to
nullfor proper UI indication of in-progress spans
242-254: Integration with reconciliation function is correct.The call passes all required data and properly uses the reconciled results for the returned trace object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/webapp/app/presenters/v3/RunPresenter.server.ts:
- Around line 228-232: The inline reconciliation block that references the
undefined postgresRunDuration (the if block checking isRoot &&
runData.isFinished && nIsPartial and setting nIsPartial, nDuration, nIsError)
must be removed to avoid the ReferenceError and duplication; rely on the
existing reconcileTraceWithRunLifecycle(rootSpan, runData) call (the function
that updates isPartial, duration, and isError for the root span) instead, so
delete that entire inline block and ensure no other code attempts to read
postgresRunDuration in RunPresenter.server.ts.
🧹 Nitpick comments (1)
apps/webapp/test/RunPresenter.test.ts (1)
1-28: Test file location does not follow project conventions.Per coding guidelines, test files should be placed next to source files with
.test.tsnaming convention. This file should be atapps/webapp/app/presenters/v3/reconcileTrace.server.test.tsinstead ofapps/webapp/test/RunPresenter.test.ts.Additionally, the coding guidelines prefer avoiding mocks/stubs in tests. Since
reconcileTraceWithRunLifecycleis a pure function that doesn't require database or external services, consider restructuring the imports in the source module to make the function testable without these mocks.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/test/RunPresenter.test.ts
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.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/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
Files:
apps/webapp/test/RunPresenter.test.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Test files should only import classes and functions from
app/**/*.tsfiles and should not importenv.server.tsdirectly or indirectly; pass configuration through options instead
Files:
apps/webapp/test/RunPresenter.test.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx,js,jsx}: Test files should live beside the files under test and use descriptivedescribeanditblocks
Avoid mocks or stubs in tests; use helpers from@internal/testcontainerswhen Redis or Postgres are needed
Use vitest for unit tests
Files:
apps/webapp/test/RunPresenter.test.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/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use vitest exclusively for testing and never mock anything; use testcontainers instead
Files:
apps/webapp/test/RunPresenter.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place test files next to source files with .test.ts naming convention (e.g., MyService.ts → MyService.test.ts)
Use testcontainers helpers (redisTest, postgresTest, containerTest from @internal/testcontainers) for Redis/PostgreSQL testing instead of mocks
Files:
apps/webapp/test/RunPresenter.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025年07月12日T18: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.
📚 Learning: 2025年11月27日T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025年11月27日T16:26:37.432Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Use vitest for all tests in the Trigger.dev repository
Applied to files:
apps/webapp/test/RunPresenter.test.ts
📚 Learning: 2025年07月12日T18: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月12日T18: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/test/RunPresenter.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.ts
🧬 Code graph analysis (2)
apps/webapp/test/RunPresenter.test.ts (3)
packages/core/src/v3/index.ts (1)
millisecondsToNanoseconds(38-38)packages/core/src/v3/workers/taskExecutor.ts (1)
result(1285-1332)apps/webapp/app/presenters/v3/reconcileTrace.server.ts (1)
reconcileTraceWithRunLifecycle(33-89)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (4)
packages/core/src/v3/index.ts (1)
millisecondsToNanoseconds(38-38)apps/webapp/app/v3/taskStatus.ts (1)
isFailedRunStatus(88-90)apps/webapp/app/utils/timelineSpanEvents.ts (1)
createTimelineSpanEventsFromSpanEvents(26-131)apps/webapp/app/presenters/v3/reconcileTrace.server.ts (1)
reconcileTraceWithRunLifecycle(33-89)
🔇 Additional comments (4)
apps/webapp/test/RunPresenter.test.ts (1)
30-141: Good test coverage for reconciliation scenarios.The test suite covers the key scenarios well: lagging telemetry, already-completed traces, unfinished runs, failed runs, rootTaskRun duration calculation, and missing root spans. The assertions correctly verify the expected behavior of the
reconcileTraceWithRunLifecyclefunction.apps/webapp/app/presenters/v3/RunPresenter.server.ts (3)
9-11: Imports look correct.The imports for
isFailedRunStatus,isFinalRunStatusfrom~/v3/taskStatusandreconcileTraceWithRunLifecyclefrom the local module follow the project conventions.
128-128: LGTM!Adding
createdAttorunDatais necessary for the reconciliation logic to compute the Postgres-based run duration.
262-274: Reconciliation integration looks correct.The call to
reconcileTraceWithRunLifecycleproperly passes the required data and uses the returned reconciled values (rootSpanStatus,events,totalDuration) in the response. This cleanly delegates the ClickHouse lag reconciliation logic to the extracted module.
...onventional location
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/webapp/app/presenters/v3/RunPresenter.server.ts:
- Line 9: The import line in RunPresenter.server.ts includes an unused symbol
isFailedRunStatus; remove isFailedRunStatus from the import (keep
isFinalRunStatus) to eliminate the unused import, or if the intent was to use it
here instead of in reconcileTrace.server.ts then add the appropriate usage of
isFailedRunStatus in the RunPresenter logic where failure checks are needed;
update the import to only import the actually used symbols to resolve the lint
warning.
🧹 Nitpick comments (2)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (1)
219-227: Consider simplifying: intermediate variables are assigned but never modified.The variables
nIsPartial,nDuration, andnIsErrorare assigned fromn.databut never modified before use. You could usen.data.isPartial,n.data.duration, andn.data.isErrordirectly, or inline them in the return statement.However, if these are intentionally kept for future extensibility or readability, that's acceptable.
Optional simplification
- let nIsPartial = n.data.isPartial; - let nDuration = n.data.duration; - let nIsError = n.data.isError; - - //only let non-debug events extend the total duration if (!n.data.isDebug) { - totalDuration = Math.max(totalDuration, offset + (nIsPartial ? 0 : nDuration)); + totalDuration = Math.max(totalDuration, offset + (n.data.isPartial ? 0 : n.data.duration)); } return { ...n, data: { ...n.data, timelineEvents: createTimelineSpanEventsFromSpanEvents( n.data.events, user?.admin ?? false, treeRootStartTimeMs ), //set partial nodes to null duration - duration: nIsPartial ? null : nDuration, - isPartial: nIsPartial, - isError: nIsError, + duration: n.data.isPartial ? null : n.data.duration, + isPartial: n.data.isPartial, + isError: n.data.isError, offset, isRoot, }, };apps/webapp/app/presenters/v3/reconcileTrace.server.test.ts (1)
10-16: Consider using the exportedReconcileRunDatatype instead ofany.The
ReconcileRunDatatype is exported from the module under test. Using it here would provide compile-time validation and catch breaking changes early.♻️ Suggested improvement
+import { reconcileTraceWithRunLifecycle, ReconcileRunData } from "./reconcileTrace.server"; -import { reconcileTraceWithRunLifecycle } from "./reconcileTrace.server";- const runData: any = { + const runData: ReconcileRunData = { isFinished: true, status: "COMPLETED_SUCCESSFULLY", createdAt, completedAt, rootTaskRun: null, };
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/presenters/v3/RunPresenter.server.tsapps/webapp/app/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.ts
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.tsapps/webapp/app/presenters/v3/RunPresenter.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/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Test files should only import classes and functions from
app/**/*.tsfiles and should not importenv.server.tsdirectly or indirectly; pass configuration through options instead
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx,js,jsx}: Test files should live beside the files under test and use descriptivedescribeanditblocks
Avoid mocks or stubs in tests; use helpers from@internal/testcontainerswhen Redis or Postgres are needed
Use vitest for unit tests
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.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/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use vitest exclusively for testing and never mock anything; use testcontainers instead
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place test files next to source files with .test.ts naming convention (e.g., MyService.ts → MyService.test.ts)
Use testcontainers helpers (redisTest, postgresTest, containerTest from @internal/testcontainers) for Redis/PostgreSQL testing instead of mocks
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
apps/webapp/app/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In testable code, never import env.server.ts in test files; pass configuration as options instead
Files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025年07月12日T18: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.
📚 Learning: 2025年11月27日T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025年11月27日T16:26:37.432Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Use vitest for all tests in the Trigger.dev repository
Applied to files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.ts
📚 Learning: 2025年11月27日T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16:26:58.661Z
Learning: Applies to apps/webapp/app/services/**/*.server.{ts,tsx} : Separate testable services from configuration files; follow the pattern of `realtimeClient.server.ts` (testable service) and `realtimeClientGlobal.server.ts` (configuration) in the webapp
Applied to files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.ts
📚 Learning: 2025年11月27日T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.test.{ts,tsx} : Test files should only import classes and functions from `app/**/*.ts` files and should not import `env.server.ts` directly or indirectly; pass configuration through options instead
Applied to files:
apps/webapp/app/presenters/v3/reconcileTrace.server.test.tsapps/webapp/vitest.config.ts
📚 Learning: 2025年07月12日T18: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月12日T18: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/reconcileTrace.server.test.tsapps/webapp/app/presenters/v3/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:27:48.109Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025年11月27日T16:27:48.109Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vitest for unit tests
Applied to files:
apps/webapp/vitest.config.ts
📚 Learning: 2026年01月12日T11:01:34.777Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026年01月12日T11:01:34.777Z
Learning: Applies to **/*.test.{ts,tsx,js} : Use vitest exclusively for testing and never mock anything; use testcontainers instead
Applied to files:
apps/webapp/vitest.config.ts
📚 Learning: 2026年01月12日T11:01:34.777Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026年01月12日T11:01:34.777Z
Learning: Applies to **/*.test.{ts,tsx} : Place test files next to source files with .test.ts naming convention (e.g., MyService.ts → MyService.test.ts)
Applied to files:
apps/webapp/vitest.config.ts
📚 Learning: 2026年01月12日T11:01:34.777Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026年01月12日T11:01:34.777Z
Learning: Applies to apps/webapp/app/**/*.test.{ts,tsx} : In testable code, never import env.server.ts in test files; pass configuration as options instead
Applied to files:
apps/webapp/vitest.config.ts
📚 Learning: 2025年11月27日T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16: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/vitest.config.ts
📚 Learning: 2025年11月27日T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025年11月27日T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Specify task locations in trigger.config.ts using the `dirs` array, with automatic exclusion of .test and .spec files
Applied to files:
apps/webapp/vitest.config.ts
📚 Learning: 2026年01月12日T11:01:34.777Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026年01月12日T11:01:34.777Z
Learning: Applies to **/*.test.{ts,tsx} : Use testcontainers helpers (redisTest, postgresTest, containerTest from internal/testcontainers) for Redis/PostgreSQL testing instead of mocks
Applied to files:
apps/webapp/vitest.config.ts
📚 Learning: 2025年11月27日T16:27:48.109Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025年11月27日T16:27:48.109Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Test files should live beside the files under test and use descriptive `describe` and `it` blocks
Applied to files:
apps/webapp/vitest.config.ts
📚 Learning: 2025年11月27日T16:27:48.109Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025年11月27日T16:27:48.109Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocks or stubs in tests; use helpers from `internal/testcontainers` when Redis or Postgres are needed
Applied to files:
apps/webapp/vitest.config.ts
📚 Learning: 2025年11月27日T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025年11月27日T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Attach metadata to task runs using the metadata option when triggering, and access/update it inside runs using metadata functions
Applied to files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.ts
🧬 Code graph analysis (2)
apps/webapp/app/presenters/v3/reconcileTrace.server.test.ts (2)
packages/core/src/v3/index.ts (1)
millisecondsToNanoseconds(38-38)apps/webapp/app/presenters/v3/reconcileTrace.server.ts (1)
reconcileTraceWithRunLifecycle(33-89)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (3)
packages/core/src/v3/index.ts (1)
millisecondsToNanoseconds(38-38)apps/webapp/app/utils/timelineSpanEvents.ts (1)
createTimelineSpanEventsFromSpanEvents(26-131)apps/webapp/app/presenters/v3/reconcileTrace.server.ts (1)
reconcileTraceWithRunLifecycle(33-89)
🔇 Additional comments (6)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (3)
128-128: LGTM!The
createdAtfield is correctly added torunDatato support the reconciliation logic which uses it as a fallback whenrootTaskRun?.createdAtis not available.
252-257: LGTM!The reconciliation integration is well-structured. The function receives all necessary data to reconcile ClickHouse telemetry with the authoritative Postgres run lifecycle, addressing the core objective of handling trace lag. Based on learnings, this pattern of using in-memory adjustments to work around ClickHouse delays is an established approach in this codebase.
259-274: LGTM!The return structure correctly propagates the reconciled trace data, ensuring the UI reflects the corrected root span status, events, and duration when ClickHouse telemetry lags behind the Postgres run lifecycle.
apps/webapp/vitest.config.ts (1)
6-6: LGTM!The expanded include pattern correctly enables test discovery in the
app/directory, aligning with the coding guideline to place test files next to source files. This ensures the newreconcileTrace.server.test.tsand any future co-located tests are picked up by Vitest.apps/webapp/app/presenters/v3/reconcileTrace.server.test.ts (2)
93-115: LGTM!Both test cases correctly verify edge cases:
- Using
rootTaskRun.createdAtfor accurate duration calculation when the root task started earlier than the run.- Graceful handling when the root span is missing from events, still updating
totalDurationbased on Postgres data.
83-91: COMPLETED_WITH_ERRORS is correctly recognized byisFailedRunStatus. The test is valid and requires no changes. The status is included in theFAILED_RUN_STATUSESarray inapps/webapp/app/v3/taskStatus.ts, ensuring the test assertions forrootSpanStatusbeing "failed" andisErrorbeingtrueare correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (1)
219-226: Consider removing unused intermediate variables.The variables
nIsPartial,nDuration, andnIsErrorare assigned fromn.dataproperties but are never modified before use. They could be replaced with direct property access for clarity.♻️ Suggested simplification
const isRoot = index === 0; const offset = millisecondsToNanoseconds(n.data.startTime.getTime() - treeRootStartTimeMs); - let nIsPartial = n.data.isPartial; - let nDuration = n.data.duration; - let nIsError = n.data.isError; - - //only let non-debug events extend the total duration if (!n.data.isDebug) { - totalDuration = Math.max(totalDuration, offset + (nIsPartial ? 0 : nDuration)); + totalDuration = Math.max(totalDuration, offset + (n.data.isPartial ? 0 : n.data.duration)); } return { ...n, data: { ...n.data, timelineEvents: createTimelineSpanEventsFromSpanEvents( n.data.events, user?.admin ?? false, treeRootStartTimeMs ), //set partial nodes to null duration - duration: nIsPartial ? null : nDuration, - isPartial: nIsPartial, - isError: nIsError, + duration: n.data.isPartial ? null : n.data.duration, + isPartial: n.data.isPartial, + isError: n.data.isError, offset, isRoot, }, };
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/app/presenters/v3/RunPresenter.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/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/presenters/v3/RunPresenter.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 webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/presenters/v3/RunPresenter.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/presenters/v3/RunPresenter.server.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025年07月12日T18: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.
📚 Learning: 2025年07月12日T18: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月12日T18: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/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025年11月27日T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Attach metadata to task runs using the metadata option when triggering, and access/update it inside runs using metadata functions
Applied to files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.ts
📚 Learning: 2026年01月12日T11:01:34.777Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026年01月12日T11:01:34.777Z
Learning: Applies to **/*.{ts,tsx} : Every Trigger.dev task must be exported; use task() function with unique id and run async function
Applied to files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
📚 Learning: 2025年11月27日T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025年11月27日T16: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/RunPresenter.server.ts
🔇 Additional comments (4)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (4)
11-11: LGTM!The import follows the Remix server-side module naming convention and is appropriately scoped.
128-128: LGTM!Adding
createdAttorunDatais necessary for the reconciliation logic to calculate the full run lifetime from Postgres.
252-257: LGTM!The reconciliation is correctly integrated after event processing, using the root span ID from the trace summary and passing all necessary data for lifecycle-based adjustments. This approach aligns with the established pattern of using in-memory processing to work around ClickHouse data delays. Based on learnings, this in-memory reconciliation approach is acceptable.
259-273: LGTM!The return statement correctly substitutes the reconciled values while preserving the existing trace structure, ensuring backward compatibility with consumers.
Closes #2798
Problem
The task run page could remain in a partial or stale state after a run finishes. Postgres marks runs as finished immediately, but ClickHouse trace ingestion is eventually consistent. This can cause the UI to show "stuck" or incomplete run traces.
Solution (UI-Layer Only)
This PR reconciles the authoritative Postgres lifecycle with potentially lagging ClickHouse telemetry at the UI layer only.
When a run is finished in Postgres:
The root span is force-marked complete if telemetry is still partial.
Duration is normalized to match the full run lifetime from Postgres.
Failure status is reflected immediately.
Important: This is a UI-layer reconciliation only. No ClickHouse or Postgres data is modified. The goal is to prevent stale UI states, not to override actual data.
Changes
Added reconcileTraceWithRunLifecycle in RunPresenter.server.ts.
Root span duration and status now reflect Postgres lifecycle when telemetry lags.
Unit tests added for:
Lagging telemetry
Completed traces
Failed runs
Missing root spans
Testing
Verified that finished runs immediately show complete status even if ClickHouse telemetry is delayed.
Verified failed runs display failed correctly.
Unit tests cover duration adjustments and missing root spans.
Changelog
Fixed task run page refresh lag by reconciling Postgres lifecycle with UI trace data.