feat(webapp): runs live updating#3776
Conversation
|
|
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 ignored due to path filters (1)
📒 Files selected for processing (17)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (15)
📜 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). (11)
🧰 Additional context used📓 Path-based instructions (7)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.server.ts📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (7)📚 Learning: 2026-03-22T13:26:12.060ZApplied to files:
📚 Learning: 2026-03-22T19:24:14.403ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-03-26T09:02:07.973ZApplied to files:
📚 Learning: 2026-05-05T09:38:02.512ZApplied to files:
📚 Learning: 2026-05-12T21:04:05.815ZApplied to files:
🔇 Additional comments (1)
WalkthroughThis PR implements live-reload for the runs list: a client-side hook polls a typed /live loader (every 3s, paused when hidden) to patch visible run fields and detect newer runs, exposing a “New runs created” banner and dismissal flow. Root-run status cells can render a tooltip that fetches per-child status counts from a /children-statuses loader. Backend additions include a presenter to normalize run rows, a runs.live loader that optionally uses RunsRepository.hasNewRuns (ClickHouse) to mark newer results, a helper to load project/environment from requests, repository + tests covering hasNewRuns behavior, and small UI/hook updates (Button aria-label, PulsingDot, Tooltip delay/type, useInterval pauseWhenHidden). Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 docstrings
🧪 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. Comment |
a5e04f2 to
dc75e89
Compare
d454f49 to
e3e23b2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx (2)
1-12: 💤 Low valueMissing agentcrumbs markers in new file.
This new module contains no
//@Crumbs/ `#region `@crumbsmarkers. As per coding guidelines: "Add agentcrumbs markers (//@Crumbsor `#region `@crumbs) as you write code, not just when debugging. They stay on the branch throughout development and are stripped byagentcrumbs stripbefore merge."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx` around lines 1 - 12, The new RunStatusCellTooltip module is missing agentcrumbs markers; add a top-level marker (e.g., "// `@crumbs`" or "`#region` `@crumbs`") and, where helpful, small scoped markers around interactive sections such as the RunStatusCellTooltip component, any helper hooks or callbacks (e.g., the exported component/function RunStatusCellTooltip, and internal hooks like useFetcher-related logic or animation blocks using AnimatePresence/motion) so agentcrumbs can track changes during development; keep markers minimal and present in this file and ensure they will be removed by running "agentcrumbs strip" before merging.
228-244: ⚡ Quick winAdjust tooltip copy for “waiting for children” state
showLoadingintentionally stays true while the root isn’t final (hasFinished === false), even when the children-status loader returns[]; the code explicitly comments that empty child statuses can mean children haven’t been created yet and keeps polling until the root finishes. Since this means a childless (or “children not created yet”) running root can display “Loading …” for the whole run, consider changing the copy to reflect “waiting for children” (instead of an ongoing load) rather than falling back todescriptionForTaskRunStatus.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx` around lines 228 - 244, The tooltip currently shows a generic "Loading …" when showLoading is true even for a running root that has no child statuses yet; update the JSX conditional around showLoading (which uses showLoading, hasFinished, hasChildStatuses, orderedChildStatuses) so that when showLoading is true AND hasChildStatuses is false AND hasFinished is false you render a clearer "Waiting for children…" (or similar) message instead of "Loading …", while keeping the existing ChildStatusBreakdown and descriptionForTaskRunStatus fallbacks for the other branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx`:
- Around line 1-12: The new RunStatusCellTooltip module is missing agentcrumbs
markers; add a top-level marker (e.g., "// `@crumbs`" or "`#region` `@crumbs`") and,
where helpful, small scoped markers around interactive sections such as the
RunStatusCellTooltip component, any helper hooks or callbacks (e.g., the
exported component/function RunStatusCellTooltip, and internal hooks like
useFetcher-related logic or animation blocks using AnimatePresence/motion) so
agentcrumbs can track changes during development; keep markers minimal and
present in this file and ensure they will be removed by running "agentcrumbs
strip" before merging.
- Around line 228-244: The tooltip currently shows a generic "Loading …" when
showLoading is true even for a running root that has no child statuses yet;
update the JSX conditional around showLoading (which uses showLoading,
hasFinished, hasChildStatuses, orderedChildStatuses) so that when showLoading is
true AND hasChildStatuses is false AND hasFinished is false you render a clearer
"Waiting for children…" (or similar) message instead of "Loading …", while
keeping the existing ChildStatusBreakdown and descriptionForTaskRunStatus
fallbacks for the other branches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 77dc994d-e2cd-4b11-a17a-babca6486b45
📒 Files selected for processing (4)
apps/webapp/app/components/primitives/Tooltip.tsxapps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsxapps/webapp/app/components/runs/v3/TaskRunsTable.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.children-statuses.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.children-statuses.ts
- apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
- apps/webapp/app/components/primitives/Tooltip.tsx
📜 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). (10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: typecheck / typecheck
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
**/*.{ts,tsx,js,jsx}: Prefer static imports over dynamic imports. Only use dynamicimport()when circular dependencies cannot be resolved otherwise, code splitting is needed for performance, or the module must be loaded conditionally at runtime.
Import from@trigger.dev/coreusing subpaths only - never import from the root.
When writing Trigger.dev tasks, always import from@trigger.dev/sdk. Never use@trigger.dev/sdk/v3or deprecatedclient.defineJob.
Add agentcrumbs markers (//@Crumbsor `#region `@crumbs) as you write code, not just when debugging. They stay on the branch throughout development and are stripped byagentcrumbs stripbefore merge.
Files:
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.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/components/runs/v3/RunStatusCellTooltip.tsx
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/components/runs/v3/RunStatusCellTooltip.tsx
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting must be enforced using Prettier before committing
Files:
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx
🧠 Learnings (9)
📚 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/components/runs/v3/RunStatusCellTooltip.tsx
📚 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/components/runs/v3/RunStatusCellTooltip.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/components/runs/v3/RunStatusCellTooltip.tsx
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.
Applied to files:
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.
Applied to files:
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx
📚 Learning: 2026-03-22T13:32:44.229Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/metrics/ProvidersFilter.tsx:74-96
Timestamp: 2026-03-22T13:32:44.229Z
Learning: When reviewing components under `apps/webapp/app/components/runs/v3/`, avoid flagging “broken/unconnected search state” in filters that use `FilterMenuProvider` wrapping Ariakit’s `ComboboxProvider` and expose `(search, setSearch)` (render props). In this intentional pattern, the `searchValue` render-prop value should be treated as reactive (it re-renders on every keystroke), passed into the dropdown child, and used in `useMemo` to filter options. Do not require additional wiring beyond this established render-prop/ComboboxProvider integration.
Applied to files:
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.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/runs/v3/RunStatusCellTooltip.tsx
📚 Learning: 2026-05-08T21:00:20.973Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3538
File: apps/webapp/app/components/primitives/Resizable.tsx:60-78
Timestamp: 2026-05-08T21:00:20.973Z
Learning: In the triggerdotdev/trigger.dev codebase, treat Zod as a boundary validation tool (API handlers, request/response validation, and storage/DB read/write validation), not as inline render-time validation inside React components/primitive UI code. For render-time guards, prefer small manual type-narrowing checks (e.g., a short predicate like ~10–20 lines) over importing Zod into UI primitives, to avoid per-render schema-parse overhead and unnecessary abstraction. Use the manual guard approach unless you truly need schema validation at a boundary; only then introduce Zod.
Applied to files:
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.
Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.
Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.
Applied to files:
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx
🔇 Additional comments (4)
apps/webapp/app/components/runs/v3/RunStatusCellTooltip.tsx (4)
14-51: LGTM!
53-87: LGTM!
89-195: LGTM!
197-255: LGTM!
e4925d1 to
6a16c7e
Compare
Poll runs/live every 3s for unfinished visible runs and patch status in place. Detect newer runs matching filters (~6s) and show a pulsing refresh banner. Pause polling when the tab is hidden or the banner is shown.
Add runs/children-statuses resource route with PG groupBy per root. Show breakdown on root rows after a 400ms hover delay; fetch when the tooltip opens; poll every 3s while open until children settle.
6a16c7e to
dca4165
Compare
| const grouped = await $replica.taskRun.groupBy({ | ||
| by: ["rootTaskRunId", "status"], | ||
| where: { | ||
| projectId: project.id, | ||
| runtimeEnvironmentId: environment.id, | ||
| rootTaskRunId: { | ||
| in: roots.map((run) => run.id), | ||
| }, | ||
| }, | ||
| _count: { | ||
| _all: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🔴 GROUP BY on TaskRun table without index on rootTaskRunId (dropped in prior migration)
The children-statuses resource route performs a groupBy on TaskRun filtering by rootTaskRunId IN (...). The index TaskRun_rootTaskRunId_idx was explicitly dropped in migration 20260401132508_drop_task_run_root_task_run_id_idx, so this query will require a sequential scan on a multi-million-row table. This violates two explicit 🔴 rules from REVIEW.md: (1) "Missing index on a hot table — New Prisma queries against TaskRun must use an existing index" and (2) "Aggregations on hot tables — No COUNT / GROUP BY on TaskRun or other multi-million-row tables. Use Redis or ClickHouse for counts." The endpoint is called on tooltip hover per root run row, and polling continues every 3 seconds while the tooltip is open, so this can generate sustained load against an unindexed aggregation.
Prompt for agents
The children-statuses route performs a Prisma groupBy on the TaskRun table filtering by rootTaskRunId, but the index on that column was dropped in migration 20260401132508. This violates the repo rules against aggregations on hot tables and queries without indexes on TaskRun.
The query at lines 92-104 in resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.children-statuses.ts does:
$replica.taskRun.groupBy({ by: ["rootTaskRunId", "status"], where: { rootTaskRunId: { in: ... } }, _count: { _all: true } })
Possible approaches:
1. Move this aggregation to ClickHouse, which already stores task run data and is the approved path for counts/aggregations per REVIEW.md.
2. If keeping it in Postgres is necessary, re-add the rootTaskRunId index (but this conflicts with the decision to drop it in migration 20260401132508 and still violates the aggregation-on-hot-tables rule).
3. Pre-compute and cache child status counts in Redis, updated as run statuses change.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
The Runs list now updates live without requiring a page refresh. Status changes and other run fields are updated in place while runs are executing.
When new runs matching the current filters are created, a "New runs created" refresh button appears above the list.
Root runs now show a live child-run status breakdown directly in the status tooltip.
List live update
Child-status tooltip
Supporting changes
runIdsdeduping and limits).Test plan
Manual smoke: