fix(dashboard): remove span ID to original span ID cache and use a different approach to link to the cached runs

[ericallam]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 0cc5a5b

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

[coderabbitai]

Walkthrough

The changes refactor how linked run IDs for cached spans are tracked throughout the system. The originalRunIdCache module and related getSpanOriginalRunId methods are removed from EventRepository classes. Instead, a new linkedRunIdBySpanId mapping is populated in RunPresenter and passed through the trace payload. TreeView types are extended with optional runId fields. SpanPresenter is updated to accept linkedRunId as a parameter and use it directly for run lookups instead of querying a database. Route handlers now retrieve and propagate this linkedRunId through the SpanView component and its data loaders.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is missing entirely. The template requires Testing, Changelog, and Screenshots sections along with a checklist, but none of these are provided.	Add a complete PR description following the template, including Testing steps, Changelog summary, and relevant sections. At minimum, describe what changed and how it was tested.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: removing a caching mechanism for span-to-original-run-ID lookups and replacing it with a different linking approach for cached runs.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx (1)

132-156: Consider URL-encoding linkedRunId in the fetch URL.

The linkedRunId is appended directly to the URL without encoding. While run IDs typically contain only URL-safe characters (alphanumeric + underscores), using encodeURIComponent would be more defensive.

🔧 Suggested fix

-    const url = `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${
-      environment.slug
-    }/runs/${runParam}/spans/${spanId}${linkedRunId ? `?linkedRunId=${linkedRunId}` : ""}`;
+    const url = `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${
+      environment.slug
+    }/runs/${runParam}/spans/${spanId}${linkedRunId ? `?linkedRunId=${encodeURIComponent(linkedRunId)}` : ""}`;

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 260fb7c and 0cc5a5b.

📒 Files selected for processing (9)

• apps/webapp/app/components/primitives/TreeView/TreeView.tsx
• apps/webapp/app/presenters/v3/RunPresenter.server.ts
• apps/webapp/app/presenters/v3/SpanPresenter.server.ts
• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
• apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
• apps/webapp/app/v3/eventRepository/clickhouseEventRepository.server.ts
• apps/webapp/app/v3/eventRepository/eventRepository.server.ts
• apps/webapp/app/v3/eventRepository/eventRepository.types.ts
• apps/webapp/app/v3/eventRepository/originalRunIdCache.server.ts

💤 Files with no reviewable changes (4)

• apps/webapp/app/v3/eventRepository/eventRepository.types.ts
• apps/webapp/app/v3/eventRepository/eventRepository.server.ts
• apps/webapp/app/v3/eventRepository/clickhouseEventRepository.server.ts
• apps/webapp/app/v3/eventRepository/originalRunIdCache.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 instead

Every 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
• apps/webapp/app/components/primitives/TreeView/TreeView.tsx
• apps/webapp/app/presenters/v3/SpanPresenter.server.ts
• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
• apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx

{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use zod for validation in packages/core and apps/webapp

Files:

• apps/webapp/app/presenters/v3/RunPresenter.server.ts
• apps/webapp/app/components/primitives/TreeView/TreeView.tsx
• apps/webapp/app/presenters/v3/SpanPresenter.server.ts
• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
• apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

Files:

• apps/webapp/app/presenters/v3/RunPresenter.server.ts
• apps/webapp/app/components/primitives/TreeView/TreeView.tsx
• apps/webapp/app/presenters/v3/SpanPresenter.server.ts
• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
• apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx

apps/webapp/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access all environment variables through the env export of env.server.ts instead of directly accessing process.env in the Trigger.dev webapp

In 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/app/components/primitives/TreeView/TreeView.tsx
• apps/webapp/app/presenters/v3/SpanPresenter.server.ts
• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
• apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx

apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: When importing from @trigger.dev/core in 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
• apps/webapp/app/components/primitives/TreeView/TreeView.tsx
• apps/webapp/app/presenters/v3/SpanPresenter.server.ts
• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
• apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx

**/*.{js,ts,jsx,tsx,json,md,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier

Files:

• apps/webapp/app/presenters/v3/RunPresenter.server.ts
• apps/webapp/app/components/primitives/TreeView/TreeView.tsx
• apps/webapp/app/presenters/v3/SpanPresenter.server.ts
• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
• apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.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/RunPresenter.server.ts
• apps/webapp/app/presenters/v3/SpanPresenter.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
• apps/webapp/app/components/primitives/TreeView/TreeView.tsx
• apps/webapp/app/presenters/v3/SpanPresenter.server.ts
• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
• apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx

🧠 Learnings (1)

📚 Learning: 2025-12-08T15:19:56.823Z

Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.

Applied to files:

• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
• apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)

• GitHub Check: Cursor Bugbot
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
• 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 (8, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
• GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
• 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 - npm)
• GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
• GitHub Check: typecheck / typecheck
• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (10)

apps/webapp/app/components/primitives/TreeView/TreeView.tsx (3)

528-545: LGTM!

The optional runId field is consistently added to both Tree<TData> and FlatTreeItem<TData> types. Using types (rather than interfaces) aligns with the coding guidelines.

---

549-572: LGTM!

The runId is correctly propagated from the tree node to the flattened item representation.

---

574-604: LGTM!

The runId field is properly added to FlatTreeWithoutChildren<TData> and correctly propagated in createTreeFromFlatItems. This ensures runId is preserved during both tree flattening and tree reconstruction operations.

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (1)

479-535: LGTM! Clean implementation of linkedRunId propagation.

The extraction of linkedRunIdBySpanId from the trace and derivation of selectedSpanLinkedRunId using optional chaining correctly handles all edge cases (no trace, no selected span, no mapping entry). Passing it to SpanView enables the cached span linking feature.

apps/webapp/app/presenters/v3/SpanPresenter.server.ts (3)

38-44: LGTM! Clean parameter threading through call() to getRun().

The linkedRunId parameter is correctly added as optional and threaded to the internal getRun method.

Also applies to: 93-93

---

126-146: LGTM! Correct use of linkedRunId for cached span resolution.

The logic correctly uses linkedRunId when provided (for cached spans) and falls back to spanId lookup otherwise. The comment on line 145 clearly documents the intent.

---

272-272: LGTM! Correct derivation of isCached flag.

Deriving isCached from the presence of linkedRunId is semantically correct - if a linked run ID was passed, the span represents a cached execution pointing to the original run.

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx (1)

96-107: LGTM! Loader correctly extracts and forwards linkedRunId.

The loader properly reads linkedRunId from the URL query parameters and passes it to SpanPresenter.call().

apps/webapp/app/presenters/v3/RunPresenter.server.ts (2)

212-229: LGTM! Efficient mapping construction during tree traversal.

The linkedRunIdBySpanId mapping is built efficiently during the existing flattenTree iteration without additional passes. The guard n.data.style?.icon === "task-cached" && n.runId correctly filters to only cached task spans that have an associated run ID.

---

273-273: LGTM! Correctly exposed in trace payload.

Including linkedRunIdBySpanId in the returned trace object enables the route to pass the linked run ID to SpanView for cached span navigation.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

PR Review: Remove span ID to original span ID cache

Summary

This PR removes the Redis-based originalRunIdCache and replaces it with a simpler approach where the linked run ID for cached spans is computed during the tree traversal in RunPresenter and passed directly to the SpanPresenter. This is a solid architectural improvement that reduces complexity and eliminates a Redis dependency.

Strengths

1. Eliminates Redis cache overhead: Removes ~75 lines of caching infrastructure (originalRunIdCache.server.ts) and associated Redis operations, simplifying the system.

2. Single-pass computation: The linkedRunIdBySpanId map is built during the existing tree walk in RunPresenter, avoiding additional queries.

3. Cleaner data flow: The linked run ID is now passed explicitly through the component hierarchy (RunPresenter → route → SpanView → SpanPresenter) rather than being looked up asynchronously via cache.

4. Removes unused interface method: Cleans up IEventRepository.getSpanOriginalRunId from both implementations.

Observations / Minor Suggestions

1. SpanSummary already has runId: The SpanSummary type at eventRepository.types.ts:249-252 already includes a runId field. The TreeView changes add runId to Tree<TData> and FlatTreeItem<TData> which appears to propagate this existing field through the tree flattening. This is correct usage of the existing data model.

2. Icon check for cached detection (RunPresenter.server.ts:227):

   if (n.data.style?.icon === "task-cached" && n.runId) {
     linkedRunIdBySpanId[n.id] = n.runId;
   }

   Using style.icon === "task-cached" as the source of truth for cached runs is a coupling to presentation logic. However, this is consistent with how other parts of the codebase identify cached runs, so it's acceptable. If this icon name ever changes, this logic would need updating.

3. Query parameter injection (route.tsx:152-154):

   const url = `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${
     environment.slug
   }/runs/${runParam}/spans/${spanId}${linkedRunId ? `?linkedRunId=${linkedRunId}` : ""}`;

   The linkedRunId comes from trusted internal state (the linkedRunIdBySpanId map built from database data), so there's no injection risk here. The value is a run friendly ID like run_xxxx which is validated by Prisma when used in queries.

4. Formatting changes: Several unrelated formatting changes appear in RunBody component (lines 308-315, 577-580, 967-970, 988-989). These improve readability but could have been in a separate commit for cleaner history.

Potential Edge Cases to Consider

1. Cache invalidation during migration: If there are cached runs that were created with the old cache system but the cache entries have expired (TTL was 30-31 days), those spans would have relied on the fallback getSpanOriginalRunId lookup. With this PR, if linkedRunId is not provided, findRun falls back to spanId lookup which should still work for non-cached runs. For cached runs, the runId is stored in SpanSummary, so as long as the trace data exists in ClickHouse/events, the tree traversal will find it. This seems correctly handled.

2. Direct URL access to cached span: If a user bookmarks or shares a URL directly to a cached span panel, and later accesses it without going through the parent run page first, the linkedRunId won't be in the URL. The findRun will fall back to spanId lookup. For cached runs, this might not find the original run. However, this is likely an acceptable edge case since users typically navigate through the UI.

Verdict

LGTM - This is a clean simplification that removes unnecessary caching complexity. The approach of computing the linked run ID during tree traversal and passing it through the component hierarchy is more predictable and easier to debug than the previous Redis cache approach.

[vibe-kanban-cloud]

Review Complete

Your review story is ready!

View Story

Comment !reviewfast on this PR to re-generate the story.