Skip to content

Realtime and task run performance improvements #2158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Jun 10, 2025

Conversation

ericallam
Copy link
Member

@ericallam ericallam commented Jun 9, 2025

This PR makes a number of performance improvements using the following changes:

  • Using realtime with filtering by tags is now much more performant by adding proper indexes, as well as enforcing a createdAt filter that works with Electric. Users are also now able to subscribe to tags for runs created in the last X time period. Maximum is 24 hours.
  • Removes projectId based indexes on the TaskRun table in favor on runtimeEnvironmentId. Removed some indexes that were redundant or completely unused to make room.
  • The aggregate queries on the TaskRun table in the Task table has moved from PostgreSQL (🐢) to ClickHouse (🐇)
  • The task usage aggregate query on the Usage page is now powered by ClickHouse

We've also prepared a "next" runs list page that uses ClickHouse, so we can test against real usage before fully switching over.

Copy link

changeset-bot bot commented Jun 9, 2025

🦋 Changeset detected

Latest commit: 39e97de

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

coderabbitai bot commented Jun 9, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This change set implements a significant architectural refactor and feature expansion for task run querying, filtering, and real-time streaming within the application. The core changes include:

  • Refactoring run and task list presenters and repositories to use environment-based filtering instead of project-based, with corresponding updates to the database schema and indexes.
  • Introducing a new RunsRepository class for paginated, filterable querying of task runs from ClickHouse and Prisma, with comprehensive integration tests.
  • Adding a new NextRunListPresenter for paginated run lists, and refactoring TaskListPresenter to use a repository abstraction for metrics.
  • Enhancing ClickHouse integration with new query builders, typed query functions, and schema validation for task activity, running stats, durations, and usage.
  • Adding support for skipping columns in real-time subscriptions and streaming, with corresponding updates in API clients, React hooks, and SDKs.
  • Improving duration parsing utilities with more robust, order-independent parsing, and adding comprehensive unit tests.
  • Updating environment configuration to support ClickHouse and sharded electric origins.
  • Modifying real-time client logic to support multi-origin, caching, and dynamic filtering by creation time.
  • Removing obsolete project-based indexes and adding new environment-based indexes and GIN indexes for array fields in the database schema.
  • Adding and updating various migration scripts to reflect schema changes.
  • Making minor adjustments to component and route logic to align with new filtering and presenter interfaces.

No breaking changes to public APIs are introduced, but the filtering and querying mechanisms are now environment-centric, more modular, and extensible, with improved performance and test coverage.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25e6267 and 39e97de.

📒 Files selected for processing (1)
  • .changeset/small-birds-arrive.md (1 hunks)

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 16

🧹 Nitpick comments (16)
.claude/settings.local.json (2)

4-6: Restrict broad shell command permissions
Allowing grep:*, rg:*, and find:* without any constraints grants overly permissive shell access. Narrow the scope to only the flags and patterns you actually need, or consolidate into specific targeted entries.


9-9: Broaden web-fetch domain if needed
WebFetch(domain:clickhouse.com) may not cover subdomains (e.g. docs.clickhouse.com). If you intend to fetch from any ClickHouse endpoint, consider a wildcard like *.clickhouse.com.

internal-packages/database/prisma/migrations/20250605220110_remove_unused_completed_at_idnex/migration.sql (1)

1-1: Add IF EXISTS clause for safer migration.

Consider adding IF EXISTS to prevent potential errors if the index doesn't exist, maintaining consistency with other migrations in this PR.

-DROP INDEX CONCURRENTLY "TaskRun_completedAt_idx";
+DROP INDEX CONCURRENTLY IF EXISTS "TaskRun_completedAt_idx";
apps/webapp/app/routes/realtime.v1.runs.ts (1)

12-12: Ensure valid date format for createdAt filter.
Accepting arbitrary strings for createdAt may lead to runtime errors when parsing downstream. Consider refining the Zod schema to enforce an ISO-8601 format (e.g., z.string().datetime()) or transforming to a Date via z.coerce.date().

packages/core/src/v3/apiClient/types.ts (1)

44-45: Strengthen typing and docs for new query params.
The added createdAt?: string and skipColumns?: string[] enable filtering and bandwidth control, but consider:

  • Using the RealtimeRunSkipColumns union type instead of a plain string[] for skipColumns.
  • Adding JSDoc to clarify the expected format for createdAt (e.g., ISO-8601).
apps/webapp/app/models/task.server.ts (1)

6-6: Update the comment to reflect environment-scoped filtering.

The comment still mentions "get all task identifiers for a project" but the function now takes environmentId and filters by runtime environment.

- * @param prisma An efficient query to get all task identifiers for a project.
+ * @param prisma An efficient query to get all task identifiers for an environment.
apps/webapp/app/services/clickhouseInstance.server.ts (1)

9-13: Use consistent logging service instead of console.log.

The file uses console.log while the rest of the codebase likely uses the logger service for consistency.

Apply this diff to use consistent logging:

+ import { logger } from "~/services/logger.server";
  if (!env.CLICKHOUSE_URL) {
-   console.log("🗃️  Clickhouse service not enabled");
+   logger.info("🗃️  Clickhouse service not enabled");
    return;
  }

- console.log("🗃️  Clickhouse service enabled");
+ logger.info("🗃️  Clickhouse service enabled");
internal-packages/clickhouse/src/client/queryBuilder.ts (1)

41-46: Improve type safety for the condition parameter.

The condition parameter should have a more specific type than any to maintain type safety.

-  whereIf(condition: any, clause: string, params?: QueryParams): this {
+  whereIf(condition: unknown, clause: string, params?: QueryParams): this {
apps/webapp/app/presenters/v3/UsagePresenter.server.ts (1)

121-183: Consider extracting the common transformation logic to reduce duplication.

The transformation logic for mapping database results to TaskUsageItem objects is duplicated between the ClickHouse path (lines 139-147) and the SQL path (lines 172-180). Both apply the same transformations including adding env.CENTS_PER_RUN / 100 to the average cost.

Extract the transformation into a helper function:

+function transformTaskUsageData(item: any): TaskUsageItem {
+  return {
+    taskIdentifier: item.task_identifier || item.taskIdentifier,
+    runCount: Number(item.run_count || item.runCount),
+    averageDuration: Number(item.average_duration || item.averageDuration),
+    averageCost: Number(item.average_cost || item.averageCost) + env.CENTS_PER_RUN / 100,
+    totalDuration: Number(item.total_duration || item.totalDuration),
+    totalCost: Number(item.total_cost || item.totalCost) + Number(item.total_base_cost || item.totalBaseCost),
+  };
+}

 async function getTaskUsageByOrganization(
   organizationId: string,
   startOfMonth: Date,
   endOfMonth: Date,
   replica: PrismaClientOrTransaction
 ) {
   if (clickhouseClient) {
     const [queryError, tasks] = await clickhouseClient.taskRuns.getTaskUsageByOrganization({
       startTime: startOfMonth.getTime(),
       endTime: endOfMonth.getTime(),
       organizationId,
     });

     if (queryError) {
       throw queryError;
     }

-    return tasks
-      .map((task) => ({
-        taskIdentifier: task.task_identifier,
-        runCount: Number(task.run_count),
-        averageDuration: Number(task.average_duration),
-        averageCost: Number(task.average_cost) + env.CENTS_PER_RUN / 100,
-        totalDuration: Number(task.total_duration),
-        totalCost: Number(task.total_cost) + Number(task.total_base_cost),
-      }))
-      .sort((a, b) => b.totalCost - a.totalCost);
+    return tasks
+      .map(transformTaskUsageData)
+      .sort((a, b) => b.totalCost - a.totalCost);
   } else {
     return replica.$queryRaw<TaskUsageItem[]>`
     SELECT
       tr."taskIdentifier",
       COUNT(*) AS "runCount",
       AVG(tr."usageDurationMs") AS "averageDuration",
       SUM(tr."usageDurationMs") AS "totalDuration",
       AVG(tr."costInCents") / 100.0 AS "averageCost",
       SUM(tr."costInCents") / 100.0 AS "totalCost",
       SUM(tr."baseCostInCents") / 100.0 AS "totalBaseCost"
     FROM
         ${sqlDatabaseSchema}."TaskRun" tr
         JOIN ${sqlDatabaseSchema}."Project" pr ON pr.id = tr."projectId"
         JOIN ${sqlDatabaseSchema}."Organization" org ON org.id = pr."organizationId"
         JOIN ${sqlDatabaseSchema}."RuntimeEnvironment" env ON env."id" = tr."runtimeEnvironmentId"
     WHERE
         env.type <> 'DEVELOPMENT'
         AND tr."createdAt" > ${startOfMonth}
         AND tr."createdAt" < ${endOfMonth}
         AND org.id = ${organizationId}
     GROUP BY
         tr."taskIdentifier";
   `.then((data) => {
       return data
-        .map((item) => ({
-          taskIdentifier: item.taskIdentifier,
-          runCount: Number(item.runCount),
-          averageDuration: Number(item.averageDuration),
-          averageCost: Number(item.averageCost) + env.CENTS_PER_RUN / 100,
-          totalDuration: Number(item.totalDuration),
-          totalCost: Number(item.totalCost) + Number(item.totalBaseCost),
-        }))
+        .map(transformTaskUsageData)
         .sort((a, b) => b.totalCost - a.totalCost);
     });
   }
 }
apps/webapp/test/runsRepository.test.ts (1)

1408-1501: Add test coverage for missing scenarios.

The test suite is comprehensive but missing coverage for:

  1. Backward pagination direction (only forward is tested)
  2. The period filter option
  3. Negative test cases (e.g., empty results, invalid inputs)

Would you like me to generate additional test cases to cover these scenarios? This would ensure complete coverage of the RunsRepository functionality.

apps/webapp/app/presenters/v3/RunListPresenter.server.ts (1)

42-64: Breaking change: Method signature updated to accept environmentId

The call method now requires environmentId as the first parameter. This is a significant architectural change that aligns with the shift to environment-scoped queries.

Ensure all callers of this method have been updated to pass the environmentId parameter.

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

131-135: Enhance error handling with logging

The error handling could be improved by utilizing the logger provided in options.

Consider adding error logging:

const [queryError, result] = await queryBuilder.execute();

if (queryError) {
+  this.options.logger?.error("Failed to query runs from ClickHouse", { 
+    error: queryError,
+    options 
+  });
  throw queryError;
}
apps/webapp/app/presenters/v3/TaskListPresenter.server.ts (1)

34-98: Smart performance optimization with deferred loading

The method correctly implements deferred loading by returning promises without awaiting them. The comment on lines 76-77 clearly explains this design decision.

Consider documenting how consumers should handle potential errors from these deferred promises.

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

189-225: Smart createdAt filter calculation with proper boundaries

The implementation correctly:

  • Enforces a 7-day maximum age limit
  • Handles both new filters and cached filters
  • Provides sensible defaults

Consider extracting the magic number "24h" to a constant for maintainability.

apps/webapp/app/services/environmentMetricsRepository.server.ts (2)

67-67: Use lowercase primitive types for consistency.

TypeScript convention is to use lowercase primitive types instead of their object wrapper types.

Apply these changes throughout the file:

-        count: BigInt;
+        count: bigint;
-        duration: Number;
+        duration: number;

Also applies to: 110-110, 152-152, 274-274, 318-318

🧰 Tools
🪛 Biome (1.9.4)

[error] 67-67: Don't use 'BigInt' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'bigint' instead

(lint/complexity/noBannedTypes)


293-296: Initialize all task run statuses to prevent undefined values.

Currently only COMPLETED_SUCCESSFULLY is initialized to 0, but other statuses remain undefined. This could cause issues when accessing status counts.

Initialize all possible task run statuses:

existingTask.push({
  day: day.toISOString(),
-  ["COMPLETED_SUCCESSFULLY"]: 0,
+  PENDING: 0,
+  WAITING_FOR_DEPLOY: 0,
+  WAITING_TO_RESUME: 0,
+  QUEUED: 0,
+  EXECUTING: 0,
+  COMPLETED_SUCCESSFULLY: 0,
+  COMPLETED_WITH_ERRORS: 0,
+  SYSTEM_FAILURE: 0,
+  CRASHED: 0,
+  INTERRUPTED: 0,
+  CANCELED: 0,
+  EXPIRED: 0,
} as { day: string } & Record<TaskRunStatus, number>);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e617c14 and d4c6d06.

⛔ Files ignored due to path filters (4)
  • internal-packages/clickhouse/src/__snapshots__/taskRuns.test.ts.snap is excluded by !**/*.snap
  • references/d3-chat/src/components/chat-container.tsx is excluded by !references/**
  • references/d3-chat/src/trigger/chat.ts is excluded by !references/**
  • references/hello-world/src/trigger/realtime.ts is excluded by !references/**
📒 Files selected for processing (56)
  • .claude/settings.local.json (1 hunks)
  • apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (1 hunks)
  • apps/webapp/app/env.server.ts (2 hunks)
  • apps/webapp/app/models/runtimeEnvironment.server.ts (1 hunks)
  • apps/webapp/app/models/task.server.ts (2 hunks)
  • apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (4 hunks)
  • apps/webapp/app/presenters/v3/NextRunListPresenter.server.ts (1 hunks)
  • apps/webapp/app/presenters/v3/RunListPresenter.server.ts (9 hunks)
  • apps/webapp/app/presenters/v3/TaskListPresenter.server.ts (2 hunks)
  • apps/webapp/app/presenters/v3/UsagePresenter.server.ts (2 hunks)
  • apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts (3 hunks)
  • apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts (1 hunks)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsx (2 hunks)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.next.runs._index/route.tsx (2 hunks)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx (1 hunks)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules.$scheduleParam/route.tsx (3 hunks)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens.$waitpointParam/route.tsx (0 hunks)
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts (1 hunks)
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts (1 hunks)
  • apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts (1 hunks)
  • apps/webapp/app/routes/realtime.v1.runs.ts (1 hunks)
  • apps/webapp/app/services/clickhouseInstance.server.ts (1 hunks)
  • apps/webapp/app/services/environmentMetricsRepository.server.ts (1 hunks)
  • apps/webapp/app/services/realtimeClient.server.ts (10 hunks)
  • apps/webapp/app/services/realtimeClientGlobal.server.ts (1 hunks)
  • apps/webapp/app/services/runsRepository.server.ts (1 hunks)
  • apps/webapp/test/runsRepository.test.ts (1 hunks)
  • apps/webapp/test/utils/replicationUtils.ts (1 hunks)
  • internal-packages/clickhouse/src/client/client.ts (4 hunks)
  • internal-packages/clickhouse/src/client/noop.ts (1 hunks)
  • internal-packages/clickhouse/src/client/queryBuilder.ts (1 hunks)
  • internal-packages/clickhouse/src/client/types.ts (3 hunks)
  • internal-packages/clickhouse/src/index.ts (2 hunks)
  • internal-packages/clickhouse/src/taskRuns.test.ts (2 hunks)
  • internal-packages/clickhouse/src/taskRuns.ts (2 hunks)
  • internal-packages/database/prisma/migrations/20250605220110_remove_unused_completed_at_idnex/migration.sql (1 hunks)
  • internal-packages/database/prisma/migrations/20250605220552_remove_unused_schedule_instance_id_index/migration.sql (1 hunks)
  • internal-packages/database/prisma/migrations/20250606131749_remove_task_run_project_id_index/migration.sql (1 hunks)
  • internal-packages/database/prisma/migrations/20250606132144_remove_task_run_project_id_task_identifier_index/migration.sql (1 hunks)
  • internal-packages/database/prisma/migrations/20250606132316_remove_task_run_project_id_status_index/migration.sql (1 hunks)
  • internal-packages/database/prisma/migrations/20250606132630_remove_task_run_project_id_task_identifier_status_index/migration.sql (1 hunks)
  • internal-packages/database/prisma/migrations/20250606132928_remove_project_id_created_at_task_identifier_index/migration.sql (1 hunks)
  • internal-packages/database/prisma/migrations/20250606133133_remove_task_run_project_id_id_desc_index/migration.sql (1 hunks)
  • internal-packages/database/prisma/migrations/20250609163214_add_task_run_environment_created_at_id_index/migration.sql (1 hunks)
  • internal-packages/database/prisma/migrations/20250609164201_add_task_run_run_tags_gin_index/migration.sql (1 hunks)
  • internal-packages/database/prisma/schema.prisma (1 hunks)
  • packages/core/src/v3/apiClient/index.ts (5 hunks)
  • packages/core/src/v3/apiClient/runStream.ts (3 hunks)
  • packages/core/src/v3/apiClient/stream.ts (1 hunks)
  • packages/core/src/v3/apiClient/types.ts (1 hunks)
  • packages/core/src/v3/isomorphic/duration.ts (1 hunks)
  • packages/core/src/v3/schemas/api.ts (1 hunks)
  • packages/core/test/duration.test.ts (1 hunks)
  • packages/react-hooks/src/hooks/useRealtime.ts (12 hunks)
  • packages/react-hooks/src/hooks/useTaskTrigger.ts (2 hunks)
  • packages/trigger-sdk/src/v3/runs.ts (4 hunks)
💤 Files with no reviewable changes (1)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens.$waitpointParam/route.tsx
🧰 Additional context used
🧬 Code Graph Analysis (14)
packages/react-hooks/src/hooks/useTaskTrigger.ts (1)
packages/core/src/v3/apiClient/runStream.ts (1)
  • RealtimeRunSkipColumns (58-77)
internal-packages/clickhouse/src/client/noop.ts (2)
internal-packages/clickhouse/src/client/types.ts (3)
  • ClickhouseReader (20-75)
  • ClickhouseWriter (85-94)
  • ClickhouseQueryBuilderFunction (16-18)
internal-packages/clickhouse/src/client/queryBuilder.ts (1)
  • ClickhouseQueryBuilder (7-93)
internal-packages/clickhouse/src/client/client.ts (2)
internal-packages/clickhouse/src/client/types.ts (1)
  • ClickhouseQueryBuilderFunction (16-18)
internal-packages/clickhouse/src/client/queryBuilder.ts (1)
  • ClickhouseQueryBuilder (7-93)
internal-packages/clickhouse/src/index.ts (1)
internal-packages/clickhouse/src/taskRuns.ts (5)
  • getTaskRunsQueryBuilder (97-104)
  • getTaskActivityQueryBuilder (120-147)
  • getCurrentRunningStats (162-186)
  • getAverageDurations (200-222)
  • getTaskUsageByOrganization (240-268)
internal-packages/clickhouse/src/taskRuns.test.ts (3)
internal-packages/testcontainers/src/index.ts (1)
  • clickhouseTest (219-223)
internal-packages/clickhouse/src/client/client.ts (2)
  • insert (231-310)
  • queryBuilder (218-229)
internal-packages/clickhouse/src/taskRuns.ts (2)
  • insertTaskRuns (51-65)
  • getTaskRunsQueryBuilder (97-104)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsx (2)
apps/webapp/app/presenters/v3/TaskListPresenter.server.ts (1)
  • taskListPresenter (100-100)
apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts (1)
  • environment (2169-2192)
apps/webapp/app/services/realtimeClientGlobal.server.ts (2)
apps/webapp/app/env.server.ts (1)
  • env (762-762)
apps/webapp/app/services/realtimeClient.server.ts (1)
  • RealtimeClient (69-526)
internal-packages/clickhouse/src/client/types.ts (1)
internal-packages/clickhouse/src/client/queryBuilder.ts (1)
  • ClickhouseQueryBuilder (7-93)
apps/webapp/test/runsRepository.test.ts (3)
internal-packages/testcontainers/src/index.ts (2)
  • containerTest (233-241)
  • postgresContainer (78-89)
apps/webapp/test/utils/replicationUtils.ts (1)
  • setupClickhouseReplication (7-54)
apps/webapp/app/services/runsRepository.server.ts (1)
  • RunsRepository (40-226)
apps/webapp/app/presenters/v3/UsagePresenter.server.ts (3)
internal-packages/clickhouse/src/taskRuns.ts (1)
  • getTaskUsageByOrganization (240-268)
apps/webapp/app/services/clickhouseInstance.server.ts (1)
  • clickhouseClient (5-5)
apps/webapp/app/db.server.ts (1)
  • sqlDatabaseSchema (252-252)
packages/core/test/duration.test.ts (1)
packages/core/src/v3/isomorphic/duration.ts (5)
  • parseNaturalLanguageDuration (1-63)
  • safeParseNaturalLanguageDuration (65-71)
  • parseNaturalLanguageDurationAgo (75-137)
  • safeParseNaturalLanguageDurationAgo (139-145)
  • stringifyDuration (147-167)
apps/webapp/app/presenters/v3/TaskListPresenter.server.ts (4)
apps/webapp/app/services/environmentMetricsRepository.server.ts (6)
  • DailyTaskActivity (7-7)
  • EnvironmentMetricsRepository (11-29)
  • CurrentRunningStats (8-8)
  • AverageDurations (9-9)
  • ClickHouseEnvironmentMetricsRepository (176-268)
  • PostgrestEnvironmentMetricsRepository (38-170)
apps/webapp/app/v3/models/workerDeployment.server.ts (1)
  • findCurrentWorkerFromEnvironment (197-223)
apps/webapp/app/services/clickhouseInstance.server.ts (1)
  • clickhouseClient (5-5)
apps/webapp/app/db.server.ts (1)
  • $replica (102-105)
packages/react-hooks/src/hooks/useRealtime.ts (3)
packages/core/src/v3/apiClient/runStream.ts (2)
  • RealtimeRunSkipColumns (58-77)
  • RealtimeRun (55-55)
packages/core/src/v3/apiClient/index.ts (3)
  • RealtimeRunSkipColumns (92-92)
  • ApiClient (138-1058)
  • RealtimeRun (126-126)
packages/core/src/v3/types/tasks.ts (2)
  • AnyTask (628-628)
  • InferRunTypes (904-914)
internal-packages/clickhouse/src/taskRuns.ts (1)
internal-packages/clickhouse/src/client/types.ts (1)
  • ClickhouseReader (20-75)
🪛 Biome (1.9.4)
apps/webapp/app/presenters/v3/NextRunListPresenter.server.ts

[error] 166-166: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

apps/webapp/app/services/environmentMetricsRepository.server.ts

[error] 67-67: Don't use 'BigInt' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'bigint' instead

(lint/complexity/noBannedTypes)


[error] 110-110: Don't use 'BigInt' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'bigint' instead

(lint/complexity/noBannedTypes)


[error] 152-152: Don't use 'Number' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'number' instead

(lint/complexity/noBannedTypes)


[error] 274-274: Don't use 'BigInt' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'bigint' instead

(lint/complexity/noBannedTypes)


[error] 318-318: Don't use 'BigInt' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'bigint' instead

(lint/complexity/noBannedTypes)

⏰ Context from checks skipped due to timeout of 90000ms (25)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (90)
internal-packages/database/prisma/migrations/20250606131749_remove_task_run_project_id_index/migration.sql (1)

2-2: Verify replacement index exists before removal
Before dropping "TaskRun_projectId_idx", ensure that the new environment-scoped index on runtimeEnvironmentId (and any composite ordering/index on createdAt or id) has been created in an earlier migration. This prevents query slowdowns once the project‐based index is removed.

internal-packages/database/prisma/migrations/20250606133133_remove_task_run_project_id_id_desc_index/migration.sql (1)

2-2: Confirm prior creation of new environment-based index
Dropping "TaskRun_projectId_id_idx" concurrently is safe, but double-check that the intended replacement index on (runtimeEnvironmentId, createdAt DESC, id DESC) is already in place.

internal-packages/database/prisma/migrations/20250606132144_remove_task_run_project_id_task_identifier_index/migration.sql (1)

2-2: Ensure new lookup index covers taskIdentifier
Before removing "TaskRun_projectId_taskIdentifier_idx", verify that any new environment-scoped index supports efficient filtering by taskIdentifier (if still needed), or that queries have been updated accordingly.

internal-packages/database/prisma/migrations/20250605220552_remove_unused_schedule_instance_id_index/migration.sql (1)

2-2: Validate scheduling index replacement
The removal of "TaskRun_scheduleInstanceId_idx" should follow the creation of any required environment-scoped schedule index (e.g., on runtimeEnvironmentId plus scheduleInstanceId if still used). Confirm that query paths remain covered.

internal-packages/database/prisma/migrations/20250606132316_remove_task_run_project_id_status_index/migration.sql (1)

2-2: Check that status filtering is supported post-drop
Dropping "TaskRun_projectId_status_idx" is correct for the new environment approach, but ensure that status-based queries now leverage a replacement environment-scoped index or an alternative strategy.

internal-packages/database/prisma/migrations/20250606132630_remove_task_run_project_id_task_identifier_status_index/migration.sql (1)

1-2: LGTM! Safe migration approach.

The migration correctly uses CONCURRENTLY to avoid blocking operations and IF EXISTS to handle cases where the index might not exist. This aligns well with the broader refactoring from project-based to environment-based indexing.

apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts (1)

62-62: LGTM! Consistent environment scoping.

The addition of environmentId parameter correctly uses the authenticated environment context and aligns with the broader refactoring to environment-based operations.

apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts (1)

61-61: LGTM! Consistent with activate route.

The environment ID parameter addition follows the same correct pattern as the activate route, maintaining consistency across schedule operations.

internal-packages/database/prisma/migrations/20250606132928_remove_project_id_created_at_task_identifier_index/migration.sql (1)

1-2: LGTM! Safe index removal.

The migration correctly uses CONCURRENTLY to perform non-blocking index removal and IF EXISTS to handle cases where the index may not exist. This aligns with the architectural shift to environment-scoped filtering.

apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts (1)

143-143: LGTM! Consistent with environment-scoped refactoring.

The addition of environmentId parameter correctly passes the authenticated environment's ID to the presenter, aligning with the broader architectural shift to environment-scoped filtering.

internal-packages/database/prisma/migrations/20250609163214_add_task_run_environment_created_at_id_index/migration.sql (1)

1-2: LGTM! Well-designed composite index for environment-scoped queries.

The index effectively supports environment-scoped task run queries with proper descending order on createdAt and id for efficient pagination. The use of CONCURRENTLY and IF NOT EXISTS follows best practices for safe index creation.

apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (1)

542-542: Remove environments destructuring in BlankState.
Updating the destructuring to only extract tasks, from, to, and otherFilters aligns this component with the shift to a single-environment context.

packages/react-hooks/src/hooks/useTaskTrigger.ts (2)

12-12: Import RealtimeRunSkipColumns for skipColumns support.
The addition of the RealtimeRunSkipColumns import correctly prepares the hook to expose the new skipColumns option.


117-124: Add optional skipColumns to task trigger options.
Introducing skipColumns?: RealtimeRunSkipColumns in UseRealtimeTaskTriggerOptions properly extends the hook’s configurability for real-time subscriptions.

internal-packages/database/prisma/migrations/20250609164201_add_task_run_run_tags_gin_index/migration.sql (1)

1-1: Add GIN index on runTags.
Creating the concurrent GIN index with IF NOT EXISTS on TaskRun.runTags improves tag-based query performance without blocking writes.

apps/webapp/app/models/task.server.ts (1)

10-10: LGTM! Parameter and query changes are consistent.

The function signature and SQL query have been properly updated together to support environment-scoped filtering instead of project-scoped filtering.

Also applies to: 19-19

packages/core/src/v3/apiClient/stream.ts (2)

105-109: LGTM! Clean refactoring to more concise subscription call.

The subscription logic remains the same while being more readable and concise.


114-114: LGTM! Minor formatting improvement.

apps/webapp/app/services/realtimeClientGlobal.server.ts (2)

7-7: LGTM! Clean implementation of electric origin sharding support.

The logic correctly checks for ELECTRIC_ORIGIN_SHARDS, splits by comma to create an array, and falls back to the single ELECTRIC_ORIGIN when sharding is not configured.


10-10: LGTM! Simplified variable assignment.

Removing the redundant variable assignment improves code clarity.

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

78-78: LGTM! Added import for singleton presenter.


127-130: LGTM! Proper migration to singleton pattern and environment-scoped parameters.

The changes correctly:

  • Replace direct instantiation with the singleton taskListPresenter
  • Switch from projectId to environmentType parameter, aligning with the environment-scoped architecture

This improves performance by reusing the singleton instance and maintains consistency with the broader codebase refactoring.

apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts (1)

81-85: LGTM! Consistent with environment-scoped refactoring.

The change correctly passes environmentId as the first argument to runPresenter.call, aligning with the broader refactoring to shift from project-scoped to environment-scoped filtering. The parameter passing is consistent and the environmentId is properly available from the method signature.

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

11-11: LGTM! Type definition updated correctly.

The ViewScheduleOptions type is properly extended with the required environmentId property, consistent with the environment-scoped refactoring.


21-21: LGTM! Method signature updated consistently.

The method signature correctly includes the new environmentId parameter, maintaining consistency with the updated type definition.


80-84: LGTM! Presenter call updated correctly.

The runPresenter.call is properly updated to pass environmentId as the first argument, aligning with the new environment-scoped filtering approach. The parameter destructuring and passing is correctly implemented.

internal-packages/clickhouse/src/client/noop.ts (2)

3-3: LGTM! Imports added correctly.

The necessary imports for ClickhouseQueryBuilderFunction and ClickhouseQueryBuilder are properly added to support the new queryBuilder method.

Also applies to: 7-7


14-22: LGTM! QueryBuilder method implemented correctly.

The queryBuilder method properly implements the ClickhouseReader interface. The implementation:

  • Uses the correct method signature matching the interface
  • Returns a function that creates a new ClickhouseQueryBuilder instance
  • Passes all required parameters (name, baseQuery, this, schema, settings) to the constructor
  • Provides appropriate no-op functionality for testing scenarios

This aligns well with the broader ClickHouse query builder ecosystem introduced in this PR.

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.next.runs._index/route.tsx (2)

37-37: LGTM! Imports updated correctly.

The necessary imports for $replica, NextRunListPresenter, and clickhouseClient are properly added to support the new ClickHouse-backed run listing functionality.

Also applies to: 43-44


130-131: LGTM! Presenter instantiation and call updated correctly.

The NextRunListPresenter is properly instantiated with both $replica and clickhouseClient parameters, and the call method correctly passes environment.id as the first argument, consistent with the environment-scoped refactoring pattern.

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules.$scheduleParam/route.tsx (2)

48-48: LGTM: Import addition aligns with environment-scoped architecture.

The import of findEnvironmentBySlug is correctly added to support the new environment lookup functionality.


63-64: LGTM: Parameter parsing correctly updated for environment scoping.

The addition of envParam parsing aligns with the shift to environment-scoped data access.

apps/webapp/app/models/runtimeEnvironment.server.ts (1)

279-310: LGTM: Well-implemented environment retrieval function.

The findDisplayableEnvironment function follows established patterns in the codebase:

  • Proper database query with limited selection for performance
  • Consistent return type (undefined instead of null)
  • Reuses existing displayableEnvironment logic for consistency
  • Clear separation of concerns

This supports the broader architectural shift to environment-scoped data access.

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

5-30: LGTM: Well-structured ClickHouse client singleton.

The implementation demonstrates good practices:

  • Proper singleton pattern using the utility function
  • Conditional initialization based on environment configuration
  • Comprehensive ClickHouse client configuration with all necessary parameters
  • Clean separation of initialization logic

This establishes a solid foundation for ClickHouse integration across the application.

internal-packages/clickhouse/src/taskRuns.test.ts (2)

4-4: LGTM: Import correctly added for new query builder functionality.

The import of getTaskRunsQueryBuilder supports the new test case and aligns with the enhanced ClickHouse query capabilities.


254-346: LGTM: Comprehensive test coverage for query builder functionality.

The test case demonstrates excellent testing practices:

  • Tests both query building (build()) and execution (execute())
  • Uses snapshot testing to validate generated query structure
  • Covers conditional query building with whereIf
  • Tests both positive cases (finding results) and negative cases (empty results)
  • Proper test data setup with realistic values

This provides solid coverage for the new ClickHouse query builder infrastructure.

internal-packages/clickhouse/src/index.ts (2)

5-13: LGTM! Clean integration of query builder imports.

The new imports properly extend the ClickHouse API with query builder functionality while maintaining consistency with existing patterns.


146-150: Excellent exposure of query builder methods.

The new query builder functions are properly exposed through the taskRuns getter, maintaining consistency with existing patterns by using this.reader for all read operations.

internal-packages/clickhouse/src/client/types.ts (2)

16-18: Well-defined type for query builder function.

The ClickhouseQueryBuilderFunction type properly encapsulates the optional settings parameter pattern and maintains type safety with the generic TOutput parameter.


50-72: Excellent interface extension for query builder functionality.

The queryBuilder method signature is well-designed with:

  • Clear documentation following established patterns
  • Proper generic typing with Zod schema constraints
  • Consistent parameter structure with the existing query method
  • Good example in the baseQuery documentation
apps/webapp/test/utils/replicationUtils.ts (2)

45-49: Proper cleanup pattern with afterEach hook.

The cleanup implementation correctly stops the replication service after each test, preventing resource leaks and ensuring test isolation.


20-26: Appropriate ClickHouse configuration for testing.

The ClickHouse client setup with compression enabled and a descriptive service name is well-suited for test environments.

internal-packages/clickhouse/src/client/client.ts (2)

107-113: Useful debug logging enhancement.

The added debug logging provides comprehensive information for troubleshooting, including normalized query strings, parameters, and settings. This will be valuable for debugging ClickHouse query issues.


218-229: Solid implementation of query builder method.

The queryBuilder method implementation is well-designed:

  • Proper settings merging with spread operator precedence
  • Consistent parameter structure matching the interface
  • Type safety maintained through generic constraints
  • Clean factory function pattern returning configured builder instances
apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (3)

158-161: LGTM! Proper validation ensures environment exists.

The validation logic correctly throws a ServiceValidationError when no environment is found, preventing downstream issues with undefined environment IDs.


138-161:

❓ Verification inconclusive

Environment filtering logic change may introduce breaking behavior.

The refactor correctly implements environment-scoped filtering, but when multiple environments are specified in filter[env], only the first one is used (environments.at(0)?.id). This could be a breaking change if the API previously supported filtering across multiple environments.

Verify if this breaking change is intentional by checking how multiple environment filters were previously handled:


🏁 Script executed:

#!/bin/bash
# Search for previous implementations of environment filtering in run list presenters
rg -A 10 -B 5 "filter.*env.*environments" --type ts

Length of output: 53


Ensure multi-environment filtering support is intentional

The refactored logic in ApiRunListPresenter.server.ts (lines 138–161) now only uses environments.at(0)?.id when multiple values are passed in filter[env], dropping support for filtering across multiple environments. I couldn’t find any prior implementation of multi-environment filtering in the codebase—please confirm whether this narrowing to a single environment is an intentional breaking change or if we need to restore full multi-environment support.

• File: apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (lines 138–161)


212-212:

✅ Verification successful

Verify the RunListPresenter interface change.

The call signature change to pass environmentId as the first parameter aligns with the described refactor, but ensure all callers of RunListPresenter.call have been updated consistently.


🏁 Script executed:

#!/bin/bash
# Search for other usages of RunListPresenter.call to verify consistent updates
ast-grep --pattern 'RunListPresenter.call($$$)'

Length of output: 49


I’ll search broadly for any *.call invocations on presenters to ensure nothing was missed:


🏁 Script executed:

#!/bin/bash
# 1. Look for direct RunListPresenter.call usages
rg -n "RunListPresenter\.call" -t ts || true

# 2. Find all .call invocations on presenter instances
rg -n "presenter\.call" -t ts || true

Length of output: 6774


I’ll verify where ApiRunListPresenter is referenced and how its .call is invoked:


🏁 Script executed:

#!/bin/bash
# Find all references to ApiRunListPresenter and show surrounding context
rg -n "ApiRunListPresenter" -C3 -t ts

Length of output: 4069


I’ll locate all definitions and usages of RunListPresenter to confirm its .call signature and callers:


🏁 Script executed:

#!/bin/bash
# 1. Find any files named RunListPresenter.*
fd RunListPresenter -e ts || true

# 2. Show the class definition(s) for RunListPresenter
rg -n "class RunListPresenter" -C3 -t ts || true

# 3. Find all instantiations of RunListPresenter
rg -n "new RunListPresenter" -C3 -t ts || true

# 4. Verify all .call invocations on those instances
rg -n "RunListPresenter\.call" -C2 -t ts || true
rg -n "presenter\.call" -C2 -t ts | grep -B1 "new RunListPresenter" || true

Length of output: 4362


All RunListPresenter.call usages now pass environmentId first

I’ve verified every instantiation of RunListPresenter and its .call(…) invocations; all four call sites have been updated to the new signature:

• apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (line 212)
• apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts (line 81)
• apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts (line 80)
• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx (line 124)

No remaining callers of RunListPresenter.call were found—this change is complete.

packages/core/src/v3/apiClient/runStream.ts (2)

58-77: Well-defined type for selective column skipping.

The RealtimeRunSkipColumns type provides a clear enumeration of columns that can be skipped in real-time subscriptions, enabling performance optimizations for different use cases.


423-438: Improved data transformation with consistent defaults.

The nullish coalescing operators (??) ensure that numeric fields always have defined values (0) and boolean fields have sensible defaults (false for isTest). This prevents undefined values from breaking downstream consumers of the real-time stream data.

packages/core/src/v3/schemas/api.ts (1)

873-888: Schema improvements align with real-time stream enhancements.

The reordering of fields and addition of default values ensures consistent data shapes when columns may be skipped in real-time streams. The defaults (0 for numeric fields, false for isTest) match those used in the transformRunShape method, maintaining consistency across the data pipeline.

internal-packages/database/prisma/schema.prisma (1)

684-685: Excellent index optimization for environment-scoped queries.

These new indexes strategically support the transition from project-based to environment-based filtering:

  1. Line 684: The composite index [runtimeEnvironmentId, createdAt(sort: Desc), id(sort: Desc)] optimizes environment-scoped queries with time-based ordering and provides consistent pagination through the ID tiebreaker.

  2. Line 685: The GIN index [runTags(ops: ArrayOps)] enables efficient tag-based filtering on the runTags array field, which aligns with the new filtering capabilities being added across the codebase.

These indexes will significantly improve query performance for the new environment-centric data access patterns.

packages/trigger-sdk/src/v3/runs.ts (5)

18-18: Good addition of the RealtimeRunSkipColumns import.

This import supports the new column skipping functionality being added to real-time subscriptions.


350-361: Well-designed skipColumns option with clear documentation.

The addition of the skipColumns property to SubscribeToRunOptions is implemented well:

  • Type safety: Uses the imported RealtimeRunSkipColumns type
  • Optional parameter: Maintains backward compatibility
  • Clear documentation: Includes usage example and default value
  • Performance benefit: Allows clients to exclude unnecessary columns from real-time streams

This will help optimize real-time subscription performance by reducing payload size.


405-405: Correct forwarding of skipColumns option.

The skipColumns option is properly passed through to the API client, maintaining the chain of configuration from the SDK interface to the underlying implementation.


409-433: Excellent design of the SubscribeToRunsFilterOptions interface.

The new filtering options are well-structured:

  • Duration-based filtering: The createdAt property uses intuitive duration strings like "1h", "30m"
  • Clear constraints: Maximum 1 week duration is documented
  • Server-side calculation: Timestamp calculation on server prevents client-server time sync issues
  • Consistent API: Uses the same skipColumns type as other subscription methods

The documentation clearly explains the expected format and behavior.


466-475: Clean API evolution with backward compatibility.

The updated subscribeToRunsWithTag function signature maintains backward compatibility while adding powerful new filtering capabilities:

  • Optional parameters: Both filters and options are optional
  • Proper option forwarding: Signal handling is correctly preserved
  • Type safety: Generic type parameter is properly propagated

The implementation correctly forwards all options to the underlying API client.

apps/webapp/app/env.server.ts (2)

38-40: Well-documented addition for electric origin sharding.

The ELECTRIC_ORIGIN_SHARDS environment variable is properly implemented:

  • Clear documentation: Comments explain the purpose and provide a concrete example
  • Optional configuration: Uses .optional() allowing for single-origin setups
  • Comma-separated format: Standard approach for multiple values in environment variables

This supports the multi-origin electric endpoint functionality for improved scalability and load distribution.


744-750: Comprehensive Clickhouse configuration options.

The Clickhouse environment variables provide thorough configuration control:

  • Connection management: URL, keep-alive settings, and connection pooling
  • Performance tuning: Max connections and compression settings
  • Observability: Configurable log levels for debugging
  • Sensible defaults: All variables have appropriate default values
  • Consistent patterns: Follows the same naming and typing conventions as other database configurations

These variables properly support the new Clickhouse integration capabilities mentioned in the summary.

packages/react-hooks/src/hooks/useRealtime.ts (5)

3-9: LGTM!

The addition of RealtimeRunSkipColumns to the imports is appropriate for the new column skipping functionality.


39-44: Well-documented optional property addition.

The skipColumns option is properly typed and documented with a sensible default value.


352-376: Excellent type definition with comprehensive documentation.

The UseRealtimeRunsWithTagOptions type is well-structured with clear examples for the createdAt duration format and proper documentation about server-side timestamp calculation.


115-117: Consistent propagation of filtering options.

The skipColumns and createdAt filters are properly propagated through all the processing functions using a clean filters object pattern.

Also applies to: 276-278, 440-442, 613-622, 656-672, 717-730


392-393: Helpful documentation update.

The example clearly demonstrates how to use the new createdAt filter option.

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

165-180: Code is already null-safe.

The schedule ID lookup logic correctly handles the case where no schedule is found, using optional chaining on line 178.

🧰 Tools
🪛 Biome (1.9.4)

[error] 166-166: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


114-118: Efficient concurrent data fetching.

Good use of Promise.all to fetch independent data concurrently, improving performance.


229-293: Well-structured response mapping.

The return object properly handles optional fields, sorts collections for consistency, and provides a clear pagination structure.

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

7-32: Well-designed query builder with fluent interface.

The class follows good design patterns with proper encapsulation, type safety through generics, and Zod schema validation.

apps/webapp/app/presenters/v3/UsagePresenter.server.ts (1)

143-143: Verify the average cost calculation to avoid potential double counting.

The code adds env.CENTS_PER_RUN / 100 to the average cost after retrieving it from the database. This could lead to double counting if the database already includes the per-run cost in the cost_in_cents calculation.

Please verify whether the cost_in_cents field in both ClickHouse and PostgreSQL already includes the base per-run cost. If it does, this addition would result in incorrect cost calculations.

Also applies to: 176-176

packages/core/src/v3/isomorphic/duration.ts (1)

3-14: Well-implemented flexible duration parsing!

The refactoring to support duration units in any order (e.g., "1h30m" or "30m1h") is a great improvement. The validation pattern ensures the entire string consists only of valid duration units, preventing partial matches.

apps/webapp/test/runsRepository.test.ts (1)

8-1501: Excellent test coverage and structure!

The test suite provides comprehensive coverage of the RunsRepository filtering capabilities with well-organized, focused tests. The use of containerTest for integration testing with real ClickHouse and PostgreSQL instances ensures the tests accurately reflect production behavior.

packages/core/src/v3/apiClient/index.ts (5)

69-69: LGTM!

The import and re-export of RealtimeRunSkipColumns type are correctly implemented.

Also applies to: 92-92


895-914: LGTM!

The skipColumns implementation in subscribeToRun is well-structured and maintains backward compatibility.


919-925: LGTM!

The skipColumns support in subscribeToRunsWithTag is consistent with the other subscription methods.


942-963: LGTM!

The skipColumns implementation in subscribeToBatch follows the established pattern correctly.


1075-1081: LGTM!

The query parameter handling for createdAt and skipColumns is implemented correctly.

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

107-115: LGTM!

The environment validation logic properly ensures the environment exists and the user has access before proceeding.


218-218: Good catch on field name correction

Using taskVersion instead of the deprecated version field name is the correct approach.

apps/webapp/app/services/runsRepository.server.ts (3)

44-112: Excellent query builder implementation

The query construction is well-structured with proper parameter binding and type safety. Good use of ClickHouse-specific functions like hasAny for array operations and fromUnixTimestamp64Milli for timestamp conversions.


179-224: LGTM!

The two-phase query approach (ClickHouse for filtering/pagination, Prisma for detailed data) is an excellent architectural choice for performance.


145-176:

❌ Incorrect review comment

Review backward pagination logic

The backward pagination logic appears to have issues:

  1. When direction is "backward", we query for IDs > cursor but still order by DESC, which may not give the expected results
  2. The cursor calculations for backward pagination (lines 156-161) seem incorrect

Consider this corrected approach for backward pagination:

} else {
  queryBuilder
    .where("run_id > {runId: String}", { runId: options.page.cursor })
-   .orderBy("run_id DESC")
+   .orderBy("run_id ASC")
    .limit(options.page.size + 1);
}

Then reverse the results for backward pagination before processing cursors.


🏁 Script executed:

#!/bin/bash
# Search for similar pagination patterns in the codebase to verify the approach
rg -A 10 -B 5 "direction.*backward.*cursor" --type ts

Length of output: 55


🏁 Script executed:

#!/bin/bash
# Extract the pagination query builder and cursor logic from the file
sed -n '100,200p' apps/webapp/app/services/runsRepository.server.ts

# Locate where queryBuilder is configured for cursor-based pagination
rg -n "queryBuilder" -C5 apps/webapp/app/services/runsRepository.server.ts

Length of output: 6741


Ignore backward pagination concern

The existing code correctly uses a consistent DESC order for both “forward” and “backward” queries. By querying run_id > cursor and then .orderBy("run_id DESC"), you still get the newest‐first slice, so there’s no need to switch to ASC and reverse the array. The nextCursor/previousCursor calculations align with this approach and will produce the intended pages.

No changes required.

Likely an incorrect or invalid review comment.

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

1-27: Well-structured imports and type definitions

The imports are properly organized and the type alias for TaskActivity improves code readability.


28-33: Excellent use of dependency injection pattern

The constructor properly implements dependency injection, making the class more testable and maintainable.


100-112: Clean singleton implementation with smart fallback

The factory function properly handles the choice between ClickHouse and PostgreSQL implementations, with a sensible fallback to PostgreSQL when ClickHouse is unavailable.

packages/core/test/duration.test.ts (2)

9-126: Comprehensive test coverage for parseNaturalLanguageDuration

Excellent test coverage including:

  • Valid duration strings with all supported units
  • Edge cases (zero values, large numbers)
  • Invalid inputs (empty strings, negative numbers, invalid units)
  • Proper time mocking setup

284-374: Excellent consistency and formatting tests

The tests properly verify:

  • stringifyDuration formatting with various inputs
  • Symmetry between forward and "ago" parsers
  • Consistency between safe and unsafe versions
  • Proper handling of invalid inputs across all functions
apps/webapp/app/services/realtimeClient.server.ts (4)

18-105: Well-designed caching infrastructure with security considerations

Good implementation details:

  • Security-conscious reserved columns prevent skipping critical fields
  • Reasonable 7-day maximum age limit for createdAt filters
  • Dual-store cache (memory + Redis) provides resilience
  • Appropriate cache TTLs (1 week fresh, 2 weeks stale)

149-187: Robust implementation of createdAt filtering

The implementation properly:

  • Calculates and applies createdAt filters
  • Handles cache storage after successful responses
  • Uses non-blocking error handling to prevent request failures
  • Logs errors for monitoring

301-321: Secure column filtering implementation

Excellent security considerations:

  • Filters out reserved columns to prevent skipping critical fields
  • Properly quotes column names to prevent SQL injection
  • Maintains backward compatibility

480-488:

✅ Verification successful

Verify jumpHash implementation for consistent origin routing

The multi-origin support looks good with consistent hashing. However, please verify that the jumpHash function is properly implemented for distributed load balancing.


🏁 Script executed:

#!/bin/bash
# Description: Verify the jumpHash implementation exists and is properly exported

# Search for jumpHash implementation
ast-grep --pattern 'export function jumpHash($$$) {
  $$$
}'

# Also check for any existing tests
fd -e test.ts -e spec.ts | xargs rg -l "jumpHash" -A 5

Length of output: 153


🏁 Script executed:

#!/bin/bash
# Locate jumpHash definition and inspect its implementation

# Search for the function declaration
rg -n "function jumpHash" -A5 -B5

# Also check for any named exports if not directly exported
rg -n "export.*jumpHash" -A5 -B5

Length of output: 2336


jumpHash implementation and export verified

  • Defined in packages/core/src/v3/serverOnly/jumpHash.ts and re-exported via packages/core/src/v3/serverOnly/index.ts.
  • Behavior covered by existing tests in packages/core/test/jumpHash.test.ts.

No further action required.

internal-packages/clickhouse/src/taskRuns.ts (1)

240-268: Well-structured usage aggregation query.

The implementation correctly:

  • Filters out development environments
  • Converts between Unix timestamps and ClickHouse timestamps
  • Properly converts cents to dollars for cost calculations
  • Orders by total cost for meaningful results

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🔭 Outside diff range comments (1)
apps/webapp/app/services/realtimeClient.server.ts (1)

340-340: ⚠️ Potential issue

Inconsistency in electric origin usage.

The #constructChunksElectricUrl method still uses the old single-origin approach (this.options.electricOrigin) while #constructRunsElectricUrl uses the new multi-origin resolution. This could lead to inconsistent behavior.

Apply this fix to ensure consistent multi-origin support:

-    const electricUrl = new URL(`${this.options.electricOrigin}/v1/shape`);
+    const electricOrigin = this.#resolveElectricOrigin(environment.id);
+    const electricUrl = new URL(`${electricOrigin}/v1/shape`);

And update the method signature to accept the environment parameter:

- #constructChunksElectricUrl(url: URL | string, whereClause: string, clientVersion?: string): URL {
+ #constructChunksElectricUrl(url: URL | string, environment: RealtimeEnvironment, whereClause: string, clientVersion?: string): URL {

Then update the call site:

-    const electricUrl = this.#constructChunksElectricUrl(url, whereClause, clientVersion);
+    const electricUrl = this.#constructChunksElectricUrl(url, environment, whereClause, clientVersion);
♻️ Duplicate comments (1)
apps/webapp/test/runsRepository.test.ts (1)

64-64: Replace fixed delays with a more robust replication waiting mechanism.

This issue was already identified in previous reviews. All tests use fixed 1-second delays to wait for ClickHouse replication, which can lead to flaky tests.

Also applies to: 171-171, 281-281, 391-391, 501-501, 610-610, 703-703, 807-807, 936-936, 1043-1043, 1150-1150, 1264-1264, 1384-1384, 1467-1467

🧹 Nitpick comments (1)
apps/webapp/test/runsRepository.test.ts (1)

1-1502: Consider using constants for repeated values.

The tests contain many magic values that are repeated throughout (e.g., "V2", "DEVELOPMENT", "test", "my-task"). Extracting these to constants would improve maintainability.

+const TEST_VALUES = {
+  ENGINE: "V2" as const,
+  ENVIRONMENT_TYPE: "DEVELOPMENT" as const,
+  QUEUE: "test" as const,
+  TASK_IDENTIFIER: "my-task" as const,
+  ORGANIZATION_TITLE: "test" as const,
+  PROJECT_NAME: "test" as const,
+} as const;

Then use these constants throughout the tests instead of hardcoded strings.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d4c6d06 and eb957a6.

⛔ Files ignored due to path filters (1)
  • internal-packages/clickhouse/src/__snapshots__/taskRuns.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (5)
  • .gitignore (1 hunks)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules.$scheduleParam/route.tsx (3 hunks)
  • apps/webapp/app/services/realtimeClient.server.ts (8 hunks)
  • apps/webapp/test/runsRepository.test.ts (1 hunks)
  • packages/core/src/v3/isomorphic/duration.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules.$scheduleParam/route.tsx
  • packages/core/src/v3/isomorphic/duration.ts
⏰ Context from checks skipped due to timeout of 90000ms (25)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
apps/webapp/app/services/realtimeClient.server.ts (7)

48-49: Good defensive programming with reserved columns and search params.

The protection of reserved columns and search parameters prevents clients from accidentally breaking core functionality.


81-104: Excellent caching architecture with multi-tier storage.

The implementation using both memory and Redis cache stores with appropriate TTL values (1 week fresh, 2 weeks stale) provides good performance while ensuring data consistency across instances.


161-187: Robust createdAt filtering with proper error handling.

The implementation correctly handles both new and existing shape IDs, with appropriate error logging for cache operations. The asynchronous error handling using tryCatch is well implemented.


301-320: Solid column filtering implementation with security considerations.

The implementation correctly filters out reserved columns from the skip list and provides sensible defaults. The use of quoted column names prevents SQL injection.


474-482: Robust multi-origin resolution using consistent hashing.

The implementation using jump hash ensures consistent routing of environment IDs to electric origins, which is important for maintaining session affinity. Good fallback to the first origin if index is out of bounds.


522-528: Good backward compatibility in shape ID extraction.

The fallback from handle to shape_id ensures compatibility with older clients while supporting the new parameter naming.


52-52:

✅ Verification successful

Verify multi-origin configuration handling.

The change from single string to string array for electricOrigin introduces complexity but enables load distribution. Ensure this change is backward compatible.


🏁 Script executed:

#!/bin/bash
# Search for electricOrigin configuration usage to verify backward compatibility
rg -A 3 -B 3 "electricOrigin.*=" --type ts

Length of output: 3921


🏁 Script executed:

#!/bin/bash
rg -n "resolveElectricOrigin" -A10 -B10 apps/webapp/app/services/realtimeClient.server.ts

Length of output: 1387


✅ Multi-origin electricOrigin is backward compatible.

The #resolveElectricOrigin method in realtimeClient.server.ts handles both a single string and an array, returning the original string when passed one origin and using jump hashing (with a fallback to the first element) for arrays. No further changes are needed.

• apps/webapp/app/services/realtimeClient.server.ts (lines 474–482):

#resolveElectricOrigin(environmentId: string) {
  if (typeof this.options.electricOrigin === "string") {
    return this.options.electricOrigin;
  }
  const index = jumpHash(environmentId, this.options.electricOrigin.length);
  return this.options.electricOrigin[index] ?? this.options.electricOrigin[0];
}

• apps/webapp/app/services/realtimeClientGlobal.server.ts: initialization uses env.ELECTRIC_ORIGIN_SHARDS?.split(",") ?? env.ELECTRIC_ORIGIN

apps/webapp/test/runsRepository.test.ts (3)

178-188: Well-structured test coverage for filtering scenarios.

The test logic comprehensively covers the various filtering capabilities of RunsRepository, including task identifiers, versions, statuses, tags, and other parameters. The assertions correctly verify the expected filtering behavior.


1474-1499: Excellent pagination test coverage.

The pagination test properly validates cursor-based pagination with appropriate assertions for page size, cursor presence, and navigation between pages.


1276-1277:

✅ Verification successful

Verify timestamp format expectations.

The past review comment mentioned a timestamp conversion issue with division by 1000, but the current code appears to correctly pass milliseconds. Please confirm this issue has been resolved and that RunsRepository expects timestamps in milliseconds.


🏁 Script executed:

#!/bin/bash
# Description: Verify RunsRepository timestamp parameter expectations
# Search for the listRuns method implementation to check timestamp handling

ast-grep --pattern 'listRuns($_) {
  $$$
}'

Length of output: 45


To pinpoint how listRuns handles from/to, let’s locate the RunsRepository implementation and inspect its listRuns method as well as any division by 1000:


🏁 Script executed:

#!/bin/bash
# 1. Find the RunsRepository source file
repo_file=$(rg -l "class RunsRepository" --type ts)
echo "RunsRepository implementation: $repo_file"

# 2. Show 5 lines before and after the listRuns method
rg -n -B5 -A5 "listRuns\(" "$repo_file"

# 3. Check for any division by 1000 in that file
rg -n "/ *1000" "$repo_file"

Length of output: 805


Let’s pull the full listRuns implementation and the ListRunsOptions type to confirm how from/to are handled:


🏁 Script executed:

#!/bin/bash
# 1. Locate RunsRepository and ListRunsOptions definitions
repo_file=$(rg -l "class RunsRepository" --type ts)
opts_file=$(rg -l "ListRunsOptions" --type ts | head -n1)

echo "RunsRepository: $repo_file"
echo "Options type:   $opts_file"

# 2. Dump listRuns method (2 lines before, 50 after)
rg -n -B2 -A50 "async listRuns" "$repo_file"

# 3. Show ListRunsOptions interface
rg -n -B3 -A10 "interface ListRunsOptions" "$opts_file"

Length of output: 2697


Timestamp units are correct

The listRuns method uses fromUnixTimestamp64Milli for both from and to, accepting Int64 milliseconds. Since Date.getTime() already returns milliseconds, no division by 1000 is required and the original issue is resolved.

Copy link
Member Author

@ericallam ericallam left a comment

Choose a reason for hiding this comment

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

Prepped and ready to go

@ericallam ericallam merged commit b38405c into main Jun 10, 2025
27 of 29 checks passed
@ericallam ericallam deleted the realtime-created-at-filter branch June 10, 2025 11:11
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