Skip to content

fix(webapp): make the Queues hero charts environment-wide - #4486

Merged
ericallam merged 8 commits into
mainfrom
feature/tri-12784-queues-page-hero-charts-are-scoped-to-the-current-pages-25
Aug 3, 2026
Merged

fix(webapp): make the Queues hero charts environment-wide#4486
ericallam merged 8 commits into
mainfrom
feature/tri-12784-queues-page-hero-charts-are-scoped-to-the-current-pages-25

Conversation

@ericallam

@ericallam ericallam commented Aug 3, 2026

Copy link
Copy Markdown
Member

Summary

The four charts above the queues table aggregated over at most the 25 queues on the current page. They reused the loader's already-paginated queue array as a ClickHouse queue IN (...) filter, so paging or re-sorting changed the values, and a name search matching nothing blanked the whole chart row. The stat tiles above them were already environment-wide, so the two rows disagreed.

They now read env_metrics, the environment-level rollup that already exists for exactly this (the built-in Queues dashboard and the health report read it). That is both correct and queue-count-independent: no GROUP BY queue across an entire environment, and no client-side summing.

Note this is not only a paging artifact: page 1 under-reported too. On the seeded environment below, page 1 read 82% saturation against a true 87%, because the environment's running total is not the sum of one page of per-queue gauges.

Three related fixes ride along.

Scheduling delay and throttling sawed to zero. Both are event-driven, so at the 10-second bucket a short range picks, most buckets hold no samples at all and were drawn as 0ms. Measured over a 1-hour window: 232 of 349 buckets had no scheduling-delay samples. A bucket where nothing started is not a bucket where nothing waited, so the line was both ugly and wrong. TRQL grows a minBucketSeconds floor, plumbed through the metric resource route, and the hero tiles set 60s. Buckets that still have no samples render as a gap instead of a dive to zero.

The floor must not feed a width-dependent headline. Two of the four headlines are not peaks, so widening the plotted buckets moved them:

  • Throttled is a share of buckets that saw any throttling, so a single brief throttle came to mark a whole minute instead of ten seconds: the same seeded events read 17% at 10s and 85% at 60s.
  • Scheduling delay p95 is a percentile, and merging quantile states over a wider bucket yields a p95 between the sub-buckets' own. Two 240s samples among twenty in one 10-second sub-bucket give a worst-of-six p95 of 240,000ms against a merged 60-second p95 of 5,000ms — a 48x understatement of a headline whose tooltip claims it is the worst in the window.

Both charts keep the floor, since a readable line was the point of it. Their headlines now come from a second query at the range's natural bucket width, via an optional readout on the tile, so each means what its tooltip says regardless of how the plotted buckets are sized. Saturation and backlog are genuinely width-invariant (a max of maxes is the same at any width), so they are unchanged and issue no extra query. Both caught by Devin in review; I had wrongly lumped p95 in with the peaks.

Charts reported a hydration mismatch on every render. Recharts resolved victory-vendor's CJS entry on the server and its ESM entry in the browser. Those bundle different d3-shape builds, and the CJS one predates d3-path's digit rounding, so every server-rendered curve carried full-precision coordinates while the client rounded to 3 decimals:

Server: M0,3C0.9305555555555555,3,1.8611111111111112,3,...
Client: M0,3C0.931,3,1.861,3,...

Bundling recharts for SSR makes both sides resolve the same ESM build. Verified: 45 of 45 server-rendered chart curves now match the client, and the page loads with an empty console.

Verification

An isolated stack with 40 seeded queues (20 heavily loaded, 20 idle) and 90 minutes of 10-second buckets written into queue_metrics_raw_v1, so the real materialized views built queue_metrics_v1, env_metrics_v1 and the 5m rollup. Ground truth for the environment: 260 running against a limit of 300 (87% saturation), 800 queued.

before after
Saturation, page 1 82% peak 87% peak
Saturation, page 2 5% peak 87% peak
Backlog / delay, page 2 "No activity" 800 peak / 59.5s
Name search matching nothing all four charts blank charts stay environment-wide
Metric refetches on a page change 4, each painting a skeleton 0, no skeleton
Buckets drawn as 0ms with no samples 232 of 349 0
Throttled readout 17% 17%, unchanged by the wider buckets
Worst-p95 readout source plotted buckets natural width, so a sub-minute spike is not averaged away
Crosshair reach, hovering one detail-page chart 2 of 4 others 4 of 4
SSR chart curves mismatching the client 45 0

