feat(webapp): errors page polish and GA rollout#3477
Merged
Conversation
|
… clean Pin a 'No channel' SelectItem at the top of the Slack channel dropdown so unsetting lives where the rest of channel selection lives. Mirror FeatureFlagsDialog's stableStringify(current) !== stableStringify(initial) dirty-check to keep Save disabled until the form actually changes.
Matches the prior PageAccessories pattern used on /alerts, /deployments, /bulk-actions, etc. — drops the in-sidebar header row so 'Details' stands on its own.
Add a U+2026 ellipsis to the 'Configure alerts' button on the errors list and detail pages to signal it opens a panel rather than acting inline. Center the error-detail Suspense fallback by giving it h-full so the spinner sits in the middle of the page instead of pinned to the top.
Replace the small inline 'No runs found for this error.' paragraph with a centered icon + dimmed text block, mirroring the empty state in the Query/Dashboards page. Uses IconBugFilled to tie visually to the errors context.
Drop the admin/impersonating gate on the Errors SideMenu entry so the feature is generally available. Route loaders are not gated, so this just unhides the nav link for non-admins.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📜 Recent review details⏰ 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). (29)
WalkthroughThe PR updates UI and alert-delivery behavior: the Slack channel select in ConfigureErrorAlerts is now a controlled input with an explicit "No channel" option ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/webapp/app/v3/services/alerts/deliverAlert.server.ts (1)
1184-1197: ⚡ Quick winExtract the Slack timestamp formatter into a shared helper.
Lines 1184-1197 now duplicate
DeliverErrorGroupAlertService.#formatTimestamp()almost verbatim. Centralizing the<!date^...>construction will keep both alert paths in sync the next time this Slack formatting needs to change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/v3/services/alerts/deliverAlert.server.ts` around lines 1184 - 1197, Duplicate Slack timestamp formatting in DeliverErrorGroupAlertService.#formatTimestamp() should be extracted to a single shared helper (e.g., formatSlackTimestamp or formatSlackDateToken). Create a utility function that takes a Date and returns the full Slack date token string (`<!date^...|fallback>`) using the same unix/fallback logic and Intl formatting, replace the existing private method implementation in DeliverErrorGroupAlertService with a call to that helper, and update the other duplicated caller(s) to import/use this helper so both alert paths share the same implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/webapp/app/v3/services/alerts/deliverAlert.server.ts`:
- Around line 1184-1197: Duplicate Slack timestamp formatting in
DeliverErrorGroupAlertService.#formatTimestamp() should be extracted to a single
shared helper (e.g., formatSlackTimestamp or formatSlackDateToken). Create a
utility function that takes a Date and returns the full Slack date token string
(`<!date^...|fallback>`) using the same unix/fallback logic and Intl formatting,
replace the existing private method implementation in
DeliverErrorGroupAlertService with a call to that helper, and update the other
duplicated caller(s) to import/use this helper so both alert paths share the
same implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 93a6c195-b9a0-4bc7-a1ad-d7eea5874639
📒 Files selected for processing (6)
apps/webapp/app/components/errors/ConfigureErrorAlerts.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts
📜 Review details
⏰ 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). (13)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.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/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Add crumbs as you write code using
//@Crumbscomments or `// `#region` `@crumbsblocks. These are temporary debug instrumentation and must be stripped usingagentcrumbs stripbefore merge.
Files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.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/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/v3/services/alerts/deliverAlert.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
**/*.ts{,x}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from
@trigger.dev/sdkwhen writing Trigger.dev tasks. Never use@trigger.dev/sdk/v3or deprecatedclient.defineJob.
Files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
apps/webapp/**/*.server.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/**/*.server.ts: Never userequest.signalfor detecting client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.tsinstead, which is wired directly to Expressres.on('close')and fires reliably
Access environment variables viaenvexport fromapp/env.server.ts. Never useprocess.envdirectly
Always usefindFirstinstead offindUniquein Prisma queries.findUniquehas an implicit DataLoader that batches concurrent calls and has active bugs even in Prisma 6.x (uppercase UUIDs returning null, composite key SQL correctness issues, 5-10x worse performance).findFirstis never batched and avoids this entire class of issues
Files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/v3/services/alerts/deliverAlert.server.ts
apps/webapp/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array. Don't wrap ordinary event handlers or trivial computations
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
🧠 Learnings (26)
📓 Common learnings
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3200
File: docs/config/config-file.mdx:353-368
Timestamp: 2026-03-10T12:44:19.869Z
Learning: In the triggerdotdev/trigger.dev repository, docs PRs are often written as companions to implementation PRs (e.g., PR `#3200` documents features being added in PR `#3196`). When reviewing docs PRs, the documented features may exist in a companion/companion PR branch rather than main. Always check companion PRs referenced in the PR description before flagging missing implementations.
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors/route.tsx:82-151
Timestamp: 2026-03-22T19:32:16.231Z
Learning: In `apps/webapp/app/v3/services/alerts/createAlertChannel.server.ts` and `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors/route.tsx`, the `errorAlertConfig` field on `ProjectAlertChannel` is intentionally `Json?` (nullable). The `ErrorAlertEvaluator.computeMinInterval()` in `apps/webapp/app/v3/services/alerts/errorAlertEvaluator.server.ts` uses `ErrorAlertConfig.safeParse(ch.errorAlertConfig)` and falls back to `DEFAULT_INTERVAL_MS = 300_000` when `errorAlertConfig` is null. No UI currently collects this value — it is scaffolding for a future per-channel evaluation interval feature. Do not flag the absence of `errorAlertConfig` in `CreateAlertChannelOptions` or the action handler as a bug; null configs are safe and expected.
📚 Learning: 2026-03-22T19:32:16.231Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors/route.tsx:82-151
Timestamp: 2026-03-22T19:32:16.231Z
Learning: In `apps/webapp/app/v3/services/alerts/createAlertChannel.server.ts` and `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors/route.tsx`, the `errorAlertConfig` field on `ProjectAlertChannel` is intentionally `Json?` (nullable). The `ErrorAlertEvaluator.computeMinInterval()` in `apps/webapp/app/v3/services/alerts/errorAlertEvaluator.server.ts` uses `ErrorAlertConfig.safeParse(ch.errorAlertConfig)` and falls back to `DEFAULT_INTERVAL_MS = 300_000` when `errorAlertConfig` is null. No UI currently collects this value — it is scaffolding for a future per-channel evaluation interval feature. Do not flag the absence of `errorAlertConfig` in `CreateAlertChannelOptions` or the action handler as a bug; null configs are safe and expected.
Applied to files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
📚 Learning: 2026-03-22T19:25:22.249Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/errorAlertEvaluator.server.ts:131-155
Timestamp: 2026-03-22T19:25:22.249Z
Learning: In `apps/webapp/app/v3/services/alerts/errorAlertEvaluator.server.ts`, the `ErrorAlertEvaluator` intentionally uses the minimum configured `evaluationIntervalMs` across all `ERROR_GROUP` alert channels (`computeMinInterval`) as the single project-level scheduling cadence. All channels are expected to share the same interval value in practice, so there is no per-channel interval guard in the delivery loop. Do not flag this as a bug or suggest adding per-channel interval checks.
Applied to files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts
📚 Learning: 2026-03-22T19:27:29.014Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/createAlertChannel.server.ts:104-112
Timestamp: 2026-03-22T19:27:29.014Z
Learning: In `apps/webapp/app/v3/services/alerts/createAlertChannel.server.ts`, the `#scheduleErrorAlertEvaluation` helper intentionally uses the same job id (`evaluateErrorAlerts:${projectId}`) as the evaluator's periodic self-chain. The deduplication is desired: if a future run is already queued, the immediate enqueue becomes a no-op, preventing two evaluations firing in quick succession. Do not flag this as a bug or suggest a unique/timestamped id.
Applied to files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/v3/services/alerts/deliverAlert.server.ts
📚 Learning: 2026-03-10T17:56:20.938Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3201
File: apps/webapp/app/v3/services/setSeatsAddOn.server.ts:25-29
Timestamp: 2026-03-10T17:56:20.938Z
Learning: Do not implement local userId-to-organizationId authorization checks inside org-scoped service classes (e.g., SetSeatsAddOnService, SetBranchesAddOnService) in the web app. Rely on route-layer authentication (requireUserId(request)) and org membership enforcement via the _app.orgs.$organizationSlug layout route. Any userId/organizationId that reaches these services from org-scoped routes has already been validated. Apply this pattern across all org-scoped services to avoid redundant auth checks and maintain consistency.
Applied to files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/v3/services/alerts/deliverAlert.server.ts
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/v3/services/alerts/deliverAlert.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
📚 Learning: 2026-03-29T19:16:28.864Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3291
File: apps/webapp/app/v3/featureFlags.ts:53-65
Timestamp: 2026-03-29T19:16:28.864Z
Learning: When reviewing TypeScript code that uses Zod v3, treat `z.coerce.*()` schemas as their direct Zod type (e.g., `z.coerce.boolean()` returns a `ZodBoolean` with `_def.typeName === "ZodBoolean"`) rather than a `ZodEffects`. Only `.preprocess()`, `.refine()`/`.superRefine()`, and `.transform()` are expected to wrap schemas in `ZodEffects`. Therefore, in reviewers’ logic like `getFlagControlType`, do not flag/unblock failures that require unwrapping `ZodEffects` when the input schema is a `z.coerce.*` schema.
Applied to files:
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.tsapps/webapp/app/v3/services/alerts/deliverAlert.server.ts
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx
📚 Learning: 2026-04-03T11:54:21.609Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions/route.tsx:265-267
Timestamp: 2026-04-03T11:54:21.609Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions/route.tsx`, the `TableCellMenu` for the "Set as default" region action intentionally uses `hiddenButtons` (hover-only visibility) rather than `visibleButtons`. This is a deliberate UX design choice by the team. Do not flag the hover-only visibility as an accessibility concern for this specific instance.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx
📚 Learning: 2026-04-02T19:18:34.807Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx:249-258
Timestamp: 2026-04-02T19:18:34.807Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx`, the `onCollapseChange={() => {}}` no-op on the `ResizablePanel` for the waitpoint inspector is intentional. The panel collapse is controlled externally via `collapsed={!isShowingWaitpoint}` (driven by URL params), so the callback is deliberately left as a no-op. Do not flag this as a missing implementation.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx
📚 Learning: 2026-04-02T20:25:54.203Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.models._index/route.tsx:1100-1101
Timestamp: 2026-04-02T20:25:54.203Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.models._index/route.tsx`, the `useFrozenValue(selectedModel)` + `displayModel = selectedModel ?? frozenModel` pattern is intentional. It keeps `ModelDetailPanel` rendered during the collapse animation even after `selectedModel` is cleared. The `key={displayModel.friendlyId}` handles remounting when a different model is selected. Do not flag this as a stale-state or tab-reset issue.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx
📚 Learning: 2026-04-01T13:27:35.831Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3308
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.models._index/route.tsx:579-584
Timestamp: 2026-04-01T13:27:35.831Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.models._index/route.tsx`, the `useEffect` inside `CompareDialog` intentionally uses `[open]` as its only dependency. This is a deliberate "fetch-on-open" pattern: the fetcher.load call should fire only when the dialog opens, not on every reference change of `models`, `organization`, `project`, or `environment`. Do not flag this as a missing-dependency lint issue.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
📚 Learning: 2026-02-11T16:50:14.167Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx:126-131
Timestamp: 2026-02-11T16:50:14.167Z
Learning: In apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx, MetricsDashboard entities are intentionally scoped to the organization level, not the project level. The dashboard lookup should filter by organizationId only (not projectId), allowing dashboards to be accessed across projects within the same organization. The optional projectId field on MetricsDashboard serves other purposes and should not be used as an authorization constraint.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
📚 Learning: 2026-04-02T19:18:26.255Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions/route.tsx:179-189
Timestamp: 2026-04-02T19:18:26.255Z
Learning: In this repo’s route components that render the Inspector `ResizablePanelGroup` panels, it’s acceptable to pass `collapsed={!isShowingInspector}` together with a no-op `onCollapseChange={() => {}}` when panel visibility is intentionally controlled only by route parameters (e.g., `*Param` search/route params) rather than user drag/collapse interactions. Do not flag an empty/no-op `onCollapseChange` as “missing wiring” in these cases; only flag it when collapse state is expected to change based on user interaction.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx
📚 Learning: 2026-03-22T13:45:36.346Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/navigation/SideMenu.tsx:460-489
Timestamp: 2026-03-22T13:45:36.346Z
Learning: In triggerdotdev/trigger.dev, sidebar navigation items (SideMenu.tsx) are intentionally NOT gated behind feature-flag or permission checks at the nav level. Authorization is enforced at the route/loader level instead. Hiding nav items based on access checks is considered confusing UX. This applies to items like "AI Metrics" (v3BuiltInDashboardPath) and other dashboard links — they are always rendered in the sidebar regardless of hasQueryAccess or similar flags.
Applied to files:
apps/webapp/app/components/navigation/SideMenu.tsx
📚 Learning: 2026-02-03T18:27:49.039Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:49.039Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (like the Edit button with PencilSquareIcon) intentionally have no text labels - only icons are shown in the TableCellMenu. This is a deliberate UI design pattern for compact icon-only menu items.
Applied to files:
apps/webapp/app/components/navigation/SideMenu.tsx
📚 Learning: 2026-04-16T14:21:15.229Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/components/logs/LogsTaskFilter.tsx:135-163
Timestamp: 2026-04-16T14:21:15.229Z
Learning: When rendering lists of task registry items in apps/webapp (e.g., <SelectItem /> rows) and using `key={item.slug}`, do not flag it as potentially non-unique. In trigger.dev’s `TaskIdentifier` table, the DB constraint `@unique([runtimeEnvironmentId, slug])` guarantees `slug` is unique within a given runtime environment, so `item.slug` is safe as the React key as long as the list is derived from that registry/constraint (and not from a legacy query that could produce duplicate slugs).
Applied to files:
apps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx
📚 Learning: 2026-03-26T17:27:09.938Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3264
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx:176-176
Timestamp: 2026-03-26T17:27:09.938Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx`, the variable `hasPrivateNetworking` is intentionally hardcoded to `true` as a placeholder. Plan-level gating will be wired to actual billing data (e.g., `plan?.v3Subscription?.plan?.limits?.hasPrivateNetworking`) once the billing integration is complete. The route is already guarded at the feature-flag level via `hasPrivateConnections` in the loader. Do not flag this hardcoded value as dead code or a bug until the billing integration is in place.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx
📚 Learning: 2026-02-04T16:34:48.876Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/vercel.connect.tsx:13-27
Timestamp: 2026-02-04T16:34:48.876Z
Learning: In apps/webapp/app/routes/vercel.connect.tsx, configurationId may be absent for "dashboard" flows but must be present for "marketplace" flows. Enforce this with a Zod superRefine and pass installationId to repository methods only when configurationId is defined (omit the field otherwise).
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx
📚 Learning: 2026-04-29T21:49:48.296Z
Learnt from: isshaddad
Repo: triggerdotdev/trigger.dev PR: 3475
File: apps/webapp/app/components/admin/backOffice/RateLimitSection.tsx:61-70
Timestamp: 2026-04-29T21:49:48.296Z
Learning: In `apps/webapp/app/components/admin/backOffice/RateLimitSection.tsx`, the local form state (`refillRate`, `intervalStr`, `maxTokens`) is intentionally seeded only once via `useState` initializers from `current` (the effective token-bucket config). This is safe in the Remix model because: (1) a successful save redirects, causing a remount with fresh loader data; (2) a failed 400 returns no redirect, so `current` stays the same and React preserves the user's typed input; (3) navigating to a different org causes remount and re-seeds state; (4) Cancel explicitly re-seeds via `cancelEdit()`. Do NOT add a `useEffect` that re-seeds from `current` on config changes — it would clobber mid-edit valid input during background revalidation, which is a regression.
Applied to files:
apps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Use `trigger.dev/react-hooks` for React components that need to display real-time task status and results
Applied to files:
apps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
📚 Learning: 2026-03-22T13:32:47.004Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/metrics/ProvidersFilter.tsx:74-96
Timestamp: 2026-03-22T13:32:47.004Z
Learning: In the triggerdotdev/trigger.dev codebase, `FilterMenuProvider` (in `apps/webapp/app/components/runs/v3/SharedFilters.tsx`) wraps Ariakit's `ComboboxProvider` and exposes `(search, setSearch)` as render props. Filter components like `ProvidersFilter`, `OperationsFilter`, `PromptsFilter`, `ModelsFilter`, and `QueuesFilter` all use this pattern: `searchValue` (from the render prop) is passed into the dropdown child and used in a `useMemo` to filter options. Because Ariakit's ComboboxProvider re-renders the render prop on every keystroke, `searchValue` is reactive and the filtered list updates correctly. Do not flag these patterns as broken/unconnected search state — the wiring is intentional and correct.
Applied to files:
apps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
📚 Learning: 2026-03-22T13:32:46.525Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/metrics/OperationsFilter.tsx:85-110
Timestamp: 2026-03-22T13:32:46.525Z
Learning: In triggerdotdev/trigger.dev, `FilterMenuProvider` (from `~/components/runs/v3/SharedFilters`) wraps Ariakit's `ComboboxProvider` and exposes the current search string and setter via render props `(search, setSearch) => …`. Components like `OperationsFilter`, `ModelsFilter`, `QueuesFilter`, and `ProvidersFilter` all use this pattern: they pass `searchValue={search}` and `clearSearchValue={() => setSearch("")}` into their inner dropdown component, and the `filtered` useMemo is correctly reactive. Do not flag this as a missing/broken wiring between the ComboBox input and the search state — the state is managed by Ariakit's provider and surfaced through the render-prop tuple.
Applied to files:
apps/webapp/app/components/errors/ConfigureErrorAlerts.tsx
🔇 Additional comments (9)
apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts (1)
385-398: LGTM.Using Slack’s native
<!date...>syntax here is the right approach for client-side timezone-aware rendering.apps/webapp/app/components/navigation/SideMenu.tsx (1)
527-535: Errors nav visibility change looks correct.Nice simplification—this keeps the item visible in the section and leaves access enforcement to route/loader checks.
Based on learnings: “sidebar navigation items (SideMenu.tsx) are intentionally NOT gated behind feature-flag or permission checks at the nav level… Authorization is enforced at the route/loader level instead.”
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsx (2)
467-468: “Configure alerts…” copy update is consistent.Good wording alignment with the rest of the errors/alerts flow.
710-717: Tooltip affordance for peak count is a good UX improvement.This adds context without increasing visual noise in the table cell.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx (3)
331-340: Top-nav “Configure alerts…” placement looks good.This is a cleaner, more discoverable entry point than burying it in the detail sidebar.
513-518: Empty-state presentation is improved.The icon + centered copy gives a clearer no-data state for runs.
527-543: Sidebar prop/thread cleanup is solid.Removing alert wiring from
ErrorDetailSidebarkeeps responsibilities tighter after the CTA move.apps/webapp/app/components/errors/ConfigureErrorAlerts.tsx (2)
111-135: Dirty-state guard on Save is a strong improvement.This prevents unnecessary submissions and makes the submit state behavior much clearer.
Also applies to: 388-388
230-260: Controlled Slack channel select + explicit “No channel” option is well done.Good UX and clearer intent for clearing an existing Slack target.
matt-aitken
reviewed
Apr 30, 2026
matt-aitken
approved these changes
Apr 30, 2026
Revert the formChangeTick / isDirty / stableStringify block — Save is back to disabled-while-submitting only.
D-K-P
force-pushed
the
feature/tri-8760-errors-page-improvements-and-bug-fixes
branch
from
ea49d6e to
95ae892
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
D-K-P
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
D-K-P
deleted the
feature/tri-8760-errors-page-improvements-and-bug-fixes
branch
Comment on lines
+710
to
+717
| <SimpleTooltip | ||
| button={ | ||
| <span className="-mt-1 text-xxs tabular-nums text-text-dimmed"> | ||
| {formatNumberCompact(maxCount)} | ||
| </span> | ||
| } | ||
| content="Peak occurrences in a single time bucket" | ||
| /> |
Contributor
There was a problem hiding this comment.
🚩 SimpleTooltip introduces a nested inside a in the table cell
The new SimpleTooltip wrapping the peak occurrences number (errors._index/route.tsx:710-717) renders a <button> element (via TooltipTrigger at Tooltip.tsx:87-88). This button sits inside the TableCell which has to={errorPath} (errors._index/route.tsx:583), rendering its children inside a <Link> (i.e., <a> tag) at Table.tsx:323-330. This creates a <button> nested inside an <a>, which is invalid HTML per the spec (interactive content cannot be nested in other interactive content). In practice most browsers handle this gracefully for hover-only tooltips, but it can cause accessibility warnings and subtle click-propagation issues. Consider using asChild={true} on the SimpleTooltip or restructuring so the tooltip trigger isn't a <button>.
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Polish + bug-fix pass on the Errors page so it can ship to everyone. Touches the Slack alert config UX, errors list, error detail page, and unhides the SideMenu entry for non-admins.
Decisions
"No channel" item over standalone Remove button
Chose pinning a
<XMarkIcon /> No channelSelectItemabove the channel list. Rejected the standalone "Remove channel" link in a<Hint>— color/hover behaviour clashed with the sibling<TextLink>, and "channel selection" is the right context for clearing. Server action already deletes the channel whenslackChannel=""is submitted.Slack
<!date^>token over per-user TZ field for alertsChose Slack's native
<!date^TS^…>token so each viewer sees timestamps in their own timezone (UTC fallback). Rejected per-user/per-org TZ schema work — works for multi-region channels for free. Email/dashboard TZ source-of-truth filed as TRI-8885 / TRI-8886.Make errors GA