The bucket floor was measured across ranges: it widens 10s to 60s at 30m and 1h, and is correctly a no-op at 12h (300s) and 7d (3600s). One extra request per page load, for the throttled readout.

The built-in Queues dashboard, which reads env_metrics independently, agrees at 86.7% and 260 of 300.

internal-packages/tsql suite green (612 tests), including 5 new ones for the floor that fail without it. Webapp typecheck, oxfmt and oxlint clean. Spot-checked the Run metrics dashboard and the per-queue detail page for SSR regressions from bundling recharts: both render, console clean.

The queue detail page carries the same event-driven series, so its scheduling delay, throttling and per-key mean delay take the same treatment.

Screenshots

after-page1-charts

Rollout

Already behind the per-organization queueMetricsUiEnabled flag, so only gated orgs see any of it. Blast radius is chart values on one page plus the SSR bundling of recharts; rollback is a revert with no data migration.

Stated limitations

  • wait_ms_count and the quantile state both only count wait_ms > 0, so "nothing started in this bucket" and "everything started instantly" are indistinguishable in storage. Both render as a gap. Distinguishing them needs a schema change, which is not in this PR.
  • The queue name search deliberately no longer narrows the charts. It only did so incidentally and incorrectly before (first 25 matches, and blanked on zero matches). Search-scoped charts would need the full unpaginated matching set and a server-side aggregate; worth its own ticket if we want it.
  • Bundling recharts for SSR grows the server bundle slightly. That is the cost of both sides resolving one d3-shape build.
  • The plotted delay line is a smoothed 60-second view, so a sub-minute spike above the one-minute warning threshold can fail to colour the line even though the headline reports it and colours itself.
  • Every chart inside one synced group shares the floor, because the hover crosshair is a reference line on a category x-axis and only draws where the hovered bucket exists in the other chart's own data. That costs the queue detail page's gauges some resolution (1 minute instead of 10 seconds) in exchange for the crosshair working across the row.

Separately, while taking the screenshots I found a pre-existing rendering bug unrelated to this change: a perfectly flat saturation series draws no line at all (the readout still shows the right percentage), which looks like the threshold gradient's offset degenerating when the series min equals its max. It reproduces on main, so it is not a regression here and I have left it alone; filed as its own issue.

Refs TRI-12784

The four charts above the queues table reused the loader's already-paginated
queue array as a ClickHouse `queue IN (...)` filter, so they aggregated over at
most the 25 queues on the current page. Paging or re-sorting changed the values,
and a name search that matched nothing blanked the whole chart row.

They now read `env_metrics`, the environment-level rollup that already exists
for this (the built-in Queues dashboard and the health report read it), which is
both correct and queue-count-independent: no `GROUP BY queue` across an entire
environment and no client-side summing.

Two related fixes ride along:

Scheduling delay and throttling are event-driven, so at the 10-second bucket a
short range picks, most buckets hold no samples at all and were drawn as 0ms —
measured at 232 of 349 buckets over an hour. TRQL grows a `minBucketSeconds`
floor, plumbed through the metric resource route, and the hero tiles set 60s
(one floor for all four, since the shared hover crosshair needs identical
x-axes). Buckets that still have no samples now render as a gap rather than a
dive to zero. Note that `wait_ms_count` only counts `wait_ms > 0`, so "nothing
started" and "everything started instantly" are indistinguishable in storage;
both read as a gap.

Recharts was resolving victory-vendor's CJS entry on the server and its ESM
entry in the browser. Those bundle different d3-shape builds — the CJS one
predates d3-path's digit rounding — so every server-rendered curve carried
full-precision coordinates while the client rounded to 3 decimals, and React
reported a hydration mismatch on every chart. Bundling recharts for SSR makes
both sides resolve the same ESM build.
@changeset-bot

changeset-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8ad0c59

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

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The change adds optional minimum bucket sizing to metric query options, request validation, query execution, TSQL compilation, and time-bucket calculations. Queues page charts now use environment-wide metrics, apply a 60-second minimum bucket width, and preserve null gaps for buckets without samples. SSR configuration bundles chart dependencies. A changelog entry documents the chart behavior.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the primary change: making the Queues hero charts environment-wide.
Description check ✅ Passed The description is detailed and covers the change, testing, changelog, screenshots, rollout, limitations, and issue reference.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/tri-12784-queues-page-hero-charts-are-scoped-to-the-current-pages-25

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal-packages/tsql/src/query/printer_context.ts (1)

237-245: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Forward minBucketSeconds in child contexts.

createChildContext() propagates fillGaps, but the child remains undefined for minBucketSeconds because it is not passed to the PrinterContext constructor. Child contexts are also never created from this code path, so add any child creation/use test coverage for this option; if child contexts handle subqueries, include minBucketSeconds there to avoid losing the configured bucket floor.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: da63eaf4-ef6b-4b52-8334-07177e150e8e

📥 Commits

Reviewing files that changed from the base of the PR and between 5f29ae4 and 461cebf.

📒 Files selected for processing (12)
  • .server-changes/queues-page-environment-wide-charts.md
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/services/queryService.server.ts
  • apps/webapp/vite.config.ts
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • internal-packages/tsql/src/query/time_buckets.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
  • GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
  • GitHub Check: internal / 🧪 Unit Tests: Internal
  • GitHub Check: typecheck / typecheck
  • GitHub Check: code-quality / code-quality
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}

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

**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead

**/*.{ts,tsx}: Prefer static imports over dynamic import(); use dynamic imports only for unresolvable circular dependencies, genuine performance code splitting, or conditional runtime loading.
Import Trigger.dev tasks from @trigger.dev/sdk; never use @trigger.dev/sdk/v3 or deprecated client.defineJob.
Add agentcrumbs while writing code using approved namespaces; mark lines with // @Crumbs or blocks with `// `#region` `@crumbs, and strip them before merging.

Files:

  • apps/webapp/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/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/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • apps/webapp/app/services/queryService.server.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
**/*.{ts,tsx,js,jsx}

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

Use function declarations instead of default exports

Files:

  • apps/webapp/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/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/vite.config.ts
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
apps/webapp/**/*.{ts,tsx}

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

apps/webapp/**/*.{ts,tsx}: Access environment variables through the env export of env.server.ts instead of directly accessing process.env
Use subpath exports from @trigger.dev/core package instead of importing from the root @trigger.dev/core path

Do not reintroduce the removed v1 execution path; RunEngineVersion.V1 branches may only reject or finalize gracefully so v3 clients receive a clean 4xx, never a 5xx.

Files:

  • apps/webapp/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • apps/webapp/app/services/queryService.server.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
apps/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

For apps, use typecheck for verification and never use build as the correctness check.

Files:

  • apps/webapp/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • apps/webapp/app/services/queryService.server.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
apps/webapp/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)

apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
Use useCallback and useMemo only for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.

Files:

  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • apps/webapp/app/services/queryService.server.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
internal-packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

For internal packages, use typecheck for verification and never use build as the correctness check.

Files:

  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • internal-packages/tsql/src/query/printer.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
**/*.{test,spec}.{ts,tsx}

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

Use vitest for all tests in the Trigger.dev repository

**/*.{test,spec}.{ts,tsx}: Use Vitest exclusively and never mock dependencies; use Testcontainers for integration dependencies.
Place test files next to the source files they test.

Files:

  • internal-packages/tsql/src/query/time_buckets.test.ts
apps/webapp/app/**/*.ts

📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)

apps/webapp/app/**/*.ts: Never use request.signal to detect client disconnects. Use getRequestAbortSignal() from app/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through the env export from app/env.server.ts; never use process.env directly.
Always use Prisma findFirst instead of findUnique.
Always use the $transaction helper from ~/db.server, never call prisma.$transaction or $replica.$transaction directly. Pass isolation levels as strings, use Serializable for correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.

Files:

  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • apps/webapp/app/services/queryService.server.ts
🧠 Learnings (29)
📚 Learning: 2026-05-14T14:54:39.095Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3545
File: .server-changes/agent-view-sessions.md:10-10
Timestamp: 2026-05-14T14:54:39.095Z
Learning: In the `trigger.dev` repository, do not flag inconsistent dot vs slash notation in route/path strings inside `.server-changes/*.md` files. These markdown files are consumed verbatim into the changelog, so the mixed notation (e.g., `resources.orgs.../runs.$runParam/...`) is intentional and should be preserved as-is.

Applied to files:

  • .server-changes/queues-page-environment-wide-charts.md
📚 Learning: 2026-07-26T13:14:02.968Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4378
File: .server-changes/realtime-run-reads-from-primary.md:0-0
Timestamp: 2026-07-26T13:14:02.968Z
Learning: For files in the .server-changes directory, the body text is published verbatim as dashboard-facing user release notes. Write entries in terms of user-visible behavior (what users can do/see), and avoid implementation-oriented details such as environment-variable names, internal mechanisms, or configuration knobs. If you need to include operational/configuration specifics, put those details in the PR description instead of the .server-changes entry.

Applied to files:

  • .server-changes/queues-page-environment-wide-charts.md
📚 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/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.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/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.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/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.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/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-06-13T19:53:13.759Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3937
File: packages/trigger-sdk/skills/realtime-and-frontend/SKILL.md:258-260
Timestamp: 2026-06-13T19:53:13.759Z
Learning: When reviewing code that uses `trigger.dev/react-hooks`’s `useRealtimeRun`, preserve the call signature where the first argument is the full realtime handle object (not `handle.id`). This is intentional to maintain type-safety and is consistent with the official docs; do not suggest changing the first argument from the handle object to `handle.id`.

Applied to files:

  • apps/webapp/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-06-17T17:13:49.929Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3948
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions.$bulkActionParam/route.tsx:48-62
Timestamp: 2026-06-17T17:13:49.929Z
Learning: In triggerdotdev/trigger.dev, within `dashboardLoader`/`dashboardAction` (or similar context resolver code) whenever you resolve an organization ID from an organization slug for RBAC/enterprise authorization scope, always read from the primary Prisma client (`prisma`), not `$replica`. Using `$replica` can hit replica-lag and cause the RBAC lookup/authorization to run without the correct org scope (bypassing intended role enforcement). Implement the slug→org lookup with `prisma.organization.findFirst(...)` (or equivalent primary-client query) and add an inline comment documenting why the primary client is required (replica lag could lead to unscoped RBAC checks).

Applied to files:

  • apps/webapp/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.

Applied to files:

  • apps/webapp/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-05-01T15:45:08.099Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: packages/plugins/tsup.config.ts:3-3
Timestamp: 2026-05-01T15:45:08.099Z
Learning: In build/tool configuration files (e.g., tsup.config.ts, vite.config.ts, vitest.config.ts), follow the tool’s documented export pattern and use `export default defineConfig(...)` (or the equivalent documented default export). The repo-wide guideline “use named exports instead of default exports” should apply only to application code (*.{ts,tsx,js,jsx}), not to these build/tool config files—so do not flag `export default defineConfig(...)` in these config files as a violation.

Applied to files:

  • apps/webapp/vite.config.ts
📚 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/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • apps/webapp/app/services/queryService.server.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-06-25T18:21:51.905Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/invite-revoke.tsx:0-0
Timestamp: 2026-06-25T18:21:51.905Z
Learning: During the Zod v4 migration in the triggerdotdev/trigger.dev webapp, ensure any imports from `conform-to/zod` use the Zod-4 subpath: `conform-to/zod/v4` (e.g., `import { parseWithZod } from "conform-to/zod/v4"`). Do not import from the package root `conform-to/zod`, because it is the Zod 3 implementation and may load Zod-3-only symbols (e.g., `ZodBranded`, `ZodEffects`), which can throw at module load (notably with `zod4.4.3`). This should be enforced across `apps/webapp/**/*` where helpers like `parseWithZod` and `conformZodMessage` are used.

Applied to files:

  • apps/webapp/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • apps/webapp/app/services/queryService.server.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-07-03T17:10:21.498Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4148
File: apps/webapp/app/models/orgMember.server.ts:149-168
Timestamp: 2026-07-03T17:10:21.498Z
Learning: In triggerdotdev/trigger.dev, `User.email` (Prisma schema: `internal-packages/database/prisma/schema.prisma`) currently does NOT use `citext` and does NOT have a `lower(email)` functional unique index. Therefore, do not introduce Prisma queries like `where: { email: { equals: <value>, mode: "insensitive" } }` (or any case-insensitive lookup) against `User.email`, because it can force sequential scans of the `users` table under load. During review, ensure email is normalized (e.g., lowercased/trimmed) before both writes and subsequent lookups, and if true case-insensitive behavior/uniqueness is required, implement it via a separate app-wide migration (e.g., switch to `citext` and/or add a functional unique index with backfill) rather than bolting it onto individual feature PRs.

Applied to files:

  • apps/webapp/vite.config.ts
  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • apps/webapp/app/services/queryService.server.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.

Applied to files:

  • apps/webapp/vite.config.ts
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.

Applied to files:

  • apps/webapp/vite.config.ts
  • internal-packages/clickhouse/src/client/tsql.ts
  • internal-packages/tsql/src/query/time_buckets.test.ts
  • apps/webapp/app/hooks/useMetricResourceQuery.ts
  • internal-packages/tsql/src/query/printer.ts
  • apps/webapp/app/services/queryService.server.ts
  • internal-packages/tsql/src/index.ts
  • internal-packages/tsql/src/query/printer_context.ts
  • internal-packages/tsql/src/query/time_buckets.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/resources.metric.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-07-22T11:16:06.546Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 4332
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions/route.tsx:166-168
Timestamp: 2026-07-22T11:16:06.546Z
Learning: In the Trigger.dev web app, copy-interaction accessibility (keyboard and touch behavior for the copy affordance) is owned by the shared `CopyableText` `icon-right` primitive. When reviewing route-level code (e.g., admin debug panels and runs tables) that intentionally reuses this pattern, avoid suggesting divergent call-site-only accessibility fixes; instead, route any accessibility changes back to a holistic update of the `CopyableText` `icon-right` implementation so all reuse sites benefit consistently.

Applied to files:

  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/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/resources.metric.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-07-28T21:57:20.061Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 4411
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx:818-843
Timestamp: 2026-07-28T21:57:20.061Z
Learning: When using Radix UI `DialogClose` with `asChild` (e.g., Trigger.dev dashboard components), note that it injects `type="button"` into its child via `Slot`. If the child is a local `Button` that forwards its `type` prop to the native `<button>`, then placing it inside a `<form>` will *not* submit unless you explicitly set `type="submit"` (or otherwise override the injected type / wire up submission behavior). Review form actions to ensure the intended submit vs non-submit behavior is preserved.

Applied to files:

  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.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/routes/resources.metric.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-06-25T18:21:55.847Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/invite-resend.tsx:0-0
Timestamp: 2026-06-25T18:21:55.847Z
Learning: In the triggerdotdev/trigger.dev Zod 4 migration, avoid importing from the root package `conform-to/zod` in webapp code. It can resolve to the Zod 3 build and may crash at module load under Zod 4. When reviewing TypeScript/TSX files in `apps/webapp`, prefer importing from the Zod 4 subpath `conform-to/zod/v4` for Zod 4-compatible schemas/types.

Applied to files:

  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-06-25T18:21:54.729Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/confirm-basic-details.tsx:0-0
Timestamp: 2026-06-25T18:21:54.729Z
Learning: For Remix + TypeScript files that use Conform v1 (conform-to/react) and its getInputProps helper, when you intend to suppress the helper-provided default value for non-checkbox/non-radio inputs (e.g., hidden inputs managed via an explicit value prop), use the Conform v1 option key `value: false`. Do not recommend `defaultValue: false` here, because `defaultValue` is not a valid option key for these input types in Conform v1 typings.

Applied to files:

  • apps/webapp/app/routes/resources.metric.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
📚 Learning: 2026-07-03T09:41:46.517Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4131
File: internal-packages/metrics-pipeline/src/types.ts:0-0
Timestamp: 2026-07-03T09:41:46.517Z
Learning: When generating ClickHouse `UInt64`-backed ordering keys from epoch-derived values (e.g., `ms` and `seq`), avoid JS `number` arithmetic that can exceed the safe-integer range (2^53). Compute the key using `BigInt` (e.g., `BigInt(ms) * 100000n + BigInt(seq)`) and return it as a `string` (via `.toString()`) to preserve exact ordering. Ensure the corresponding Zod schema for the raw input (e.g., `QueueMetricsRawV1Input.order_key`) accepts/preserves this exact value (typically via `z.union([z.string(), z.number()]).optional()`), so callers can assign the computed value directly into the ClickHouse `UInt64` column without precision loss or misordering.

Applied to files:

  • internal-packages/clickhouse/src/client/tsql.ts
📚 Learning: 2026-05-18T14:40:02.173Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3658
File: packages/core/src/v3/realtimeStreams/manager.test.ts:1-147
Timestamp: 2026-05-18T14:40:02.173Z
Learning: In the triggerdotdev/trigger.dev repo, the policy “Never mock anything — use testcontainers instead” should only be enforced for integration tests that interact with real external services (e.g., Redis, Postgres) via actual infrastructure. For unit tests that exercise pure in-memory logic (e.g., cache semantics) it is OK to stub collaborators such as `ApiClient` using Vitest (`vi.fn()`) to assert call counts or control behavior. Do not flag `vi.fn()`-based `ApiClient` stubs in unit tests as violations of the testcontainers policy.

Applied to files:

  • internal-packages/tsql/src/query/time_buckets.test.ts
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.

Applied to files:

  • internal-packages/tsql/src/query/time_buckets.test.ts
📚 Learning: 2026-03-26T09:02:07.973Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3274
File: apps/webapp/app/services/runsReplicationService.server.ts:922-924
Timestamp: 2026-03-26T09:02:07.973Z
Learning: When parsing Trigger.dev task run annotations in server-side services, keep `TaskRun.annotations` strictly conforming to the `RunAnnotations` schema from `trigger.dev/core/v3`. If the code already uses `RunAnnotations.safeParse` (e.g., in a `#parseAnnotations` helper), treat that as intentional/necessary for atomic, schema-accurate annotation handling. Do not recommend relaxing the annotation payload schema or using a permissive “passthrough” parse path, since the annotations are expected to be written atomically in one operation and should not contain partial/legacy payloads that would require a looser parser.

Applied to files:

  • apps/webapp/app/services/queryService.server.ts
📚 Learning: 2026-05-05T09:38:02.512Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3523
File: apps/webapp/app/routes/api.v3.batches.ts:178-181
Timestamp: 2026-05-05T09:38:02.512Z
Learning: When reviewing code that catches `ServiceValidationError` in `*.server.ts` files, do not blindly forward `error.status` to HTTP responses, because SVEs may be thrown with non-default statuses (e.g., 400/500) and forwarding them can cause client-visible behavioral regressions (e.g., surfacing 500s to clients). Prefer a safe default response status of `error.status ?? 422`, but only after confirming via the reachable call graph that the caught `ServiceValidationError` instances are expected to carry those non-default statuses; otherwise, normalize to `422` to avoid unexpected client-visible 5xx behavior.

Applied to files:

  • apps/webapp/app/services/queryService.server.ts
📚 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.queues/route.tsx
📚 Learning: 2026-05-12T21:04:00.184Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx:40-42
Timestamp: 2026-05-12T21:04:00.184Z
Learning: In triggerdotdev/trigger.dev route loader implementations (Remix `route.tsx` files under `apps/webapp/app/routes/**`), follow the existing convention for missing/unauthorized environment lookups: when `findEnvironmentBySlug` (or the equivalent env resolver) returns a falsy value, handle it by throwing `new Error("Environment not found")` rather than returning a `404` `Response` (i.e., do not flag this as “missing 404 response”). Changing the error-to-404 convention is a cross-cutting refactor and should be left out of individual PRs unless the PR explicitly addresses that broader migration.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
🔇 Additional comments (12)
internal-packages/tsql/src/query/time_buckets.ts (1)

60-89: LGTM!

Also applies to: 99-101, 122-136

internal-packages/tsql/src/index.ts (1)

137-139: LGTM!

Also applies to: 570-575, 635-635

internal-packages/tsql/src/query/time_buckets.test.ts (1)

2-6: LGTM!

Also applies to: 185-226

internal-packages/tsql/src/query/printer_context.ts (1)

131-136: LGTM!

Also applies to: 152-162, 298-302, 315-316

internal-packages/clickhouse/src/client/tsql.ts (1)

117-121: LGTM!

Also applies to: 212-212

internal-packages/tsql/src/query/printer.ts (1)

601-629: LGTM!

Also applies to: 3525-3561

apps/webapp/app/services/queryService.server.ts (1)

12-13: LGTM!

Also applies to: 137-137, 161-175, 373-375

apps/webapp/app/hooks/useMetricResourceQuery.ts (1)

24-25: LGTM!

Also applies to: 61-61, 77-92, 130-130, 161-161

apps/webapp/app/routes/resources.metric.tsx (1)

55-55: LGTM!

Also applies to: 93-93, 133-133

apps/webapp/vite.config.ts (1)

62-63: LGTM!

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

393-394: LGTM!

Also applies to: 631-638, 1191-1191, 1223-1224, 1243-1288, 1306-1314, 1330-1330, 1340-1340, 1352-1357, 1367-1373, 1390-1397, 1421-1421

.server-changes/queues-page-environment-wide-charts.md (1)

1-7: LGTM!

@ericallam
ericallam marked this pull request as ready for review August 3, 2026 11:02
devin-ai-integration[bot]

This comment was marked as resolved.

createChildContext forwarded every other context-carried option, including
fillGaps, but not minBucketSeconds, so a nested query part would have computed
an unfloored bucket interval. No caller reaches this today, so it is latent
rather than broken, but it is the same omission that would have been a live bug
for fillGaps.
devin-ai-integration[bot]

This comment was marked as resolved.

…cket width

The Throttled tile's headline is a share of buckets that saw any throttling, not
a peak, so the 60-second bucket floor inflated it: a single brief throttle now
marked a whole minute instead of ten seconds. On the same seeded data it read
17% before the floor and 85% after, for identical throttle events.

The chart keeps the floor, because a readable line was the point of it. The
headline now comes from a second query at the range's natural bucket width, so
it means what its tooltip says regardless of how the plotted buckets are sized.
Tiles declare this via an optional `readout`; the other three measure peaks,
which are width-invariant for a max over gauges, so they are unchanged and issue
no extra query.

An empty query is now a no-op in useMetricResourceQuery, so the hook can be
called unconditionally for tiles that have no separate readout.
devin-ai-integration[bot]

This comment was marked as resolved.

… charts

The queue detail page has the same event-driven series as the Queues list hero
row, and the same problem: scheduling delay and throttling only have samples
when something started or was held back, so at the 10-second width a short range
picks, most buckets held nothing and the lines read as a run of zeros.

Scheduling delay (p50/p95/p99), Throttled, and the per-key mean delay on the
Concurrency keys tab now take the same 60-second floor, and the two delay charts
break where a bucket genuinely has no samples instead of drawing a zero. The
gauge charts on this page (concurrency, queue depth, keys with backlog, worst key
wait) carry forward and read correctly at any width, so they are left alone.

None of this page's charts carry a headline readout, so there is no share-of-
buckets figure here to skew the way the list page's throttled readout did.

The floor and the no-samples break are plumbed through the shared queue-metric
card, so the task detail page and run inspector can opt in later without
further changes.
coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Flooring only the sparse charts on the queue detail page broke the shared hover
crosshair. It is drawn as a recharts ReferenceLine on a category x-axis, so it
only appears where the hovered bucket timestamp exists in the other chart's own
data: a 10-second timestamp has no match in a 60-second series, so hovering
Concurrency drew nothing on Scheduling delay or Throttled. Measured before the
fix, hovering Concurrency reached 2 of the 4 other charts; now it reaches 4.

Every chart inside each ChartSyncProvider on the page therefore takes the same
floor, which is what the Queues list hero row already does. The gauges lose some
resolution (a max over a wider bucket is still the same kind of value) in
exchange for the crosshair working across the row.

Also: a chart whose every plotted value is null still had rows, so it reported
itself as having data while rendering its own no-data placeholder, leaving the
series legend stranded above it. Presence is now derived from the plotted values
rather than the row count.
devin-ai-integration[bot]

This comment was marked as resolved.

I justified giving only the Throttled tile a separate readout on the grounds that
the other three measure peaks and a max over gauges is width-invariant. That is
true of saturation and backlog, whose max of maxes is the same at any width, and
false of the p95 tile: merging quantile states over a wider bucket yields a p95
between the sub-buckets' own, so the worst p95 over the plotted 60-second buckets
can only be lower than the worst at the natural width, while the tooltip claims
it is the worst in the window.

Demonstrated in ClickHouse: two 240s samples among twenty in one 10-second
sub-bucket give a worst-of-six-sub-buckets p95 of 240,000ms and a merged
60-second p95 of 5,000ms, a 48x understatement.

The p95 tile now takes the same readout escape hatch as Throttled, so its
headline is measured at the natural width while the line keeps the floor that
makes it readable. Saturation and backlog stay as they were and issue no extra
query.

The plotted line is still a smoothed view, so a sub-minute spike above the
warning threshold can lose its warning colour on the chart even though the
headline reports it and colours itself.
devin-ai-integration[bot]

This comment was marked as resolved.

…ding

The delay and throttled readouts each issued their own request even when the
range's natural bucket width was already at or above the 60-second floor, where
the unfloored query compiles to the same SQL and returns the same rows — so two
identical queries ran per tile, and both re-ran on every poll and focus.

The decision now comes from the plotted spacing rather than from a copy of the
table's bucket thresholds: spacing wider than the floor means the range picked
it, so the chart's own rows are what the unfloored query would have returned and
they are reused. Verified at both ends — a 1-hour range still issues both
readouts, a 7-day range issues neither and the headlines are unchanged.

That leaves the extra query only on the short ranges where the floor actually
changes the buckets, which are also the cheapest ranges to scan.
devin-ai-integration[bot]

This comment was marked as resolved.

The empty-query short circuit returned before the block that resets rows and the
failure flag on a new query signature, so a caller that stopped asking kept the
last answer. On the Queues page that meant a readout which failed once on a short
range stayed hidden after widening to a range where the readout is skipped
entirely and the headline comes from the chart's own successful rows — the number
would have been missing for the rest of the session.

Reproduced by failing only the p95 readout on a 1-hour range (headline gone as
designed) and then widening to 7 days: the headline now returns at 58.3s.

Clearing rows as well as the flag, since no query means no data, and leaving
another query's rows behind is the same trap the reset below guards against.
@ericallam
ericallam merged commit 9d57aff into main Aug 3, 2026
44 of 49 checks passed
@ericallam
ericallam deleted the feature/tri-12784-queues-page-hero-charts-are-scoped-to-the-current-pages-25 branch August 3, 2026 15:19
ericallam added a commit that referenced this pull request Aug 3, 2026
## Summary

The webapp's server bundle imports `prop-types` directly, but the
package was declared only as a `devDependency`. A production install
therefore leaves it out and the built server fails to boot:

```
Failed to start server: Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'prop-types'
  imported from /triggerdotdev/apps/webapp/build/server/assets/server-build-*.js
```

Moving it to `dependencies` is the whole change.

## Why the bundle imports it

Nothing in the webapp's own code uses `prop-types` — there is no
reference to it, or to `PropTypes`, anywhere under `apps/webapp/app`. It
arrives through `recharts`, whose `react-smooth` dependency still
declares `propTypes` on its components.

That was invisible until recently. While `recharts` was resolved at
runtime, its `prop-types` import was satisfied inside `recharts`' own
dependency tree, which is production all the way down. #4486 added
`recharts` and `victory-vendor` to `ssr.noExternal` to fix a hydration
mismatch on every server-rendered chart; that inlines `react-smooth`
into the server bundle, which moves its `prop-types` import into the
webapp's own resolution scope — where the package was not available in
production.

So the bundling change was correct about *which* d3-shape build both
sides resolve, and wrong about what the production runtime would be able
to find.

## Verification

`docker/Dockerfile` builds the runtime dependencies with `pnpm install
--prod` against a `turbo prune --scope=webapp --docker` output, so I
reproduced exactly that: pruned the workspace, installed with `--prod`,
and imported `prop-types` from `apps/webapp`.

| | result |
| -- | -- |
| `main` as it stands (devDependency only) | `FAILS:
ERR_MODULE_NOT_FOUND` |
| with this change | `prop-types resolves OK` |

It resolves both as a CommonJS `require` and as an ESM `import`, which
is the form the bundle uses.

I also checked this is not one symptom of a wider problem: of the 169
bare specifier roots the server bundle imports, `prop-types` is the
**only** one that is a devDependency and not a production dependency.
The rest are node builtins or production dependencies.

The hydration fix from #4486 is unaffected — the rebuilt bundle still
carries the rounding d3-path build.

## Notes

`prop-types` is inert in production (its entry point swaps in
`factoryWithThrowingShims`), so this adds a 124 KB package that does no
work at runtime. It has to be resolvable regardless, because the import
is real.

An alternative would be adding `prop-types` to `ssr.noExternal` so it is
inlined and needs no runtime resolution. That keeps the dependency list
honest about the fact that the webapp itself does not use it, at the
cost of bundling a CommonJS package into the ESM server output. This
route is the smaller, better-understood change.

Worth following up separately: a check that every bare import in the
server bundle resolves from a production install would have caught this
before it landed. Local development installs every devDependency, so the
gap is invisible when the built server is run from a working tree.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants