-
-
Notifications
You must be signed in to change notification settings - Fork 710
Change plan feedback modal #1393
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
Conversation
commit 886429b Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:36:46 2024 +0100 Removed emails, @trigger.dev/database and @trigger.dev/otlp-importer from changesets config commit f65157a Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:31:27 2024 +0100 Lockfile with run-engine removed commit 3d67bb8 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:24:31 2024 +0100 Removed run-engine from the webapp package.json/tsconfig commit d30e971 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:06:04 2024 +0100 Dockerfile fix because the database package has been moved commit f2babbf Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 09:41:22 2024 -0700 Internal packages (testcontainers, redis-worker and zod-worker) (#1392) * Some notes on the new run engine * lockfile with setup for the run engine * Documenting where TaskRun is currently mutated, to try figure out the shape of the new system * Added notes about how triggering currently works * Details about when triggering happens * Lots of notes about waitpoints * Started scaffolding the RunEngine * Sketch of Prisma waitpoint schema while it’s fresh in my mind * Got Prisma working with testcontainers * Use beforeEach/afterEach * Simple Prisma and Redis test * Return Redis options instead of a client * Simplified things * A very simple FIFO pull-based queue to check the tests working properly * Use vitest extend * Separate redis, postgres and combined tests for faster testing * Some fixes and test improvements * Pass a logger into the queue * A queue processor that processes items from the given queue as fast as it can * Test for retrying an item that wasn’t processed * First draft of waitpoints in the Prisma schema * Remove the custom logger from the test * Added a completedAt to Waitpoint * Notes on the flow for an execution starting * Added redlock, moved some files around * Starting point for the TaskRunExecutionSnapshot table * Added relationships to TaskRunExecutionSnapshot * Change some tsconfig * Moved some things around * Added some packages * WIP on the RunQueue * Fix for some imports * Key producer with some tests * Removed the nv type from the keys… it’s not useful to do global queries * Passing unit tests for all the public key producer functions * Some basic tests passing for the RunQueue * Simple enqueue test working * Enqueue and dequeue for dev is working * Don’t log everything during the tests * Enqueuing/dequeuing from the shared queue is working * Tests for getting a shared queue * The key producer sharedQueue can now be named, to allow multiple separate queues * The key producer uses the name of the queue as the input * Extra info in the Prisma schema * Dequeuing a message gets the payload and sets the task concurrency all in one Lua script * Adding more keys so we can read the concurrency from the queue * Setting the concurrency with dequeue and enquque is working * Improved the tests and fixed some bugs * Acking is resetting the concurrencies * Check the key has been removed after acking * Nacking is working * Changed the package to CommonJS + Node10 so it works with Redlock * Moved the database, otel and emails packages to be in internal-packages * Moved some Prisma code to the database package * Started using the RunEngine for triggering * Progress on run engine triggering, first waitpoint code * Create a delay waitpoint * Moved ZodWorker to an internal package so it can be used in the run engine as well as the webapp * Web app now uses the zod worker package * Added parseNaturalLanguageDuration to core/apps * internal-packages/zod-worker in the lockfile * Pass in the master queue, remove old rebalance workers code * Add masterQueue to TaskRun * Fixed the tests * Moved waitpoint code into the run engine, also the zod worker * Completing waitpoints * An experiment to create a new test container with environment * More changes to triggering * Started testing triggering * Test for a run getting triggered and being enqueued * Removed dequeueMessageInEnv * Update dev queue tests to use the shared queue function * Schema changes for TaskRunExecutionSnapshot * First execution snapshot when the run is created. Dequeue run function added to the engine * Separate internal package for testcontainers so they can be used elsewhere * Remove the simple queue and testcontainers from the run-engine. They’re going to be separate * Fix for the wrong path to the Prisma schem,a * Added the testcontainers package to the run-engine * redis-worker package, just a copy of the simple queue for now * The queue now uses Lua to enqueue dequeue * The queue now has a catalog and an invisible period after dequeuing * Added a visibility timeout and acking, with tests * Added more Redis connection logging, deleted todos * Visibility timeouts are now defined on the catalog and can be overridden when enqueuing * Dequeue multiple items at once * Test for dequeuing multiple items * Export some types to be used elsewhere * Partial refactor of the processor * First stab at a worker with concurrency and NodeWorkers * Don’t have a default visibility timeout in the queue * Worker setup and processing items in a simple test * Process jobs in parallel with retrying * Get the attempt when dequeuing * Workers do exponential backoff * Moved todos * DLQ functionality * DLQ tests * Same cluster for all keys in the same queue * Added DLQ tests * Whitespace * Redis pubsub to redrive from the worker * Fixed database paths * Fix for path to zod-worker * Fixes for typecheck errors, mostly with TS versions and module resolution * Redlock required a patch * Moved the new DB migrations to the new database package folder * Remove the run-engine package * Remove the RunEngine prisma schema changes * Delete triggerTaskV2 * Remove zodworker test script (no tests) * Update test-containers readme * Generate the client first * Use a specific version of the prisma package * Generate the prisma client before running the unit tests commit fc60947 Author: Dan <8297864+D-K-P@users.noreply.github.com> Date: Tue Oct 8 14:36:03 2024 +0100 Supabase database webhook example upgrade (#1386) * Added overview for guides and examples section and split them all out * New supabase guide wip * Updated images and improved docs * Trimmed the supabase prereqs * Supabase guide wip * more updates * Replaced old database webhook guide * Created one intro page and removed snippets * Updated guide sidebar titles * Code updates * More improvements * Updates and added images * Compressed image * Updated guides descriptions and edge function basic * Removed bold * Updated redirects * Fixed broken links * Updated intro commit 07f82ea Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 13:28:54 2024 +0100 Release 3.0.11 commit 13ebfcc Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue Oct 8 13:24:38 2024 +0100 chore: Update version for release (#1381) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 2a04d17 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:24:23 2024 +0100 Simplify showLogs expression commit 002ae4b Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:29 2024 +0100 Fix dotenv overrides for dev runs (#1388) * override dashboard dev env vars with local .env * add changeset * add simple task for testing env vars commit 047cb00 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:05 2024 +0100 Disable schedules for deleted orgs on next tick (#1383) * disable schedules for deleted orgs * add debug logs commit 2c014f7 Author: James Ritchie <james@trigger.dev> Date: Sun Oct 6 13:02:00 2024 -0700 Override log retention (#1385) * set full log retention as admin * If run.logsDeletedAt is set, don’t bother getting the trace commit a69e04f Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 14:18:58 2024 +0100 Include push output in logs for self-hosted deploys (#1382) * include push output in logs * changeset commit c5488df Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 13:12:47 2024 +0100 Fix CLI downgrade check (#1380) * fix downgrade detection * remove unused semver package from webapp * add changeset commit 1caec27 Author: Eric Allam <eric@trigger.dev> Date: Fri Oct 4 15:33:35 2024 -0700 docs: Max duration (#1379) * maxDuration docs * Update the init command to set the maxDuration and include a commented out maxDuration in the config file commit e14c954 Author: Eric Allam <eallam@icloud.com> Date: Fri Oct 4 15:02:05 2024 -0700 Release 3.0.10 commit 8e61f5d Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri Oct 4 14:59:07 2024 -0700 chore: Update version for release (#1378) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 08db565 Author: Eric Allam <eallam@icloud.com> Date: Thu Oct 3 12:38:25 2024 -0700 improve the timed out description commit 6d08842 Author: Eric Allam <eric@trigger.dev> Date: Thu Oct 3 12:43:26 2024 -0700 feat: Add maxDuration to tasks (#1377) * WIP * Get max duration working on deployed runs * Actually set the timed out runs to status = TIMED_OUT * The client status for TIMED_OUT is now MAX_DURATION_EXCEEDED * New TimedOutIcon * Added new timedout icon * Add ability to opt-out of maxDuration with timeout.None * MAX_DURATION_EXCEEDED -> TIMED_OUT * changeset * Improved styling for the status tooltip content --------- Co-authored-by: James Ritchie <james@trigger.dev> commit 665ccf8 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Thu Oct 3 12:33:18 2024 +0100 Update github actions and self-hosting docs commit 1ff7b86 Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 18:26:36 2024 -0700 Add max queue depth limits (#1376) * Add runs to an env queue, as well as the actual queue * Add queue size limit guard on triggering tasks commit c531a9d Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 15:30:39 2024 -0700 fix: cleanup ttl expire run graphile jobs (#1373) * fix: remove ttl expire run graphile jobs when a run is started or completed * Update expireEnqueuedRun.server.ts commit 0bf500f Author: Matt Aitken <matt@mattaitken.com> Date: Wed Oct 2 15:30:16 2024 -0700 Prioritize finishing waited runs (#1375) * If a tree node is missing, estimate the size as zero * Task to test prioritizing finishing existing runs after triggerAndWaits * When requeuing a run with a checkpoint, put it in the queue with the parent run time so it’s correctly prioritized * The same change but if there’s no checkpoint
commit 886429b Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:36:46 2024 +0100 Removed emails, @trigger.dev/database and @trigger.dev/otlp-importer from changesets config commit f65157a Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:31:27 2024 +0100 Lockfile with run-engine removed commit 3d67bb8 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:24:31 2024 +0100 Removed run-engine from the webapp package.json/tsconfig commit d30e971 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:06:04 2024 +0100 Dockerfile fix because the database package has been moved commit f2babbf Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 09:41:22 2024 -0700 Internal packages (testcontainers, redis-worker and zod-worker) (#1392) * Some notes on the new run engine * lockfile with setup for the run engine * Documenting where TaskRun is currently mutated, to try figure out the shape of the new system * Added notes about how triggering currently works * Details about when triggering happens * Lots of notes about waitpoints * Started scaffolding the RunEngine * Sketch of Prisma waitpoint schema while it’s fresh in my mind * Got Prisma working with testcontainers * Use beforeEach/afterEach * Simple Prisma and Redis test * Return Redis options instead of a client * Simplified things * A very simple FIFO pull-based queue to check the tests working properly * Use vitest extend * Separate redis, postgres and combined tests for faster testing * Some fixes and test improvements * Pass a logger into the queue * A queue processor that processes items from the given queue as fast as it can * Test for retrying an item that wasn’t processed * First draft of waitpoints in the Prisma schema * Remove the custom logger from the test * Added a completedAt to Waitpoint * Notes on the flow for an execution starting * Added redlock, moved some files around * Starting point for the TaskRunExecutionSnapshot table * Added relationships to TaskRunExecutionSnapshot * Change some tsconfig * Moved some things around * Added some packages * WIP on the RunQueue * Fix for some imports * Key producer with some tests * Removed the nv type from the keys… it’s not useful to do global queries * Passing unit tests for all the public key producer functions * Some basic tests passing for the RunQueue * Simple enqueue test working * Enqueue and dequeue for dev is working * Don’t log everything during the tests * Enqueuing/dequeuing from the shared queue is working * Tests for getting a shared queue * The key producer sharedQueue can now be named, to allow multiple separate queues * The key producer uses the name of the queue as the input * Extra info in the Prisma schema * Dequeuing a message gets the payload and sets the task concurrency all in one Lua script * Adding more keys so we can read the concurrency from the queue * Setting the concurrency with dequeue and enquque is working * Improved the tests and fixed some bugs * Acking is resetting the concurrencies * Check the key has been removed after acking * Nacking is working * Changed the package to CommonJS + Node10 so it works with Redlock * Moved the database, otel and emails packages to be in internal-packages * Moved some Prisma code to the database package * Started using the RunEngine for triggering * Progress on run engine triggering, first waitpoint code * Create a delay waitpoint * Moved ZodWorker to an internal package so it can be used in the run engine as well as the webapp * Web app now uses the zod worker package * Added parseNaturalLanguageDuration to core/apps * internal-packages/zod-worker in the lockfile * Pass in the master queue, remove old rebalance workers code * Add masterQueue to TaskRun * Fixed the tests * Moved waitpoint code into the run engine, also the zod worker * Completing waitpoints * An experiment to create a new test container with environment * More changes to triggering * Started testing triggering * Test for a run getting triggered and being enqueued * Removed dequeueMessageInEnv * Update dev queue tests to use the shared queue function * Schema changes for TaskRunExecutionSnapshot * First execution snapshot when the run is created. Dequeue run function added to the engine * Separate internal package for testcontainers so they can be used elsewhere * Remove the simple queue and testcontainers from the run-engine. They’re going to be separate * Fix for the wrong path to the Prisma schem,a * Added the testcontainers package to the run-engine * redis-worker package, just a copy of the simple queue for now * The queue now uses Lua to enqueue dequeue * The queue now has a catalog and an invisible period after dequeuing * Added a visibility timeout and acking, with tests * Added more Redis connection logging, deleted todos * Visibility timeouts are now defined on the catalog and can be overridden when enqueuing * Dequeue multiple items at once * Test for dequeuing multiple items * Export some types to be used elsewhere * Partial refactor of the processor * First stab at a worker with concurrency and NodeWorkers * Don’t have a default visibility timeout in the queue * Worker setup and processing items in a simple test * Process jobs in parallel with retrying * Get the attempt when dequeuing * Workers do exponential backoff * Moved todos * DLQ functionality * DLQ tests * Same cluster for all keys in the same queue * Added DLQ tests * Whitespace * Redis pubsub to redrive from the worker * Fixed database paths * Fix for path to zod-worker * Fixes for typecheck errors, mostly with TS versions and module resolution * Redlock required a patch * Moved the new DB migrations to the new database package folder * Remove the run-engine package * Remove the RunEngine prisma schema changes * Delete triggerTaskV2 * Remove zodworker test script (no tests) * Update test-containers readme * Generate the client first * Use a specific version of the prisma package * Generate the prisma client before running the unit tests commit fc60947 Author: Dan <8297864+D-K-P@users.noreply.github.com> Date: Tue Oct 8 14:36:03 2024 +0100 Supabase database webhook example upgrade (#1386) * Added overview for guides and examples section and split them all out * New supabase guide wip * Updated images and improved docs * Trimmed the supabase prereqs * Supabase guide wip * more updates * Replaced old database webhook guide * Created one intro page and removed snippets * Updated guide sidebar titles * Code updates * More improvements * Updates and added images * Compressed image * Updated guides descriptions and edge function basic * Removed bold * Updated redirects * Fixed broken links * Updated intro commit 07f82ea Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 13:28:54 2024 +0100 Release 3.0.11 commit 13ebfcc Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue Oct 8 13:24:38 2024 +0100 chore: Update version for release (#1381) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 2a04d17 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:24:23 2024 +0100 Simplify showLogs expression commit 002ae4b Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:29 2024 +0100 Fix dotenv overrides for dev runs (#1388) * override dashboard dev env vars with local .env * add changeset * add simple task for testing env vars commit 047cb00 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:05 2024 +0100 Disable schedules for deleted orgs on next tick (#1383) * disable schedules for deleted orgs * add debug logs commit 2c014f7 Author: James Ritchie <james@trigger.dev> Date: Sun Oct 6 13:02:00 2024 -0700 Override log retention (#1385) * set full log retention as admin * If run.logsDeletedAt is set, don’t bother getting the trace commit a69e04f Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 14:18:58 2024 +0100 Include push output in logs for self-hosted deploys (#1382) * include push output in logs * changeset commit c5488df Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 13:12:47 2024 +0100 Fix CLI downgrade check (#1380) * fix downgrade detection * remove unused semver package from webapp * add changeset commit 1caec27 Author: Eric Allam <eric@trigger.dev> Date: Fri Oct 4 15:33:35 2024 -0700 docs: Max duration (#1379) * maxDuration docs * Update the init command to set the maxDuration and include a commented out maxDuration in the config file commit e14c954 Author: Eric Allam <eallam@icloud.com> Date: Fri Oct 4 15:02:05 2024 -0700 Release 3.0.10 commit 8e61f5d Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri Oct 4 14:59:07 2024 -0700 chore: Update version for release (#1378) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 08db565 Author: Eric Allam <eallam@icloud.com> Date: Thu Oct 3 12:38:25 2024 -0700 improve the timed out description commit 6d08842 Author: Eric Allam <eric@trigger.dev> Date: Thu Oct 3 12:43:26 2024 -0700 feat: Add maxDuration to tasks (#1377) * WIP * Get max duration working on deployed runs * Actually set the timed out runs to status = TIMED_OUT * The client status for TIMED_OUT is now MAX_DURATION_EXCEEDED * New TimedOutIcon * Added new timedout icon * Add ability to opt-out of maxDuration with timeout.None * MAX_DURATION_EXCEEDED -> TIMED_OUT * changeset * Improved styling for the status tooltip content --------- Co-authored-by: James Ritchie <james@trigger.dev> commit 665ccf8 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Thu Oct 3 12:33:18 2024 +0100 Update github actions and self-hosting docs commit 1ff7b86 Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 18:26:36 2024 -0700 Add max queue depth limits (#1376) * Add runs to an env queue, as well as the actual queue * Add queue size limit guard on triggering tasks commit c531a9d Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 15:30:39 2024 -0700 fix: cleanup ttl expire run graphile jobs (#1373) * fix: remove ttl expire run graphile jobs when a run is started or completed * Update expireEnqueuedRun.server.ts commit 0bf500f Author: Matt Aitken <matt@mattaitken.com> Date: Wed Oct 2 15:30:16 2024 -0700 Prioritize finishing waited runs (#1375) * If a tree node is missing, estimate the size as zero * Task to test prioritizing finishing existing runs after triggerAndWaits * When requeuing a run with a checkpoint, put it in the queue with the parent run time so it’s correctly prioritized * The same change but if there’s no checkpoint
|
WalkthroughThe changes in this pull request involve updates across multiple files to enhance the management of task run statuses, particularly introducing a new "TIMED_OUT" status. Modifications include adjustments to configuration files, the addition of new properties and methods, and the restructuring of existing logic to accommodate the new status. The updates also streamline imports and improve the handling of maximum duration settings for tasks, reflecting a broader effort to enhance task management and error handling across the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TaskRunService
participant TaskRunStatus
participant UI
User->>TaskRunService: Start Task Run
TaskRunService->>TaskRunStatus: Update Status to RUNNING
TaskRunStatus-->>TaskRunService: Status Updated
TaskRunService->>TaskRunStatus: Check Status
TaskRunStatus-->>TaskRunService: Status TIMED_OUT
TaskRunService->>UI: Display TIMED_OUT
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 21
🧹 Outside diff range and nitpick comments (64)
apps/webapp/app/v3/utils/maxDuration.ts (2)
1-2
: LGTM! Consider exporting constants.The constants are well-defined with appropriate values. The comment for
MAXIMUM_MAX_DURATION
is helpful.Consider exporting these constants if they might be useful in other parts of the application:
-const MINIMUM_MAX_DURATION = 5; -const MAXIMUM_MAX_DURATION = 2_147_483_647; // largest 32-bit signed integer +export const MINIMUM_MAX_DURATION = 5; +export const MAXIMUM_MAX_DURATION = 2_147_483_647; // largest 32-bit signed integer
8-22
: LGTM! Consider adding type assertion for clarity.The
getMaxDuration
function correctly handles different cases and the comment explains the logic well.For improved clarity, consider adding a type assertion to the return statement on line 21:
- return maxDuration; + return maxDuration as number;This makes it explicit that the returned value is a number in this case, aligning with the function's return type.
docs/snippets/supabase-prerequisites.mdx (2)
20-22
: LGTM with a minor suggestionThe updated instructions are clearer and more focused, aligning well with the simplified setup process. However, there's a minor grammatical issue that could be addressed.
Consider adding a comma after "file" in the conditional clause:
-If your project does not already have `package.json` file (e.g. if you are using Deno), create it manually in your project's root folder. +If your project does not already have `package.json` file, (e.g. if you are using Deno), create it manually in your project's root folder.🧰 Tools
🪛 LanguageTool
[uncategorized] ~22-~22: A comma might be missing here.
Context: ...o> If your project has apackage.json
file you can skip this step. This is...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
34-34
: LGTM: Helpful note on TypeScript versionThe addition of this note is valuable, encouraging users to keep their TypeScript version up-to-date. This is a good practice that can help avoid potential compatibility issues.
Consider enhancing the note with a link to the TypeScript releases page or a command to update TypeScript. For example:
-<Note> Update your Typescript version to the latest version available. </Note> +<Note> Update your TypeScript version to the latest version available. You can do this by running `npm install typescript@latest --save-dev` in your project directory. </Note>This addition would provide users with actionable information on how to update their TypeScript version.
docs/snippets/supabase-docs-cards.mdx (1)
Line range hint
3-4
: Consider updating introductory text for enhanced clarity.The changes have created a clear distinction between "guides" in the first card group and "examples" in the second. To further improve clarity, consider updating the introductory text above each card group:
For the first group (line 3-4), you could change it to:
"Full walkthrough guides from development to deployment"For the second group (line 22-23), you could emphasize:
"Task examples with code you can copy and paste for quick implementation"These text updates would reinforce the nature of the content in each group, improving overall documentation structure.
Also applies to: 22-23
apps/webapp/app/assets/icons/TimedOutIcon.tsx (1)
3-10
: LGTM: SVG element is well-structured. Consider adding accessibility attributes.The SVG element is correctly defined with appropriate attributes for dimensions, viewBox, and namespace. The use of the
className
prop allows for flexible styling.Consider adding
role="img"
andaria-label="Timed Out Icon"
attributes to the SVG element to improve accessibility:<svg className={className} width="20" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Timed Out Icon" >apps/webapp/app/v3/services/expireEnqueuedRun.server.ts (2)
9-11
: LGTM: New dequeue static methodThe
dequeue
method is well-implemented and consistent with the class style. It correctly uses the worker queue and handles the optional transaction parameter.Consider adding a brief comment explaining the purpose of this method and the format of the job key for better clarity:
/** * Dequeues a job for expiring a run from the worker queue. * @param runId - The ID of the run to dequeue. * @param tx - Optional transaction object. */ public static async dequeue(runId: string, tx?: PrismaClientOrTransaction) { return await workerQueue.dequeue(`v3.expireRun:${runId}`, { tx }); }
13-19
: LGTM: New enqueue static methodThe
enqueue
method is well-implemented and consistent with the class style. It correctly uses the worker queue and handles all parameters appropriately.Consider the following suggestions for improved consistency and clarity:
- Add a brief comment explaining the purpose of this method, similar to the suggestion for the
dequeue
method.- Extract the
v3.expireRun
string into a constant at the class level for better maintainability and consistency.Here's an example implementation:
private static readonly JOB_TYPE = "v3.expireRun"; /** * Enqueues a job for expiring a run in the worker queue. * @param runId - The ID of the run to enqueue. * @param runAt - Optional date to schedule the job. * @param tx - Optional transaction object. */ public static async enqueue(runId: string, runAt?: Date, tx?: PrismaClientOrTransaction) { return await workerQueue.enqueue( ExpireEnqueuedRunService.JOB_TYPE, { runId }, { runAt, jobKey: `${ExpireEnqueuedRunService.JOB_TYPE}:${runId}`, tx } ); }This approach would also require updating the
dequeue
method to use the same constant:public static async dequeue(runId: string, tx?: PrismaClientOrTransaction) { return await workerQueue.dequeue(`${ExpireEnqueuedRunService.JOB_TYPE}:${runId}`, { tx }); }apps/webapp/app/v3/requeueTaskRun.server.ts (2)
73-73
: LGTM! Consider grouping similar statuses for better readability.The addition of the "TIMED_OUT" status is consistent with the existing pattern for completed task runs. It's correctly grouped with other completed statuses and follows the same logic of logging and acknowledging the message.
To improve code readability, consider grouping similar statuses together. For example:
- case "SYSTEM_FAILURE": - case "INTERRUPTED": - case "CRASHED": - case "COMPLETED_WITH_ERRORS": - case "COMPLETED_SUCCESSFULLY": - case "EXPIRED": - case "TIMED_OUT": - case "CANCELED": { + case "COMPLETED_SUCCESSFULLY": + case "COMPLETED_WITH_ERRORS": + case "EXPIRED": + case "TIMED_OUT": + case "CANCELED": + case "SYSTEM_FAILURE": + case "INTERRUPTED": + case "CRASHED": {This grouping puts all "completed" statuses together, followed by "failure" statuses, making the code more organized and easier to understand at a glance.
Line range hint
1-100
: Consider consistent error handling for all cases.The overall structure and logic of the file are sound. However, to improve consistency and robustness, consider adding error handling for all cases in the switch statement.
Here's a suggestion to wrap each case in a try-catch block:
switch (taskRun.status) { case "PENDING": { try { logger.debug("[RequeueTaskRunService] Requeueing task run", { taskRun }); await marqs?.nackMessage(taskRun.id); } catch (error) { logger.error("[RequeueTaskRunService] Error requeueing task run", { taskRun, error }); } break; } // Apply similar try-catch blocks to other cases }This approach ensures that any unexpected errors during the processing of each status are caught and logged, preventing the service from crashing and providing valuable debugging information.
docker/Dockerfile (2)
33-33
: LGTM! Consider using ARG for Prisma version.The changes to the Prisma schema path are consistent and reflect the project's new structure. Good job on updating both the COPY command and the Prisma generate command to use the new path.
Consider using an ARG to define the Prisma version, making it easier to update in the future:
+ARG PRISMA_VERSION=5.4.1 -RUN pnpx prisma@5.4.1 generate --schema /triggerdotdev/internal-packages/database/prisma/schema.prisma +RUN pnpx prisma@${PRISMA_VERSION} generate --schema /triggerdotdev/internal-packages/database/prisma/schema.prismaThis way, you can easily update the Prisma version by changing it in one place or even override it during the build process.
Also applies to: 36-36
Line range hint
1-71
: Great use of multi-stage builds! Consider further optimizations.The Dockerfile demonstrates excellent use of multi-stage builds, appropriate setting of environment variables, and adherence to security best practices like using a non-root user. Well done!
Consider the following optimizations to further improve the Dockerfile:
- Combine RUN commands where possible to reduce the number of layers. For example:
-RUN apt-get update && apt-get install -y openssl dumb-init -WORKDIR /triggerdotdev +RUN apt-get update && apt-get install -y openssl dumb-init && \ + mkdir -p /triggerdotdev && \ + chown node:node /triggerdotdev +WORKDIR /triggerdotdev
- Use
--no-install-recommends
with apt-get to reduce image size:-RUN apt-get update && apt-get install -y openssl netcat-openbsd +RUN apt-get update && apt-get install -y --no-install-recommends openssl netcat-openbsd && \ + apt-get clean && \ + rm -rf /var/lib/apt/lists/*
- Consider using a
.dockerignore
file to prevent unnecessary files from being copied into the image, which can speed up builds and reduce image size.These optimizations can help reduce the final image size and potentially speed up the build process.
docs/introduction.mdx (2)
27-29
: LGTM: Improved guide description with specific examplesThe change from "Framework guides" to "Walk-through guides" is more inclusive and descriptive. The addition of specific examples (Next.js, Remix, Supabase) provides clearer expectations for users.
Consider adding a comma before "and more" for better readability:
- Next.js, Remix, Supabase and more. + Next.js, Remix, Supabase, and more.
31-33
: LGTM: Enhanced "Example tasks" description with specific technologiesThe addition of specific technologies (OpenAI, Deepgram, FFmpeg, Puppeteer, Stripe) provides valuable context for users, helping them quickly identify relevant examples.
For consistency with the previous card, consider adding a comma before "and more":
- Supabase and more. + Supabase, and more.apps/webapp/app/v3/taskStatus.ts (1)
100-100
: LGTM: Addition of "TIMED_OUT" to FAILED_RUN_STATUSES is correct and consistent.The addition of "TIMED_OUT" to the FAILED_RUN_STATUSES array is appropriate and aligns with the array's purpose. This change ensures that the isFailedRunStatus function will correctly recognize a timed-out task as failed.
For improved consistency and maintainability, consider using a TypeScript satisfies clause for FAILED_RUN_STATUSES, similar to how it's used for FINAL_RUN_STATUSES:
export const FAILED_RUN_STATUSES = [ "INTERRUPTED", "COMPLETED_WITH_ERRORS", "SYSTEM_FAILURE", "CRASHED", "TIMED_OUT", ] satisfies TaskRunStatus[];This change would provide better type checking and consistency across the module.
apps/webapp/app/v3/marqs/types.ts (2)
48-48
: LGTM: New methodenvQueueKeyFromQueue
is a valuable addition.The
envQueueKeyFromQueue
method is a well-designed addition to theMarQSKeyProducer
interface. It maintains consistency with existing methods and provides a useful way to derive an environment queue key from a queue string.Consider adding a brief JSDoc comment to clarify the method's purpose and the expected format of the
queue
parameter. For example:/** * Generates an environment queue key from a given queue string. * @param queue The queue identifier string * @returns The environment queue key */ envQueueKeyFromQueue(queue: string): string;
29-29
: Summary: Valuable additions to the MarQSKeyProducer interfaceThe two new methods,
envQueueKey
andenvQueueKeyFromQueue
, are well-designed additions to theMarQSKeyProducer
interface. They enhance the interface's capabilities for handling environment-specific queues and provide flexibility in key generation. These changes are consistent with the existing codebase and should improve the overall functionality of the queue management system.As the
MarQSKeyProducer
interface grows, consider grouping related methods (e.g., all env-related methods) and adding clear documentation for each group. This will enhance maintainability and make it easier for developers to understand and use the interface correctly.Also applies to: 48-48
docs/guides/introduction.mdx (2)
13-21
: LGTM: New "Frameworks" section enhances overviewThe addition of the "Frameworks" section with a CardGroup component provides a clear, visual overview of supported frameworks. This improves the document's structure and user experience.
Consider adding a brief introductory sentence before the CardGroup to provide context for the frameworks listed. For example:
## Frameworks + +Trigger.dev supports a variety of popular frameworks. Choose your preferred framework to get started: <CardGroup cols={3}> <CardBun /> <CardNodejs /> <CardNextjs /> <CardRemix /> <CardSupabase /> </CardGroup>
Line range hint
32-52
: LGTM: Comprehensive "Example tasks" section addedThe new "Example tasks" section provides a valuable resource for users, showcasing a wide variety of tasks that can be accomplished with Trigger.dev. The table format is consistent with the "Guides" section, maintaining a cohesive structure throughout the document.
Consider adding a "Difficulty" or "Complexity" column to the table to help users quickly identify tasks that match their skill level. For example:
| Example task | Description | Difficulty | | :---------------------------------------------------------------------------- | :----------------------------------------------------------------------------- | :--------- | -| [DALL·E 3 image generation](/guides/examples/dall-e3-generate-image) | Use OpenAI's GPT-4o and DALL·E 3 to generate an image and text. | +| [DALL·E 3 image generation](/guides/examples/dall-e3-generate-image) | Use OpenAI's GPT-4o and DALL·E 3 to generate an image and text. | Beginner | | [Deepgram audio transcription](/guides/examples/deepgram-transcribe-audio) | Transcribe audio using Deepgram's speech recognition API. | ...This addition would further enhance the user experience by allowing users to quickly find tasks appropriate for their skill level.
apps/webapp/app/v3/services/resumeTaskDependency.server.ts (1)
66-67
: LGTM! Consider adding a comment for clarity.The addition of
dependentRun.createdAt.getTime()
to theenqueueMessage
call is a good improvement. It provides valuable timing information that can be useful for tracking or debugging purposes.Consider adding a brief comment explaining the purpose of including the creation timestamp in the message payload. This would enhance code readability and make the intent clearer for future developers.
apps/webapp/app/presenters/v3/RunPresenter.server.ts (1)
79-98
: LGTM: Return structure changes improve code quality.The changes to the return structure are well-implemented:
- The creation of
runData
reduces code duplication and improves maintainability.- The consistent use of
runData
in all return statements ensures uniformity.These changes align with the DRY principle and make the code more robust.
Consider using object shorthand notation for properties in
runData
where the property name matches the variable name. For example:const runData = { id: run.id, number: run.number, friendlyId: run.friendlyId, traceId: run.traceId, spanId: run.spanId, status: run.status, // ... other properties };This can make the code slightly more concise and easier to read.
Also applies to: 101-104, 111-112, 154-154
apps/webapp/app/routes/_app.orgs.$organizationSlug.v3.billing/route.tsx (2)
120-120
: LGTM! Consider adding a comment for clarity.The addition of the
periodEnd
prop to thePricingPlans
component is a good enhancement. It allows the component to access the billing period end date, which can be useful for displaying relevant information to the user.Consider adding a brief comment explaining the purpose of this prop, for example:
periodEnd={periodEnd} // Pass the billing period end date to display in pricing plans
Line range hint
126-147
: Good addition, but consider handling cancellation for enterprise plans.The
planLabel
function is a great addition that provides clear, user-friendly messages about the current subscription plan. It handles different scenarios well, including free plans and canceled plans.Consider updating the enterprise plan case to handle cancellations similarly to other paid plans. Here's a suggested modification:
if (plan.type === "enterprise") { const text = `You're on the Enterprise plan`; if (canceled) { return ( <> {text}. From <DateTime includeTime={false} date={periodEnd} /> you're on the Free plan. </> ); } return text; }This change ensures consistent behavior across all plan types when a subscription is canceled.
docs/runs/max-duration.mdx (5)
1-11
: LGTM with a minor suggestion.The frontmatter and introduction provide a clear and concise overview of the
maxDuration
feature. However, there's a small grammatical improvement that can be made.Consider adding a comma after "By default" in line 7:
-By default tasks can execute indefinitely, which can be great! +By default, tasks can execute indefinitely, which can be great!🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: Did you mean: “By default,”?
Context: ...ximum duration for a task to run." --- By default tasks can execute indefinitely, which c...(BY_DEFAULT_COMMA)
13-40
: LGTM with a suggestion for improved clarity.The "How it works" section provides a comprehensive explanation of the
maxDuration
feature, including a helpful code example and visual representation of the error scenario.To enhance clarity, consider adding a brief explanation of what happens when a task exceeds the
maxDuration
. For example, after line 37, you could add:If a task exceeds its `maxDuration`, it will be forcibly terminated, and any remaining operations will not be executed.This addition would provide users with a clear understanding of the consequences of exceeding the
maxDuration
.
41-41
: Consider providing context for the maximum duration.The note about minimum and maximum durations is informative. However, the maximum duration of ~68 years might benefit from additional context.
Consider adding a brief explanation for why such a high maximum is allowed, or if it's a technical limitation. For example:
<Note> The minimum maxDuration is 5 seconds. The maximum is ~68 years (2^31 - 1 seconds, which is the maximum value for a 32-bit signed integer). While such a long duration is unlikely to be used in practice, it allows for flexibility in long-running tasks. </Note>This addition would provide users with a better understanding of the reasoning behind the maximum duration limit.
57-87
: LGTM with a minor suggestion for consistency.The "Configuring for a task" section provides clear explanations and helpful code examples for setting and disabling
maxDuration
for specific tasks.For consistency with the previous section, consider adding a brief comment in the code example to explain the
maxDuration
value. For example:export const maxDurationTask = task({ id: "max-duration-task", - maxDuration: 300, // 300 seconds or 5 minutes + maxDuration: 300, // 300 seconds (5 minutes) run: async (payload: any, { ctx }) => { //... }, });This change would maintain the style consistency with the previous code example.
120-138
: LGTM with a suggestion for additional information.The sections on "maxDuration in run context" and "maxDuration and lifecycle functions" provide valuable information about accessing
maxDuration
and its impact on lifecycle functions.Consider expanding the explanation about lifecycle functions not being called when a task exceeds
maxDuration
. For example:## maxDuration and lifecycle functions When a task run exceeds the `maxDuration`, it is forcibly terminated. As a result, the lifecycle functions `cleanup`, `onSuccess`, and `onFailure` will not be called. This is important to note for error handling and resource cleanup strategies. Consider implementing alternative cleanup mechanisms for long-running tasks that might exceed their `maxDuration`.This additional information would help users better understand the implications of exceeding
maxDuration
and plan their error handling and cleanup strategies accordingly.apps/webapp/app/v3/marqs/marqsKeyProducer.server.ts (2)
131-135
: LGTM with a minor suggestion for robustness.The
envQueueKeyFromQueue
method looks good and is consistent with other key generation methods in the class. It correctly extracts the environment ID from the queue string and constructs the key using the appropriate constants.Consider adding a check for the length of the split array to ensure the environment ID exists before accessing it. This would make the method more robust against malformed input:
envQueueKeyFromQueue(queue: string) { - const envId = this.normalizeQueue(queue).split(":")[3]; + const parts = this.normalizeQueue(queue).split(":"); + if (parts.length < 4) { + throw new Error("Invalid queue string format"); + } + const envId = parts[3]; return `${constants.ENV_PART}:${envId}:${constants.QUEUE_PART}`; }
137-139
: LGTM with a minor suggestion for consistency.The
envQueueKey
method looks good. It correctly constructs the key using the appropriate constants and the shortened environment ID.For consistency with other methods in the class, consider using the
envKeySection
private method:envQueueKey(env: AuthenticatedEnvironment): string { - return [constants.ENV_PART, this.shortId(env.id), constants.QUEUE_PART].join(":"); + return [this.envKeySection(env.id), constants.QUEUE_PART].join(":"); }This change would make the method more consistent with others like
envConcurrencyLimitKey
andenvCurrentConcurrencyKey
.docs/guides/frameworks/supabase-edge-functions-basic.mdx (5)
29-29
: Approved: Important prerequisite addedThe addition of the Docker Desktop requirement is crucial for users to successfully deploy Edge Functions. This information is up-to-date and tied to a specific Supabase CLI version.
Consider adding a link to the Docker Desktop installation page for user convenience.
Line range hint
91-101
: Approved: Clear and informative code snippet with crucial notesThe added code snippet effectively demonstrates how to set up the edge function, including necessary imports and task triggering. The notes about type-only imports and task location restrictions are essential for correct implementation.
Consider adding a brief comment explaining the purpose of the
Deno.serve
function for users who might be unfamiliar with Deno.
Line range hint
134-152
: Approved: Clear and detailed instructions for setting up the secret keyThe added instructions for setting the Trigger.dev production secret key in the Supabase dashboard are comprehensive and user-friendly. The step-by-step guidance, enhanced with visual aids, significantly improves the user experience and reduces the likelihood of setup errors.
Consider adding a note about the importance of keeping the secret key secure and not sharing it publicly.
Line range hint
181-196
: Approved: Clear instructions for testing the deployed edge functionThe added instructions for triggering a production run from the deployed edge function provide a clear and concise way for users to test their setup. The step-by-step guidance on finding and using the edge function URL is helpful, and the congratulatory message at the end provides a nice touch of positive reinforcement.
Consider adding a troubleshooting section or link to common issues that users might encounter when testing their edge function.
Line range hint
1-196
: Excellent improvements to the documentationThe changes made to this document significantly enhance its value as a guide for setting up and using Supabase edge functions with Trigger.dev. The additions, including new prerequisites, detailed code snippets, and step-by-step instructions with visual aids, provide a more comprehensive and user-friendly experience. These improvements will likely reduce user errors and improve the overall success rate of implementations.
Consider adding a section on best practices for error handling and logging in edge functions to help users create more robust implementations.
docs/mint.json (1)
Line range hint
274-290
: LGTM: New "Guides" group added with appropriate entries.The addition of a new "Guides" group and the inclusion of Prisma and Sequin entries here is a good restructuring decision. It provides a more appropriate categorization for these frameworks.
Consider adding a brief description or introduction to this new "Guides" section to help users understand the difference between "Frameworks" and "Guides".
apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (1)
313-315
: LGTM! Consider a minor improvement for consistency.The addition of the "TIMED_OUT" case is a good improvement to the
apiStatusToRunStatuses
method, allowing it to handle this new status correctly. This change aligns well with the overall goal of enhancing task management and error handling.For consistency with other single-status cases in this method, you might consider simplifying the return statement:
case "TIMED_OUT": { - return "TIMED_OUT"; + return "TIMED_OUT"; }This minor change would make the "TIMED_OUT" case consistent with other single-status cases like "DELAYED" or "CANCELED".
docs/tasks/overview.mdx (1)
124-138
: Approve newmaxDuration
section with minor grammatical correction.The new section on the
maxDuration
option is well-written and provides valuable information to users. It aligns with the PR objectives and enhances the documentation.Consider adding a comma after "By default" in the first sentence for better readability:
-By default tasks can execute indefinitely, which can be great! +By default, tasks can execute indefinitely, which can be great!🧰 Tools
🪛 LanguageTool
[uncategorized] ~125-~125: Did you mean: “By default,”?
Context: ... }, }); ``` ###maxDuration
option By default tasks can execute indefinitely, which c...(BY_DEFAULT_COMMA)
apps/webapp/app/presenters/v3/SpanPresenter.server.ts (1)
313-313
: LGTM with a suggestion: Add a safety check for undefined values.The addition of the
maxDurationInSeconds
property using thegetMaxDuration
function is a good enhancement. However, to improve robustness, consider adding a safety check for undefined values.Consider modifying the line as follows to handle potential undefined values:
maxDurationInSeconds: run.maxDurationInSeconds !== undefined ? getMaxDuration(run.maxDurationInSeconds) : undefined,This change ensures that
getMaxDuration
is only called whenrun.maxDurationInSeconds
has a defined value, preventing potential issues with undefined arguments.apps/webapp/app/env.server.ts (1)
217-218
: LGTM. Consider adding documentation for the new queue size limits.The addition of
MAXIMUM_DEV_QUEUE_SIZE
andMAXIMUM_DEPLOYED_QUEUE_SIZE
as optional integer environment variables is a good enhancement for controlling queue sizes in different environments. The implementation is correct and consistent with the existing schema.To improve clarity for other developers:
- Consider adding comments explaining the purpose and impact of these queue size limits.
- Update any relevant documentation to include these new configuration options.
- If applicable, provide guidelines on recommended values for these limits in different scenarios.
apps/webapp/app/services/runExecutionRateLimiter.server.ts (1)
120-120
: Consider adding an explicit return statement for clarityThe removal of the explicit
return false
statement in the Lua script for thebeforeTask
command might affect readability. While the behavior remains the same (Lua functions implicitly returnnil
if no return statement is reached, which is equivalent tofalse
in boolean contexts), an explicit return could improve code clarity.Consider adding an explicit
return false
statement at the end of the Lua script:redis.call('SADD', forbiddenFlagsKey, forbiddenFlag) +return false end
This change would make the intention of the function more explicit and improve readability.
docs/guides/frameworks/sequin.mdx (6)
25-29
: LGTM: Improved formatting for readabilityThe adjustments to the component's formatting enhance readability. The added line breaks provide better visual separation, making the content easier to scan.
Consider adding a blank line before and after the component to further improve visual separation:
+ <Info> If you don't have one already, follow [Trigger.dev's Next.js setup guide](/guides/frameworks/nextjs) to setup your project. You can return to this guide when you're ready to write your first Trigger.dev task. </Info> +🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: The official spelling of this programming framework is “Next.js”.
Context: ...w Trigger.dev's Next.js setup guide to setup your project. You can return ...(NODE_JS)
[grammar] ~27-~27: The word “setup” is a noun. The verb is spelled with a white space.
Context: ... guide](/guides/frameworks/nextjs) to setup your project. You can return to this gu...(NOUN_VERB_CONFUSION)
Line range hint
47-120
: Good addition, but some improvements neededThe new
createEmbeddingForPost
task is a valuable addition. However, there are a few points to address:
The OpenAI client is initialized outside the task. Consider moving it inside the task to ensure the API key is loaded from the environment variables at runtime.
The
upsertEmbedding
function is referenced but its implementation is in a separate file. Consider adding a comment to clarify this for readers.Error handling could be improved, especially for the OpenAI API call and the database operation.
Consider the following improvements:
- Move the OpenAI client initialization inside the task:
-const openai = new OpenAI({ - apiKey: process.env.OPENAI_API_KEY, -}); export const createEmbeddingForPost = task({ id: "create-embedding-for-post", run: async (payload: { // ... (payload type definition) }) => { + const openai = new OpenAI({ + apiKey: process.env.OPENAI_API_KEY, + }); // ... (rest of the task implementation) } });
- Add a comment about the
upsertEmbedding
function:+ // Upsert the embedding in the database. + // Note: The upsertEmbedding function is implemented in utils.ts await upsertEmbedding(embedding, payload.record.id);
- Improve error handling:
run: async (payload: { // ... (payload type definition) }) => { + try { // Create an embedding using the title and body of payload.record const content = `${payload.record.title}\n\n${payload.record.body}`; const embedding = (await openai.embeddings.create({ model: "text-embedding-ada-002", input: content, })).data[0].embedding; // Upsert the embedding in the database. // Note: The upsertEmbedding function is implemented in utils.ts await upsertEmbedding(embedding, payload.record.id); // Return the updated record return { ...payload.record, embedding: JSON.stringify(embedding), }; + } catch (error) { + console.error("Error in createEmbeddingForPost task:", error); + throw error; // Re-throw the error to mark the task as failed + } } });These changes will improve the robustness and clarity of the task implementation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~87-~87: Loose punctuation mark.
Context: ...N.stringify(embedding), }; } }); ```` ```ts utils.ts import pg from ...(UNLIKELY_OPENING_PUNCTUATION)
Line range hint
211-268
: Clear instructions, but some improvements possibleThe instructions for creating a Sequin consumer are comprehensive and well-structured. However, there are a few points that could be clarified or improved:
The SQL commands for creating a publication and replication slot are provided without much context. It might be helpful to explain what these commands do and why they're necessary.
The instructions for tunneling to a local endpoint assume that the user has already installed the Sequin CLI. It might be helpful to provide a link to the CLI installation instructions earlier in the guide.
The section on creating a Push Consumer could benefit from a brief explanation of what a Push Consumer is and why it's needed in this context.
Consider the following improvements:
- Add context to the SQL commands:
create publication sequin_pub for all tables; select pg_create_logical_replication_slot('sequin_slot', 'pgoutput'); + +# These commands set up logical replication in your database: +# - The first command creates a publication that includes all tables. +# - The second command creates a replication slot that Sequin will use to capture changes.
- Add a note about installing the Sequin CLI earlier in the guide:
## Create Sequin consumer +Before proceeding, ensure you have the [Sequin CLI](https://sequinstream.com/docs/cli) installed on your system. + You'll now configure Sequin to send every row in your `posts` table to your Trigger.dev task.
- Add a brief explanation of Push Consumers:
<Step title="Create a Push Consumer"> +A Push Consumer in Sequin captures changes from your database and sends them to a specified endpoint. In this case, we'll create a consumer that captures changes to the `posts` table and sends them to your Trigger.dev task. + Create a push consumer that will capture posts from your database and deliver them to your local endpoint:These changes will provide more context and improve the overall user experience of following the guide.
Line range hint
273-315
: Comprehensive test instructions, with room for minor improvementsThe end-to-end test instructions are clear and cover all aspects of the setup. However, there are a couple of points that could be improved:
- The SQL insert statement could be formatted for better readability.
- A step to verify that the embedding was actually inserted into the database could be added.
Consider the following improvements:
- Format the SQL insert statement for better readability:
- insert into - posts (title, body, author) - values - ( - 'The Future of AI', - 'An insightful look into how artificial intelligence is shaping the future of technology and society.', - 'Alice H Johnson' - ); + INSERT INTO posts (title, body, author) + VALUES ( + 'The Future of AI', + 'An insightful look into how artificial intelligence is shaping the future of technology and society.', + 'Alice H Johnson' + );
- Add a step to verify the database update:
<Frame> <img src="/images/sequin-final-run.png" alt="Task run" /> </Frame> </Step> + <Step title="Verify the database update"> + Check your database to confirm that the embedding has been inserted: + + ```sql + SELECT id, title, embedding IS NOT NULL AS has_embedding + FROM posts + WHERE title = 'The Future of AI'; + ``` + + This query should return a row with `has_embedding` set to `true`. + </Step> </Steps>These changes will improve the clarity of the test instructions and provide a complete verification of the entire process.
Line range hint
319-330
: Good next steps, but could be more comprehensiveThe next steps section provides relevant suggestions for error handling and deployment. However, it could be expanded to provide more specific guidance and cover additional aspects of production deployment.
Consider expanding the next steps section with the following additions:
- Add more specific steps for production deployment:
- Deploy to [production](/guides/frameworks/nextjs#deploying-your-task-to-trigger-dev) and update your Sequin consumer to point to your production database and endpoint. + + - Set up environment variables in your production environment for `SEQUIN_WEBHOOK_SECRET`, `TRIGGER_SECRET_KEY`, `OPENAI_API_KEY`, and `DATABASE_URL`. + - Update your Sequin consumer in the production environment to use the production API endpoint URL. + - Implement proper logging and monitoring for your production deployment.
- Add suggestions for scaling and optimization:
+- Optimize your database queries and indexes to handle increased load. +- Implement caching strategies to reduce the number of embedding generations for frequently accessed posts. +- Set up monitoring and alerting for your Trigger.dev tasks and Sequin consumers to quickly identify and resolve any issues.
- Suggest exploring additional use cases:
+- Explore other use cases for combining Sequin and Trigger.dev, such as: + - Implementing real-time search functionality using the generated embeddings. + - Automating content categorization or tagging based on the embeddings. + - Creating a recommendation system using similarity between post embeddings.These additions will provide users with a more comprehensive roadmap for taking their implementation to the next level and exploring further possibilities with the integration.
Line range hint
1-330
: Address minor language and formatting issuesThere are a few minor language and formatting issues that should be addressed:
- The spelling of "Next.js" should be consistent throughout the document.
- There's a grammatical issue with the use of "setup" as a verb.
- There's an extra backtick causing a formatting issue in one of the code blocks.
Please make the following corrections:
- Ensure "Next.js" is spelled correctly throughout the document:
-guide](/guides/frameworks/nextjs) to setup your project. You can return +guide](/guides/frameworks/next-js) to set up your project. You can return
- Correct the use of "setup" as a verb:
-guide](/guides/frameworks/nextjs) to setup your project. You can return +guide](/guides/frameworks/next-js) to set up your project. You can return
- Remove the extra backtick in the code block:
- }); ```` + }); ```These changes will improve the overall quality and consistency of the document.
docs/open-source-self-hosting.mdx (4)
35-39
: Valuable addition of detailed technical warnings.The new accordion section with detailed warnings is an excellent addition to the document. It provides crucial information about potential issues and limitations related to Docker usage, resource management, and security considerations. This level of detail helps users make informed decisions about self-hosting.
Consider adding a brief introductory sentence before the list of warnings to provide context, such as: "Please be aware of the following technical considerations when self-hosting:"
65-86
: Excellent addition of detailed installation instructions.The new installation instructions for Docker and Ngrok are a valuable addition to the document. They provide clear, step-by-step guidance for setting up the required software. The use of an accordion improves the document's organization and allows users to expand the details when needed.
Consider adding a note about potential variations in these instructions depending on the specific Debian derivative being used, as package names or repository URLs might differ slightly.
6-6
: Valuable addition of advanced configuration options.The new sections on "Large payloads" and "Version locking" are excellent additions to the document. They provide important information for advanced usage scenarios, which is crucial for users who need to handle large data or maintain specific versions. The instructions are clear and include helpful examples.
Consider adding a brief note about the potential performance implications of increasing the payload threshold, to help users make informed decisions about this configuration.
6-6
: Comprehensive coverage of authentication options.The new "Auth options" section is a valuable addition to the document. It provides clear explanations of the available authentication methods, including magic link auth and GitHub OAuth. The inclusion of important warnings about the risks of enabling GitHub auth for self-hosted instances is particularly noteworthy and helps users make informed decisions.
Consider adding a brief explanation of why GitHub OAuth is not recommended for self-hosted instances, to help users better understand the associated risks.
docs/guides/frameworks/supabase-edge-functions-database-webhooks.mdx (1)
89-261
: Comprehensive guide for creating and deploying the Trigger.dev task.This extensive section covers all aspects of creating the Trigger.dev task, including:
- Generating database type definitions
- Creating the transcription task with detailed code explanation
- Adding the FFmpeg build extension
- Setting up environment variables
- Deploying the task
The code provided is well-commented and includes necessary error handling. However, there's a minor typo in the package installation commands.
In the package installation commands, there's a typo. The
yarn
command should useadd
instead ofinstall
. Here's the corrected version:-yarn install @deepgram/sdk @supabase/supabase-js fluent-ffmpeg +yarn add @deepgram/sdk @supabase/supabase-js fluent-ffmpegdocs/config/config-file.mdx (1)
155-168
: LGTM! Consider adding a brief explanation ofmaxDuration
.The new section on
maxDuration
is a valuable addition to the documentation. It clearly shows how to set a default maximum duration for all tasks in the project.Consider adding a brief explanation of what
maxDuration
does and its importance. For example:
"ThemaxDuration
setting allows you to specify a maximum time limit (in seconds) for task execution, helping to prevent long-running tasks from consuming excessive resources."apps/webapp/app/components/runs/v3/RunInspector.tsx (1)
327-332
: LGTM! Consider adding a tooltip for clarity.The addition of the "Max duration" property item is well-implemented and consistent with other properties in the table. It provides valuable information about the maximum duration of a run.
To enhance user understanding, consider adding a tooltip that explains what "Max duration" represents and its implications. You can use the
InfoIconTooltip
component, similar to how it's used in other parts of this file. For example:<Property.Item> <Property.Label>Max duration</Property.Label> <Property.Value> - {run.maxDurationInSeconds ? `${run.maxDurationInSeconds}s` : "–"} + <span className="flex items-center gap-1"> + {run.maxDurationInSeconds ? `${run.maxDurationInSeconds}s` : "–"} + <InfoIconTooltip + content="The maximum time allowed for this run to complete before it times out." + contentClassName="normal-case tracking-normal" + /> + </span> </Property.Value> </Property.Item>This addition would provide users with more context about the "Max duration" property.
docs/triggering.mdx (1)
745-748
: LGTM! Consider enhancing themaxDuration
introduction.The addition of the
maxDuration
option is a valuable update to the documentation. It provides users with a quick reference and directs them to more detailed information.To improve clarity and consistency with other sections, consider adding a brief description of what
maxDuration
does. For example:### `maxDuration` +You can set a maximum duration for a task run. If the run exceeds this duration, it will be terminated. + View our [maxDuration doc](/runs/max-duration) for more information.This addition would give users a quick understanding of the option's purpose before they navigate to the detailed documentation.
🧰 Tools
🪛 LanguageTool
[style] ~746-~746: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...r more information. ###maxDuration
View our [maxDuration doc](/runs/max-duratio...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.v3.$projectParam.runs.$runParam.spans.$spanParam/route.tsx (2)
680-685
: LGTM! Consider adding a tooltip for clarity.The addition of the "Max duration" property item is a good improvement to provide more information about the run. The implementation is correct and consistent with the existing code style.
Consider adding a tooltip to explain what the max duration represents and its implications. This could be particularly helpful for users who may not be familiar with this concept. You can use the existing
InfoIconTooltip
component for this purpose. For example:<Property.Item> <Property.Label>Max duration</Property.Label> <Property.Value> - {run.maxDurationInSeconds ? `${run.maxDurationInSeconds}s` : "–"} + <span className="flex items-center gap-1"> + {run.maxDurationInSeconds ? `${run.maxDurationInSeconds}s` : "–"} + <InfoIconTooltip + content="The maximum time allowed for this run to complete before it times out." + contentClassName="normal-case tracking-normal" + /> + </span> </Property.Value> </Property.Item>
Line range hint
1-1023
: Consider refactoring for improved code organization and reusability.While the current implementation is functional, there are opportunities to enhance code organization and reusability.
Consider the following improvements:
Extract the
Property.Item
components into a separate, reusable component. This would reduce repetition and make it easier to maintain consistent styling and behavior across all property items.Create a configuration object or array for the properties in the
RunBody
component. This would allow for easier reordering and maintenance of the properties.Consider splitting the large
RunBody
component into smaller, more focused components. This would improve readability and maintainability.Here's an example of how you could implement the first two suggestions:
// PropertyItem.tsx type PropertyItemProps = { label: string; value: React.ReactNode; }; function PropertyItem({ label, value }: PropertyItemProps) { return ( <Property.Item> <Property.Label>{label}</Property.Label> <Property.Value>{value}</Property.Value> </Property.Item> ); } // RunBody.tsx const runProperties = [ { label: "Status", value: (run) => <TaskRunStatusCombo status={run.status} /> }, { label: "Max duration", value: (run) => run.maxDurationInSeconds ? `${run.maxDurationInSeconds}s` : "–" }, // ... other properties ]; function RunBody({ run, ...otherProps }) { // ... other code return ( // ... other JSX <Property.Table> {runProperties.map(({ label, value }) => ( <PropertyItem key={label} label={label} value={value(run)} /> ))} </Property.Table> // ... other JSX ); }This refactoring would make the code more maintainable and easier to update in the future.
apps/webapp/app/v3/queueSizeLimits.server.ts (2)
11-32
: Add JSDoc comments for better maintainabilityConsider adding JSDoc comments to the
guardQueueSizeLimitsForEnv
function to enhance code readability and maintainability. Documenting the function's purpose, parameters, and return type will help other developers understand its usage.
34-40
: Add JSDoc comments to helper functionAdding JSDoc comments to the
getMaximumSizeForEnvironment
function would improve code documentation and assist in understanding the function's behavior.apps/webapp/app/db.server.ts (1)
15-15
: Consider Using a More Descriptive Alias for$transaction
Renaming
$transaction
totransac
could reduce readability. Consider using a more descriptive alias likeexecuteTransaction
orrunTransaction
for better code clarity.apps/webapp/app/v3/services/completeAttempt.server.ts (1)
366-375
: Simplify the conditional assignment of 'status'The current ternary operation can be refactored for clarity and maintainability.
Apply this diff to refactor the conditional assignment:
const status = - sanitizedError.type === "INTERNAL_ERROR" && - sanitizedError.code === "MAX_DURATION_EXCEEDED" - ? "TIMED_OUT" - : "COMPLETED_WITH_ERRORS"; + sanitizedError.type === "INTERNAL_ERROR" && sanitizedError.code === "MAX_DURATION_EXCEEDED" + ? "TIMED_OUT" + : "COMPLETED_WITH_ERRORS";Alternatively, consider using an
if...else
statement for better readability:let status; if (sanitizedError.type === "INTERNAL_ERROR" && sanitizedError.code === "MAX_DURATION_EXCEEDED") { status = "TIMED_OUT"; } else { status = "COMPLETED_WITH_ERRORS"; }apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (2)
813-832
: Add Accessible Label to the Tooltip TriggerIn the
TierLimit
component, the tooltip trigger is an<a>
element without discernible text, which may affect accessibility.Consider adding an
aria-label
or ensuring the link text is accessible to screen readers.<a href={href} + aria-label="View detailed compute pricing information" className="text-left font-sans text-lg font-normal text-text-bright underline decoration-charcoal-500 underline-offset-4 transition hover:decoration-text-bright" > {children} </a>
425-440
: Ensure Unique Checkbox IDsWhen rendering checkboxes in a list, using the index in
id={
reason-${index + 1}}
can lead to duplicate IDs if the list changes dynamically.Consider using a more stable identifier for the
id
attribute, such as the label text or a UUID, to ensure uniqueness.- id={`reason-${index + 1}`} + id={`reason-${label.replace(/\s+/g, '-').toLowerCase()}`}apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts (1)
407-410
: Inconsistent Naming of Max Duration PropertiesThere's an inconsistency in the naming of the max duration properties. In the
lockedTaskRun
update (lines 407-410), the property is namedmaxDurationInSeconds
, while in the execution payload (line 1075), it's referred to asmaxDuration
. For clarity and to prevent potential confusion, consider standardizing the property name across the codebase.Also applies to: 1075-1075
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (9)
docs/images/supabase-create-webhook-2.png
is excluded by!**/*.png
docs/images/supabase-create-webhook-3.png
is excluded by!**/*.png
docs/images/supabase-new-table-2.png
is excluded by!**/*.png
docs/images/supabase-new-table-3.png
is excluded by!**/*.png
docs/images/supabase-new-table-4.png
is excluded by!**/*.png
docs/images/supabase-run-result.png
is excluded by!**/*.png
docs/images/supabase-table-result.png
is excluded by!**/*.png
docs/images/supabase-trigger-screenshot.png
is excluded by!**/*.png
docs/runs/max-duration-error.png
is excluded by!**/*.png
📒 Files selected for processing (66)
- .changeset/config.json (1 hunks)
- .github/workflows/unit-tests.yml (1 hunks)
- .gitmodules (1 hunks)
- apps/webapp/app/assets/icons/TimedOutIcon.tsx (1 hunks)
- apps/webapp/app/components/primitives/Checkbox.tsx (3 hunks)
- apps/webapp/app/components/primitives/Dialog.tsx (1 hunks)
- apps/webapp/app/components/primitives/Select.tsx (1 hunks)
- apps/webapp/app/components/primitives/TreeView/TreeView.tsx (1 hunks)
- apps/webapp/app/components/runs/v3/RunInspector.tsx (1 hunks)
- apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (7 hunks)
- apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (4 hunks)
- apps/webapp/app/database-types.ts (1 hunks)
- apps/webapp/app/db.server.ts (1 hunks)
- apps/webapp/app/env.server.ts (1 hunks)
- apps/webapp/app/models/taskRun.server.ts (1 hunks)
- apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (1 hunks)
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (1 hunks)
- apps/webapp/app/presenters/v3/RunPresenter.server.ts (3 hunks)
- apps/webapp/app/presenters/v3/SpanPresenter.server.ts (4 hunks)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.environments/ConfigureEndpointSheet.tsx (1 hunks)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.environments/route.tsx (1 hunks)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.runs.$runParam/route.tsx (2 hunks)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.v3.billing/route.tsx (1 hunks)
- apps/webapp/app/routes/_app.orgs.$organizationSlug/route.tsx (0 hunks)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.v3.$projectParam.runs.$runParam.spans.$spanParam/route.tsx (1 hunks)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (15 hunks)
- apps/webapp/app/services/runExecutionRateLimiter.server.ts (2 hunks)
- apps/webapp/app/services/worker.server.ts (6 hunks)
- apps/webapp/app/v3/friendlyIdentifiers.ts (1 hunks)
- apps/webapp/app/v3/marqs/devQueueConsumer.server.ts (2 hunks)
- apps/webapp/app/v3/marqs/index.server.ts (20 hunks)
- apps/webapp/app/v3/marqs/marqsKeyProducer.server.ts (1 hunks)
- apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts (3 hunks)
- apps/webapp/app/v3/marqs/types.ts (2 hunks)
- apps/webapp/app/v3/queueSizeLimits.server.ts (1 hunks)
- apps/webapp/app/v3/requeueTaskRun.server.ts (1 hunks)
- apps/webapp/app/v3/services/completeAttempt.server.ts (1 hunks)
- apps/webapp/app/v3/services/createBackgroundWorker.server.ts (2 hunks)
- apps/webapp/app/v3/services/createTaskRunAttempt.server.ts (3 hunks)
- apps/webapp/app/v3/services/enqueueDelayedRun.server.ts (2 hunks)
- apps/webapp/app/v3/services/expireEnqueuedRun.server.ts (1 hunks)
- apps/webapp/app/v3/services/finalizeTaskRun.server.ts (2 hunks)
- apps/webapp/app/v3/services/resumeTaskDependency.server.ts (2 hunks)
- apps/webapp/app/v3/services/triggerScheduledTask.server.ts (1 hunks)
- apps/webapp/app/v3/services/triggerTask.server.ts (4 hunks)
- apps/webapp/app/v3/taskStatus.ts (2 hunks)
- apps/webapp/app/v3/utils/maxDuration.ts (1 hunks)
- apps/webapp/package.json (2 hunks)
- apps/webapp/tsconfig.json (1 hunks)
- docker/Dockerfile (1 hunks)
- docs/config/config-file.mdx (1 hunks)
- docs/github-actions.mdx (0 hunks)
- docs/guides/frameworks/introduction.mdx (0 hunks)
- docs/guides/frameworks/prisma.mdx (1 hunks)
- docs/guides/frameworks/sequin.mdx (8 hunks)
- docs/guides/frameworks/supabase-edge-functions-basic.mdx (1 hunks)
- docs/guides/frameworks/supabase-edge-functions-database-webhooks.mdx (2 hunks)
- docs/guides/introduction.mdx (1 hunks)
- docs/introduction.mdx (1 hunks)
- docs/mint.json (3 hunks)
- docs/open-source-self-hosting.mdx (3 hunks)
- docs/runs/max-duration.mdx (1 hunks)
- docs/snippets/supabase-docs-cards.mdx (1 hunks)
- docs/snippets/supabase-prerequisites.mdx (1 hunks)
- docs/tasks/overview.mdx (6 hunks)
- docs/triggering.mdx (1 hunks)
💤 Files with no reviewable changes (3)
- apps/webapp/app/routes/_app.orgs.$organizationSlug/route.tsx
- docs/github-actions.mdx
- docs/guides/frameworks/introduction.mdx
✅ Files skipped from review due to trivial changes (4)
- .gitmodules
- apps/webapp/app/components/primitives/Select.tsx
- apps/webapp/app/v3/friendlyIdentifiers.ts
- docs/guides/frameworks/prisma.mdx
🧰 Additional context used
🪛 LanguageTool
docs/guides/frameworks/sequin.mdx
[uncategorized] ~27-~27: The official spelling of this programming framework is “Next.js”.
Context: ...w Trigger.dev's Next.js setup guide to setup your project. You can return ...(NODE_JS)
[grammar] ~27-~27: The word “setup” is a noun. The verb is spelled with a white space.
Context: ... guide](/guides/frameworks/nextjs) to setup your project. You can return to this gu...(NOUN_VERB_CONFUSION)
[uncategorized] ~87-~87: Loose punctuation mark.
Context: ...N.stringify(embedding), }; } }); ```` ```ts utils.ts import pg from ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~330-~330: The official spelling of this programming framework is “Next.js”.
Context: ...ured and logged. - Deploy to production and...(NODE_JS)
docs/guides/frameworks/supabase-edge-functions-database-webhooks.mdx
[typographical] ~20-~20: Consider adding two commas here.
Context: ...ebhook triggers an Edge Function when a row including a video URL is inserted into a table - The Edge Fun...(UNLIKE_COMMA)
[style] ~79-~79: Consider a more expressive alternative.
Context: ...re the video URL and transcription. To do this, click on 'Table Editor' <Icon ico...(DO_ACHIEVE)
[duplication] ~79-~79: Possible typo: you repeated a word
Context: ...n. To do this, click on 'Table Editor' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~85-~85: Possible typo: you repeated a word
Context: ...calledvideo_url
with the typetext
<Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~85-~85: Possible typo: you repeated a word
Context: ...anscription, also with the type
text` <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[style] ~265-~265: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...s to your Trigger.dev project You will need to add yourDEEPGRAM_SECRET_KEY
, `SUPABA...(REP_NEED_TO_VB)
[duplication] ~297-~297: Possible typo: you repeated a word
Context: ... to use, navigate to 'Project settings' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~297-~297: Possible typo: you repeated a word
Context: ...lor="A8FF53" />, click 'Edge Functions' <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~297-~297: Possible typo: you repeated a word
Context: ...nu, and then click the 'Add new secret' <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~299-~299: Possible typo: you repeated a word
Context: ...3" /> button. AddTRIGGER_SECRET_KEY
<Icon icon="circle-4" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~355-~355: Possible typo: you repeated a word
Context: ...ect dashboard, click 'Project settings' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~355-~355: Possible typo: you repeated a word
Context: ...} color="A8FF53" />, then the 'API' tab <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~355-~355: Possible typo: you repeated a word
Context: ...anon
public
API key from the table <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~359-~359: Possible typo: you repeated a word
Context: ...se-api-key.png) Then, go to 'Database' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~359-~359: Possible typo: you repeated a word
Context: ...} color="A8FF53" /> click on 'Webhooks' <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~359-~359: Possible typo: you repeated a word
Context: ... />, and then click 'Create a new hook' <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[style] ~396-~396: Consider a more expressive alternative.
Context: ... yourvideo_transcriptions
table. To do this, go back to your Supabase project ...(DO_ACHIEVE)
[duplication] ~396-~396: Possible typo: you repeated a word
Context: ...ject dashboard, click on 'Table Editor' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~396-~396: Possible typo: you repeated a word
Context: ...ick on thevideo_transcriptions
table <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~396-~396: Possible typo: you repeated a word
Context: ..., and then click 'Insert', 'Insert Row' <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~400-~400: Possible typo: you repeated a word
Context: ...rvideo_url
, with a public video url. <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~406-~406: Possible typo: you repeated a word
Context: .../cloud.trigger.dev) project 'Runs' list <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~406-~406: Possible typo: you repeated a word
Context: ...processingvideoProcessAndUpdate
task <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
docs/runs/max-duration.mdx
[uncategorized] ~6-~6: Did you mean: “By default,”?
Context: ...ximum duration for a task to run." --- By default tasks can execute indefinitely, which c...(BY_DEFAULT_COMMA)
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...efault-machine). This will apply to all tasks unless you override it on a specific ta...(AI_HYDRA_LEO_MISSING_COMMA)
docs/snippets/supabase-prerequisites.mdx
[uncategorized] ~22-~22: A comma might be missing here.
Context: ...o> If your project has apackage.json
file you can skip this step. This is...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
docs/tasks/overview.mdx
[uncategorized] ~125-~125: Did you mean: “By default,”?
Context: ... }, }); ``` ###maxDuration
option By default tasks can execute indefinitely, which c...(BY_DEFAULT_COMMA)
docs/triggering.mdx
[style] ~746-~746: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...r more information. ###maxDuration
View our [maxDuration doc](/runs/max-duratio...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (106)
.changeset/config.json (1)
10-10
: LGTM: Compact formatting of thefixed
array.The
fixed
array has been reformatted to a single line. This change is purely cosmetic and does not affect functionality. The new format is more compact and appropriate for this short array..github/workflows/unit-tests.yml (1)
30-32
: New step added to generate Prisma Client before running tests.The addition of the "Generate Prisma Client" step is a good practice. It ensures that the most up-to-date Prisma Client is available before running the unit tests, which can prevent issues related to outdated or missing client code.
A few points to consider:
- The placement of this step is correct, as it's after dependency installation and before running the tests.
- The step aligns well with the workflow's purpose of preparing the environment for and then running unit tests.
- The command
pnpm run generate
seems appropriate, assuming it's defined in yourpackage.json
scripts.To ensure the
generate
script is correctly defined, please run the following:This will display the
generate
script if it exists in yourpackage.json
. Ensure it's correctly set to generate the Prisma Client.✅ Verification successful
To ensure that the
generate
script is correctly set up and that Turbo is properly orchestrating the Prisma Client generation across your projects, please run the following scripts:
The
generate
script is correctly configured and ensures Prisma Client is generated as intended before running unit tests.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the 'generate' script in package.json jq '.scripts.generate' package.jsonLength of output: 56
Script:
#!/bin/bash # Find all 'generate' scripts in package.json files across the repository fd package.json | xargs -I {} sh -c "echo 'In {}:'; jq '.scripts.generate' {} || echo 'No generate script found.'" # Verify Turbo configuration for running 'generate' tasks cat turbo.jsonLength of output: 4087
apps/webapp/app/v3/utils/maxDuration.ts (1)
4-6
: LGTM! Well-implemented clamping function.The
clampMaxDuration
function efficiently implements the clamping logic usingMath.min
andMath.max
. The function name clearly describes its purpose.docs/snippets/supabase-prerequisites.mdx (2)
18-18
: LGTM: Simplified step titleThe updated title accurately reflects the simplified setup process, focusing only on creating the
package.json
file. This change aligns well with the PR objectives and improves clarity for users.
22-24
: LGTM: Consistent update to skip noteThe updated note accurately reflects the focus on the
package.json
file, maintaining consistency with the earlier changes. This helps prevent confusion for users who already have the necessary file.🧰 Tools
🪛 LanguageTool
[uncategorized] ~22-~22: A comma might be missing here.
Context: ...o> If your project has apackage.json
file you can skip this step. This is...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
docs/snippets/supabase-docs-cards.mdx (2)
7-8
: Improved clarity and consistency in card representation.The changes to the first card's title and icon enhance user understanding and maintain consistency:
- Adding "guide" to the title clearly indicates the content type.
- Changing the icon from "bolt" to "book" better represents a guide or tutorial.
These updates align well with the PR's objective of improving user experience and clarity.
14-15
: Enhanced consistency and clarity in card presentation.The modifications to the second card's title and icon improve overall consistency and user understanding:
- Simplifying the title to "Database webhooks guide" provides a clearer description of the content.
- Changing the icon to "book" maintains consistency with the first card and accurately represents a guide.
These updates contribute to a more cohesive and user-friendly documentation structure.
apps/webapp/tsconfig.json (2)
35-36
: New internal module added: @internal/zod-workerA new internal module
@internal/zod-worker
has been added to the project. This addition suggests new functionality related to Zod (a TypeScript-first schema validation library) in a worker context.Ensure that:
- The module is properly implemented in the specified path.
- Its usage is documented for other developers.
- If it replaces any existing functionality, the old code is properly deprecated or removed.
To verify the implementation and usage of the new module, run the following script:
✅ Verification successful
@internal/zod-worker has been successfully implemented and is actively used within the project.
- The implementation file exists at
internal-packages/zod-worker/src/index.ts
.- The module is imported in the following files:
apps/webapp/app/services/worker.server.ts
apps/webapp/app/services/runExecutionRateLimiter.server.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the implementation and usage of @internal/zod-worker echo "Checking @internal/zod-worker implementation..." ls -l ../../internal-packages/zod-worker/src/index.ts echo "Checking for @internal/zod-worker usage..." rg --type typescript --type javascript "from ['\"]@internal/zod-worker" ./app echo "If the implementation file exists and usage is found, the module is properly set up."Length of output: 759
Script:
#!/bin/bash # Description: Verify the implementation and usage of @internal/zod-worker without relying on --type flags echo "Checking @internal/zod-worker implementation..." if [ -f ../../internal-packages/zod-worker/src/index.ts ]; then echo "@internal/zod-worker implementation exists." else echo "Error: @internal/zod-worker implementation file does not exist at ../../internal-packages/zod-worker/src/index.ts" exit 1 fi echo "Checking for @internal/zod-worker usage in .ts and .js files within ./apps/webapp..." rg "from ['\"]@internal/zod-worker['\"]" --glob '*.ts' --glob '*.js' ./apps/webapp echo "Verification complete."Length of output: 411
Script:
#!/bin/bash # Description: Locate the implementation files of @internal/zod-worker within the repository echo "Searching for zod-worker implementation files..." rg --files | rg 'zod-worker/src/index\.ts$' echo "Searching for any occurrences of @internal/zod-worker in the codebase..." rg "@internal/zod-worker" --glob '*.ts' --glob '*.js' echo "If implementation files are found and usage references exist, please verify their correctness."Length of output: 814
27-36
: Verify project-wide impact of path changesThe restructuring of package paths and addition of a new internal module may have broader implications for the project.
To ensure these changes haven't introduced any issues, run the following verification steps:
✅ Verification successful
Path changes verified successfully
The verification process has been completed, and no issues were found. The restructuring of package paths and addition of the new internal module appear to have been applied consistently across the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify project-wide impact of path changes echo "Checking for any 'packages' references that should now be 'internal-packages'..." rg --type typescript --type javascript "from ['\"]../../packages/(database|otlp-importer|emails)" ./app echo "Verifying no unintended changes to import statements..." git diff --name-only | xargs rg --type typescript --type javascript "import .* from" echo "If no unexpected results are shown above, the path changes have been applied consistently."Length of output: 759
Script:
#!/bin/bash # Description: Verify project-wide impact of path changes with correct file types echo "Checking for any 'packages' references that should now be 'internal-packages'..." rg --type ts --type js "from ['\"]../../packages/(database|otlp-importer|emails)" ./app echo "Verifying no unintended changes to import statements..." git diff --name-only | fd --extension ts --extension js | xargs rg "import .* from" echo "If no unexpected results are shown above, the path changes have been applied consistently."Length of output: 429664
apps/webapp/app/assets/icons/TimedOutIcon.tsx (4)
1-2
: LGTM: Component declaration and props are well-defined.The
TimedOutIcon
component is correctly declared as an exported function with proper TypeScript typing for the optionalclassName
prop. This follows React best practices and allows for flexible styling.
11-16
: LGTM: Path element is well-defined and follows SVG best practices.The path element is correctly implemented with appropriate fill and clip rules. The use of
currentColor
for the fill attribute allows for easy color customization based on the parent element's text color.
17-19
: LGTM: Component return statement is correct and concise.The component correctly returns the SVG element without any unnecessary wrapping elements, following React best practices.
1-19
: Great implementation of the TimedOutIcon component!This new component aligns well with the PR objectives, providing a visual representation for the new "TIMED_OUT" status. The implementation is clean, reusable, and follows React and SVG best practices. It will be a valuable addition to the UI for representing timed-out tasks.
A few key points:
- The component is properly typed with TypeScript.
- The SVG implementation allows for easy styling and color inheritance.
- The icon design clearly communicates a "timed out" state.
Great job on this addition to the project!
apps/webapp/app/v3/services/expireEnqueuedRun.server.ts (2)
5-5
: LGTM: New import for workerQueueThe new import for
workerQueue
is correctly added and necessary for the new static methods.
Line range hint
1-95
: Verify integration with existing codebaseThe new
dequeue
andenqueue
static methods are well-implemented and provide valuable functionality for managing the expiration of enqueued runs. However, it's important to ensure that these new methods are properly integrated with the existing codebase.Please run the following script to check for potential usage of these new methods and to verify if any updates to the
call
method or other parts of the codebase are necessary:Based on the results, consider if any updates to the
call
method or other parts of the codebase are necessary to fully utilize these new methods.apps/webapp/app/database-types.ts (1)
45-45
: LGTM: Addition ofTIMED_OUT
status toTaskRunStatus
The addition of the
TIMED_OUT
status toTaskRunStatus
is consistent with the existing pattern and doesn't break type satisfaction. This enhancement aligns with the PR objectives of improving task management.apps/webapp/app/v3/requeueTaskRun.server.ts (1)
Line range hint
1-100
: Overall, the changes look good and enhance task run management.The addition of the "TIMED_OUT" status is well-implemented and consistent with the existing code structure. The file maintains good practices in terms of logging, error handling, and type safety. The suggested improvements for grouping similar statuses and adding consistent error handling are minor and aimed at enhancing readability and robustness.
docker/Dockerfile (1)
Line range hint
58-71
: Verify path consistency in the runner stageWhile there are no visible changes in the runner stage, it's crucial to ensure that the paths used here are still correct, given the modifications made in earlier stages (e.g., the change in the Prisma schema path).
Please run the following script to check for any inconsistencies:
This script will help identify any potential inconsistencies or overlooked path updates in the Dockerfile.
docs/introduction.mdx (2)
22-22
: LGTM: Improved clarity in Quick Start Guide descriptionThe addition of "in 3 minutes" provides a clear expectation for users, which is a good improvement to the documentation.
22-33
: Overall documentation improvements are well-executedThe changes in this file successfully enhance the clarity and user-friendliness of the Trigger.dev v3 documentation. The modifications to the "Getting started" section, including more specific descriptions and examples, provide users with clearer expectations and better guidance. These improvements align well with the PR objectives of enhancing user experience and clarity in the documentation.
apps/webapp/app/v3/taskStatus.ts (2)
Line range hint
1-105
: Summary: Changes are correct and consistent with the module's purpose.The additions of "TIMED_OUT" to both FINAL_RUN_STATUSES and FAILED_RUN_STATUSES are appropriate and maintain the consistency of the module. These changes correctly expand the set of final and failed statuses, ensuring that timed-out tasks are properly categorized.
The impact on dependent functions (isFinalRunStatus and isFailedRunStatus) has been considered, and no issues were found. The verification script provided earlier will help ensure there are no unintended impacts elsewhere in the codebase.
A minor suggestion was made to improve consistency by using the TypeScript satisfies clause for FAILED_RUN_STATUSES.
Overall, these changes enhance the task status management capabilities of the module without introducing any apparent issues.
44-44
: LGTM: Addition of "TIMED_OUT" to FINAL_RUN_STATUSES is correct and consistent.The addition of "TIMED_OUT" to the FINAL_RUN_STATUSES array is appropriate and aligns with the array's purpose. This change will be automatically reflected in the FINAL_RUN_STATUSES type and the isFinalRunStatus function, ensuring consistent behavior across the module.
To verify the impact of this change, let's check for any other occurrences or dependencies:
✅ Verification successful
To successfully verify the changes, let's adjust the search to target
.ts
files directly:
Verified: "TIMED_OUT" has been correctly added to FINAL_RUN_STATUSES with no additional dependencies or hardcoded lists impacted.
The addition of "TIMED_OUT" to the
FINAL_RUN_STATUSES
array is properly reflected across the codebase. No other dependencies or hardcoded lists were found that require updates, ensuring consistent behavior and maintaining code integrity.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other occurrences of FINAL_RUN_STATUSES and potential impact # Search for other uses of FINAL_RUN_STATUSES echo "Searching for other uses of FINAL_RUN_STATUSES:" rg --type typescript "FINAL_RUN_STATUSES" -C 3 # Search for any hardcoded lists of final statuses that might need updating echo "Searching for potential hardcoded lists of final statuses:" rg --type typescript -e "CANCELED.*COMPLETED_SUCCESSFULLY.*COMPLETED_WITH_ERRORS.*INTERRUPTED.*SYSTEM_FAILURE.*EXPIRED.*CRASHED" -C 3Length of output: 484
Script:
#!/bin/bash # Description: Check for other occurrences of FINAL_RUN_STATUSES and potential impact # Search for other uses of FINAL_RUN_STATUSES echo "Searching for other uses of FINAL_RUN_STATUSES:" rg "FINAL_RUN_STATUSES" -C 3 --glob "*.ts" # Search for any hardcoded lists of final statuses that might need updating echo "Searching for potential hardcoded lists of final statuses:" rg -e "CANCELED.*COMPLETED_SUCCESSFULLY.*COMPLETED_WITH_ERRORS.*INTERRUPTED.*SYSTEM_FAILURE.*EXPIRED.*CRASHED" -C 3 --glob "*.ts"Length of output: 3091
apps/webapp/app/v3/marqs/types.ts (1)
29-29
: LGTM: New methodenvQueueKey
is a good addition.The new
envQueueKey
method is a logical extension to theMarQSKeyProducer
interface. It maintains consistency with existing methods in terms of naming convention and parameter types. This addition enhances the interface's capability to generate keys for environment-specific queues.docs/guides/introduction.mdx (3)
2-4
: LGTM: Updated frontmatter reflects expanded contentThe changes to the title and description accurately represent the broader scope of the document, aligning well with the PR objectives of enhancing user experience and providing more comprehensive information.
7-11
: LGTM: New imports for framework cardsThe added import statements are necessary for the new "Frameworks" section and follow a consistent naming convention. This addition enhances the document's structure and visual organization.
23-30
: LGTM: New "Guides" section provides valuable resourcesThe addition of the "Guides" section with a clear table of setup guides enhances the document's utility. The table format is easy to read, and the descriptions are concise and informative. This section aligns well with the PR objective of providing comprehensive setup information.
apps/webapp/app/components/primitives/Dialog.tsx (1)
85-85
: LGTM! Verify visual alignment in different scenarios.The change from
sm:justify-end
tosm:justify-between
in the DialogFooter's className is appropriate and aligns with the PR objectives of implementing new confirmation modals. This modification will create more space between footer elements, potentially improving the layout for new feedback forms or buttons.To ensure this change doesn't negatively impact existing dialogs, please verify the visual alignment in various scenarios:
- Dialogs with a single button
- Dialogs with multiple buttons
- Dialogs with other footer content (e.g., text, links)
Consider running the following command to find all usages of DialogFooter:
This will help identify all the places where the footer layout might be affected.
apps/webapp/app/v3/services/resumeTaskDependency.server.ts (2)
Line range hint
1-101
: Summary: Enhancements to message handling in ResumeTaskDependencyServiceThe changes in this file focus on improving the message payload by including the
createdAt
timestamp. This enhancement applies to both theenqueueMessage
and the newly introducedreplaceMessage
calls. These modifications align well with the PR objectives and provide valuable timing information for tracking and debugging purposes.The core logic of the
ResumeTaskDependencyService
remains intact, ensuring that the existing functionality is preserved while adding this new feature. The consistent application of the timestamp across different message handling scenarios is a positive aspect of these changes.Overall, these modifications appear to be well-implemented and should improve the system's ability to track and manage task dependencies effectively.
88-101
: LGTM! Verify the behavior ofreplaceMessage
.The switch to
replaceMessage
and the addition ofdependentRun.createdAt.getTime()
are good improvements. This change maintains consistency with the earlierenqueueMessage
call and potentially improves message handling.Please verify the behavior of the
replaceMessage
method:
- Confirm that it correctly updates or replaces existing messages rather than creating duplicates.
- Ensure that the
createdAt
timestamp is being used as intended within the message queue system.You can use the following script to check for any other usages of
replaceMessage
in the codebase:✅ Verification successful
LGTM! The
replaceMessage
method is consistently used across the codebase.No inconsistent usages detected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of replaceMessage to ensure consistent usage rg --type typescript 'replaceMessage'Length of output: 72
Script:
#!/bin/bash # Search for other usages of replaceMessage in TypeScript files using glob patterns rg 'replaceMessage' --glob '*.ts' --glob '*.tsx'Length of output: 1094
apps/webapp/app/presenters/v3/RunPresenter.server.ts (2)
24-24
: LGTM: Method signature update is appropriate.The addition of the
showDeletedLogs
parameter to thecall
method signature is consistent with the PR objectives. It provides a way to control the visibility of deleted logs, which aligns with the described changes in the PR summary.Also applies to: 30-30
77-78
: LGTM: Log visibility logic is well-implemented.The new logic for determining log visibility is well-structured:
- The
showLogs
constant effectively combines the newshowDeletedLogs
parameter with the existinglogsDeletedAt
check.- The
logsDeletedAt
property inrunData
is consistently set based onshowDeletedLogs
.- The early return when
!showLogs
is an efficient optimization.These changes align well with the PR objectives and provide a clean implementation of the new feature.
Also applies to: 88-88, 100-105
apps/webapp/app/routes/_app.orgs.$organizationSlug.v3.billing/route.tsx (2)
Line range hint
85-87
: LGTM! Proper usage of the newplanLabel
function.The
planLabel
function is correctly implemented within theChoosePlanPage
component. The arguments passed to the function (v3Subscription?.plan
,v3Subscription?.canceledAt !== undefined
, andperiodEnd
) are appropriate and consistent with the function's implementation.This usage enhances the user experience by providing clear and context-specific information about the current subscription plan.
Line range hint
1-147
: Overall, the changes look good and align with the PR objectives.The modifications to this file successfully implement the enhanced plan feedback and billing period information as described in the PR objectives. The new
planLabel
function and the addition of theperiodEnd
prop to thePricingPlans
component improve the user experience by providing clear, context-specific information about subscription plans and billing periods.A few minor suggestions were made to further improve code clarity and consistency:
- Adding a comment to explain the purpose of the new
periodEnd
prop.- Updating the
planLabel
function to handle cancellations for enterprise plans consistently with other plan types.These changes enhance the billing information presented to users and should contribute to a better overall user experience when managing subscriptions.
docs/runs/max-duration.mdx (3)
43-55
: LGTM!The "Configuring a default max duration" section provides clear instructions on setting a default
maxDuration
in the config file. The code example is well-commented and includes helpful information about locating the project ref.🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...efault-machine). This will apply to all tasks unless you override it on a specific ta...(AI_HYDRA_LEO_MISSING_COMMA)
89-118
: LGTM!The "Configuring for a run" section provides clear explanations and helpful code examples for setting and disabling
maxDuration
for specific runs. The inclusion of import statements in the code examples is particularly useful for developers to understand the required modules.
1-138
: Overall, excellent documentation with minor suggestions for improvement.This new documentation file provides comprehensive and well-structured information about the
maxDuration
feature. The explanations are clear, and the code examples are helpful and relevant. The suggested improvements throughout the review are minor and aimed at enhancing clarity, consistency, and completeness of the information.Key points for consideration:
- Adding a comma after "By default" in the introduction.
- Providing more context for the maximum duration limit of ~68 years.
- Maintaining consistency in code comments across examples.
- Expanding the explanation of how exceeding
maxDuration
affects lifecycle functions.These small adjustments will further improve an already strong documentation file.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: Did you mean: “By default,”?
Context: ...ximum duration for a task to run." --- By default tasks can execute indefinitely, which c...(BY_DEFAULT_COMMA)
[uncategorized] ~45-~45: Possible missing comma found.
Context: ...efault-machine). This will apply to all tasks unless you override it on a specific ta...(AI_HYDRA_LEO_MISSING_COMMA)
apps/webapp/app/v3/marqs/marqsKeyProducer.server.ts (1)
131-139
: Summary: New methods enhance key generation capabilities.The two new methods,
envQueueKeyFromQueue
andenvQueueKey
, are valuable additions to theMarQSShortKeyProducer
class. They provide new ways to generate keys for environment queues, which aligns well with the class's overall purpose. The implementations are consistent with the existing code style and patterns.Minor suggestions were made to improve robustness and consistency, but overall, these changes are well-implemented and ready for integration.
apps/webapp/app/components/primitives/Checkbox.tsx (4)
64-64
: LGTM: Addition oflabelClassName
enhances customization options.The new optional
labelClassName
property allows for more flexible styling of the checkbox label, which is a good improvement in component customization.
82-82
: LGTM: Renaming improves code clarity.Renaming
labelClassName
toexternalLabelClassName
in the component props enhances code readability by clearly distinguishing it from the internallabelClassName
.
153-156
: LGTM: Improved label styling and click handling.The changes enhance the component in two ways:
- The label's className now combines both internal (
labelClassName
) and external (externalLabelClassName
) styles, allowing for flexible customization.- The addition of
onClick={(e) => e.preventDefault()}
to the label ensures proper checkbox behavior by preventing unintended default actions.These improvements contribute to better component flexibility and user interaction handling.
Line range hint
1-200
: Overall assessment: Excellent improvements to the Checkbox component.The changes made to the Checkbox component enhance its flexibility and usability:
- The addition of
labelClassName
toCheckboxProps
allows for more customizable label styling.- The renaming of
labelClassName
toexternalLabelClassName
in the component props improves code clarity.- The updated label's className assignment combines internal and external styles effectively.
- The new
onClick
handler for the label ensures proper checkbox behavior.These improvements align well with the PR objectives of enhancing user interface components. The changes are well-implemented and maintain the component's existing design patterns.
apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (6)
18-18
: LGTM: New icon import added correctly.The
TimedOutIcon
import is correctly placed and follows the existing import structure.
32-32
: LGTM: New "TIMED_OUT" status added consistently.The "TIMED_OUT" status has been correctly added to both
allTaskRunStatuses
andfilterableTaskRunStatuses
arrays, maintaining consistency in their structure.Also applies to: 50-50
72-72
: LGTM: Clear description added for "TIMED_OUT" status.The description for the "TIMED_OUT" status is clear and accurately explains the reason for this status.
148-149
: LGTM: Icon case added for "TIMED_OUT" status.The case for the "TIMED_OUT" status has been correctly added to the
TaskRunStatusIcon
function, returning the appropriate icon with consistent className application.
184-185
: LGTM: Color and title added for "TIMED_OUT" status.The "TIMED_OUT" status has been correctly added to both
runStatusClassNameColor
andrunStatusTitle
functions:
- The color class "text-error" is consistent with other error states.
- The title "Timed out" is clear and follows the naming convention of other status titles.
Also applies to: 222-223
Line range hint
1-226
: Overall assessment: Well-implemented new status.The addition of the "TIMED_OUT" status has been implemented consistently throughout the file. All necessary components (arrays, descriptions, icons, colors, and titles) have been updated to include the new status. The changes are well-integrated with the existing code structure and maintain the established patterns.
docs/mint.json (4)
91-98
: LGTM: New redirects align with PR objectives.The added redirects for
/guides/frameworks/introduction
and/guides/examples/intro
to/guides/introduction
are consistent with the PR's goal of restructuring the documentation. This change will ensure users are directed to the appropriate content after the reorganization.
162-162
: Approved, but please clarify the purpose of "runs/max-duration".The addition of "runs/max-duration" to the "Writing tasks" navigation group seems appropriate. However, could you provide more context on this new entry? It's not explicitly mentioned in the PR objectives, and it would be helpful to understand its relevance to the changes being made.
261-273
: LGTM: Restructuring of "Frameworks" group.The removal of Prisma and Sequin entries from the "Frameworks" group while retaining the Remix entry aligns with the documentation restructuring mentioned in the PR summary. This change contributes to a more organized and focused documentation structure.
Line range hint
291-309
: LGTM: Removal of redundant "guides/examples/intro" entry.The removal of the "guides/examples/intro" entry from the "Example tasks" group is consistent with the new redirect added earlier. This change helps streamline the documentation structure and avoids redundancy, aligning well with the PR objectives.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.environments/ConfigureEndpointSheet.tsx (1)
25-25
: LGTM! Verify type consistency across the codebase.The change from a relative import to an absolute import from
@trigger.dev/database
looks good. This modification suggests a move towards centralizing type definitions, which can improve maintainability.To ensure this change doesn't introduce any inconsistencies, please run the following script to check for any other usages of these types in the codebase:
✅ Verification successful
Re-running verification scripts with corrected TypeScript handling.
To address the previous errors, please execute the following updated scripts:
Re-running verification scripts with corrected file type handling and path patterns.
To address the previous errors, please execute the following updated scripts:
Re-running verification scripts with improved file path discovery and usage checks.
To accurately locate and verify the usage of
EndpointIndexStatus
andRuntimeEnvironmentType
inConfigureEndpointSheet.tsx
, please execute the following scripts:
Verified type consistency across the codebase.
The change from a relative import to an absolute import from
@trigger.dev/database
has been successfully verified. The typesEndpointIndexStatus
andRuntimeEnvironmentType
are correctly imported and consistently used withinConfigureEndpointSheet.tsx
and other relevant files, ensuring no inconsistencies or issues have been introduced.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other usages of EndpointIndexStatus and RuntimeEnvironmentType # Test: Search for other imports of these types echo "Searching for other imports of EndpointIndexStatus and RuntimeEnvironmentType:" rg --type typescript "import.*\{.*(?:EndpointIndexStatus|RuntimeEnvironmentType).*\}" # Test: Verify that the types are used consistently throughout the file echo "Verifying usage of EndpointIndexStatus and RuntimeEnvironmentType in the current file:" rg --type typescript "(?:EndpointIndexStatus|RuntimeEnvironmentType)" "apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.environments/ConfigureEndpointSheet.tsx"Length of output: 663
Script:
#!/bin/bash # Description: Check for other usages of EndpointIndexStatus and RuntimeEnvironmentType # Test: Search for other imports of these types echo "Searching for other imports of EndpointIndexStatus and RuntimeEnvironmentType:" rg "import.*\{.*(?:EndpointIndexStatus|RuntimeEnvironmentType).*\}" --glob "*.ts" --glob "*.tsx" # Test: Verify usage of EndpointIndexStatus and RuntimeEnvironmentType in the current file echo "Verifying usage of EndpointIndexStatus and RuntimeEnvironmentType in the current file:" rg "(?:EndpointIndexStatus|RuntimeEnvironmentType)" "apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.environments/ConfigureEndpointSheet.tsx"Length of output: 4099
Script:
#!/bin/bash # Description: Check for other usages of EndpointIndexStatus and RuntimeEnvironmentType # Test: Search for other imports of these types echo "Searching for other imports of EndpointIndexStatus and RuntimeEnvironmentType:" rg "import.*\{.*(?:EndpointIndexStatus|RuntimeEnvironmentType).*\}" --glob "*.ts" --glob "*.tsx" # Test: Verify usage of EndpointIndexStatus and RuntimeEnvironmentType in the current file echo "Verifying usage of EndpointIndexStatus and RuntimeEnvironmentType in the current file:" rg "(?:EndpointIndexStatus|RuntimeEnvironmentType)" "apps/webapp/app/routes/_app.orgs.*.projects.*.environments/ConfigureEndpointSheet.tsx"Length of output: 4105
Script:
#!/bin/bash # Description: Locate all instances of ConfigureEndpointSheet.tsx and verify type usage # Step 1: Find all paths to ConfigureEndpointSheet.tsx echo "Locating all ConfigureEndpointSheet.tsx files:" fd "ConfigureEndpointSheet\.tsx$" --type f # Step 2: For each found file, verify usage of EndpointIndexStatus and RuntimeEnvironmentType echo "Verifying usage of EndpointIndexStatus and RuntimeEnvironmentType in each ConfigureEndpointSheet.tsx file:" fd "ConfigureEndpointSheet\.tsx$" --type f | while read -r file; do echo "Checking file: $file" rg "(EndpointIndexStatus|RuntimeEnvironmentType)" "$file" doneLength of output: 1452
docs/tasks/overview.mdx (3)
174-174
: Approve concise wording ininit
function section.The removal of "it's worth mentioning that" improves the directness of the documentation. This change aligns with best practices for technical writing, making the information more accessible to readers.
192-192
: Approve consistent improvements in documentation wording.The removal of "it's worth mentioning that" across multiple sections (
cleanup
,middleware
,onSuccess
, andonFailure
) improves the overall clarity and conciseness of the documentation. These changes maintain consistency throughout the document and align with best practices for technical writing.Also applies to: 199-200, 263-263, 294-294
Line range hint
124-294
: Overall improvements enhance documentation quality and user experience.The changes made to this documentation file significantly improve its clarity, conciseness, and informativeness. The addition of the
maxDuration
option aligns with the PR objectives and provides users with valuable information about controlling task execution time. The consistent removal of unnecessary phrases across multiple sections enhances readability.These modifications will contribute to a better user experience for developers working with Trigger.dev tasks. Great job on improving the documentation!
🧰 Tools
🪛 LanguageTool
[uncategorized] ~125-~125: Did you mean: “By default,”?
Context: ... }, }); ``` ###maxDuration
option By default tasks can execute indefinitely, which c...(BY_DEFAULT_COMMA)
apps/webapp/app/presenters/v3/SpanPresenter.server.ts (3)
12-12
: LGTM: New import added correctly.The import of
getMaxDuration
is properly formatted and consistent with the existing import style. This new function will likely be used in conjunction with themaxDurationInSeconds
field.
73-73
: LGTM: New field added to select statement.The
maxDurationInSeconds
field has been correctly added to the select statement. This addition is consistent with the existing structure and maintains the alphabetical ordering of fields.
Line range hint
1-365
: Overall assessment: Changes enhance task run management with minor improvement suggested.The modifications to the
SpanPresenter
class effectively introduce support for maximum duration of task runs. These changes align well with the PR objectives of improving task management. The code additions are consistent with the existing structure and style.Key points:
- New import for
getMaxDuration
function added correctly.maxDurationInSeconds
field added to the database query.- New property added to the return object of
getRun
method.A minor suggestion was made to add a safety check when calling
getMaxDuration
to handle potential undefined values.apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (1)
273-275
: LGTM! Consider verifying the new status handling across the codebase.The addition of the "TIMED_OUT" case in the
apiStatusFromRunStatus
method is correct and consistent with the existing implementation. This change aligns with the PR objectives related to updating task run statuses.To ensure comprehensive implementation, please verify that the new "TIMED_OUT" status is handled appropriately in other relevant parts of the codebase. Run the following script to identify potential areas that may need updates:
✅ Verification successful
Verified: "TIMED_OUT" status is correctly handled throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for occurrences of TaskRunStatus and RunStatus enums or string literals # Test 1: Search for TaskRunStatus enum usage echo "Searching for TaskRunStatus usage:" rg --type typescript "TaskRunStatus[^a-zA-Z]" -g '!*.test.ts' # Test 2: Search for RunStatus enum usage echo "Searching for RunStatus usage:" rg --type typescript "RunStatus[^a-zA-Z]" -g '!*.test.ts' # Test 3: Search for status-related string literals echo "Searching for status-related string literals:" rg --type typescript '"(COMPLETED|FAILED|CANCELED|INTERRUPTED|CRASHED|SYSTEM_FAILURE|TIMED_OUT)"' -g '!*.test.ts' # Note: Review the results to ensure the new TIMED_OUT status is handled where necessaryLength of output: 595
Script:
#!/bin/bash # Description: Search for occurrences of TaskRunStatus and RunStatus enums or string literals # Test 1: Search for TaskRunStatus enum usage echo "Searching for TaskRunStatus usage:" rg "TaskRunStatus[^a-zA-Z]" -g '*.ts' -g '!*.test.ts' # Test 2: Search for RunStatus enum usage echo "Searching for RunStatus usage:" rg "RunStatus[^a-zA-Z]" -g '*.ts' -g '!*.test.ts' # Test 3: Search for status-related string literals echo "Searching for status-related string literals:" rg '"(COMPLETED|FAILED|CANCELED|INTERRUPTED|CRASHED|SYSTEM_FAILURE|TIMED_OUT)"' -g '*.ts' -g '!*.test.ts' # Note: Review the results to ensure the new TIMED_OUT status is handled where necessaryLength of output: 42793
apps/webapp/app/env.server.ts (1)
Line range hint
1-218
: Verify alignment with PR objectivesThe changes in this file, while well-implemented, don't appear to directly relate to the PR objectives mentioned in the summary (implementing plan feedback modals and tooltips). Could you please clarify how these queue size limits contribute to the stated goals of the PR?
To help understand the context, could you provide more information on:
- How these queue size limits relate to the plan feedback modals?
- Are there other files in this PR that implement the described modal and tooltip changes?
- If these changes are unrelated, should they be part of a separate PR?
apps/webapp/app/services/runExecutionRateLimiter.server.ts (2)
Line range hint
158-158
: Verify implementation of ZSET cleanup logicA comment has been added indicating that the cleanup of old job IDs is now essential for maintaining the ZSET's size. It's important to ensure that this cleanup logic is properly implemented and functioning as expected.
Please confirm that the cleanup logic is correctly implemented. You can use the following script to check the Lua code for the presence of the cleanup logic:
#!/bin/bash # Check for the presence of ZREMRANGEBYSCORE command in the afterTask Lua script rg --type typescript "ZREMRANGEBYSCORE.*setKey.*'-inf'.*timestamp - windowSize"If the script doesn't return any results, consider adding the cleanup logic to maintain the ZSET's size effectively.
14-14
: Verify consistency of import change across the projectThe import for
ZodWorkerRateLimiter
has been updated to use an internal package path. This change might be part of a larger refactoring effort.Please ensure that this import change is consistent across the project. Run the following script to check for any remaining imports from the old path:
If the script returns any results, consider updating those imports as well for consistency.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.environments/route.tsx (2)
42-42
: LGTM: Improved import statement for RuntimeEnvironmentTypeThe change from a relative import path to an absolute import path for
RuntimeEnvironmentType
is a good improvement. This modification:
- Simplifies the import structure
- Enhances maintainability
- Reduces the risk of import errors due to file relocations
The rest of the file's functionality remains unaffected by this change.
Line range hint
1-424
: Verified: No impact on existing functionalityAfter reviewing the entire file, I can confirm that:
- The usage of
RuntimeEnvironmentType
remains consistent with the previous implementation.- No other changes were required in the file due to the updated import statement.
- The existing functionality of the component is preserved.
This change is safe and doesn't introduce any breaking changes or require further modifications.
To further verify the impact of this change, you can run the following script:
This script will show all occurrences of
RuntimeEnvironmentType
in the file, allowing you to verify that it's being used correctly with the new import.apps/webapp/app/v3/services/createBackgroundWorker.server.ts (3)
Line range hint
1-424
: Summary of changes: Improved task duration handlingThe changes in this file introduce a new utility function
clampMaxDuration
to handle the maximum duration for tasks. This addition enhances the robustness of the task creation process by ensuring that duration limits are properly constrained.Key points:
- The new import of
clampMaxDuration
is correctly added.- The
createBackgroundTasks
function now usesclampMaxDuration
to set themaxDurationInSeconds
field.- These changes improve the safety and reliability of task duration settings.
The implementation looks good, but it's important to verify the behavior of
clampMaxDuration
to ensure it handles all possible input scenarios correctly.
160-160
: LGTM. Verify the behavior of clampMaxDuration.The use of
clampMaxDuration
to setmaxDurationInSeconds
is a good addition. It ensures that the maximum duration for tasks is within acceptable limits.To understand the behavior of
clampMaxDuration
, run the following script:#!/bin/bash # Extract and display the clampMaxDuration function implementation rg --type typescript -A 10 'export function clampMaxDuration' '../utils/maxDuration.ts'This will help verify that the function behaves as expected, particularly in edge cases (e.g., very large or negative durations).
18-18
: LGTM. Verify the imported function.The import of
clampMaxDuration
is correctly added. This function will be used to handle the maximum duration for tasks.To ensure the imported function exists and is exported correctly, run the following script:
✅ Verification successful
LGTM. The
clampMaxDuration
function is correctly imported and properly exported from../utils/maxDuration
.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence and export of clampMaxDuration function rg --type typescript 'export.*function clampMaxDuration' '../utils/maxDuration.ts'Length of output: 122
Script:
#!/bin/bash # Verify the existence and export of clampMaxDuration function rg 'export function clampMaxDuration' ../utils/maxDuration.tsLength of output: 127
Script:
#!/bin/bash # Search for all instances of maxDuration.ts in the repository fd 'maxDuration\.ts' --type fLength of output: 120
Script:
#!/bin/bash # Verify the existence and export of clampMaxDuration function rg 'export function clampMaxDuration' apps/webapp/app/v3/utils/maxDuration.tsLength of output: 143
docs/guides/frameworks/sequin.mdx (2)
Line range hint
1-7
: LGTM: Improved title and metadataThe updates to the title, sidebarTitle, and description enhance clarity and better reflect the content of the guide. These changes will help users understand the purpose of the document more quickly.
Line range hint
1-330
: Overall, excellent improvements with some minor enhancements suggestedThe changes made to this document significantly improve its structure, clarity, and content. The guide now provides a more comprehensive and user-friendly walkthrough of integrating Sequin with Trigger.dev for database-triggered tasks.
Key improvements include:
- Better title and metadata reflecting the content.
- Clearer formatting and structure throughout the document.
- Addition of a detailed
createEmbeddingForPost
task implementation.- Comprehensive instructions for setting up Sequin consumers and testing the integration.
The suggested enhancements, if implemented, will further improve the document by:
- Addressing potential security concerns in the API route setup.
- Providing more context and explanations in the Sequin consumer setup instructions.
- Improving the clarity of test instructions and adding a database verification step.
- Expanding the "Next steps" section with more specific guidance for production deployment and future development.
Additionally, addressing the minor language and formatting issues will polish the overall presentation of the guide.
Great work on improving this documentation! The suggested enhancements will make it even more valuable for users integrating Sequin with Trigger.dev.
apps/webapp/app/v3/marqs/devQueueConsumer.server.ts (2)
27-27
: LGTM: New import forgetMaxDuration
The import of
getMaxDuration
from "../utils/maxDuration" is appropriate and follows good module organization practices.
382-385
: LGTM: Dynamic calculation ofmaxDurationInSeconds
The use of
getMaxDuration
to calculatemaxDurationInSeconds
is a good improvement. It allows for more flexible duration management based on both the existing task run and the background task.To ensure this change doesn't introduce any unexpected behavior, please run the following verification:
This will help ensure that the new implementation is consistent with the existing system design and doesn't affect any dependent processes.
✅ Verification successful
Verified:
getMaxDuration
Implementation and UsageThe
getMaxDuration
function is correctly implemented and consistently used across the codebase. The dynamic calculation ofmaxDurationInSeconds
aligns with existing time-related constraints, ensuring flexibility and preventing duration mismatches.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of getMaxDuration and its impact # Check the implementation of getMaxDuration echo "Checking getMaxDuration implementation:" cat $(fd --type f --exec grep -l "export function getMaxDuration" {}) # Find all usages of getMaxDuration echo "\nChecking all usages of getMaxDuration:" rg "getMaxDuration\(" --type ts # Check for any existing time-related constraints or assumptions echo "\nChecking for existing time-related constraints:" rg "maxDurationInSeconds|timeoutInSeconds|MAX_DURATION" --type tsLength of output: 7657
docs/open-source-self-hosting.mdx (5)
6-7
: Excellent addition of a clear warning message.The new warning effectively communicates the limitations of the self-hosting guide and sets appropriate expectations for users. This addition enhances the document's clarity and helps prevent misunderstandings about the guide's scope.
28-30
: Improved clarity in the "Caveats" section.The rephrasing of this section provides a more direct and clearer explanation of the guide's limitations. It explicitly states that the guide won't result in a production-ready deployment, which is crucial information for users. The more precise language helps prevent misunderstandings about the guide's scope and purpose.
90-91
: Improved simplicity in Trigger.dev setup instructions.The removal of the specific branch checkout instruction simplifies the setup process and makes the guide more maintainable. This change reduces the likelihood of users encountering issues related to branch mismatches. The retained instructions for running the start script remain clear and concise.
6-6
: Enhanced clarity in tunnelling instructions.The expansion of the "Tunnelling" section with more detailed steps is a significant improvement. The emphasis on the necessity of exposing the webapp to the internet helps users understand the importance of this step. The clear, step-by-step approach makes the process easier to follow, which is particularly helpful for users who may be less familiar with Ngrok or tunnelling in general.
6-6
: Excellent overall improvements to the self-hosting guide.The changes made to this document significantly enhance its usefulness and clarity. The addition of new sections covering advanced topics like large payload handling, version locking, and authentication options provides a more comprehensive guide for self-hosting Trigger.dev. The improved organization, expanded instructions, and clear warnings about limitations and potential issues make this guide much more valuable for users considering self-hosting.
The document now offers a more complete picture of the self-hosting process and its considerations, which will help users make informed decisions and successfully set up their self-hosted instances. These improvements demonstrate a strong commitment to supporting the self-hosting community and ensuring users have the information they need to succeed.
docs/guides/frameworks/supabase-edge-functions-database-webhooks.mdx (9)
2-4
: Title and description updates enhance clarity and specificity.The updated title and description provide a more precise explanation of the guide's content, focusing on the specific use case of triggering a transcribing task when a row is added to a Supabase database table.
18-25
: Improved overview section with detailed workflow explanation.The revised overview section offers a clearer and more comprehensive explanation of the integration between Supabase and Trigger.dev. It outlines the specific steps involved in the workflow, including the use of FFmpeg for audio extraction and Deepgram for transcription.
🧰 Tools
🪛 LanguageTool
[typographical] ~20-~20: Consider adding two commas here.
Context: ...ebhook triggers an Edge Function when a row including a video URL is inserted into a table - The Edge Fun...(UNLIKE_COMMA)
30-34
: Updated prerequisites with additional requirements.The prerequisites section now includes important additions such as Docker Desktop and a Deepgram account. This ensures users have all necessary tools and accounts before starting the tutorial.
40-69
: Streamlined initial setup instructions.The initial setup instructions have been simplified and now focus on using the Trigger.dev CLI to initialize the project. This approach is more straightforward and user-friendly.
75-87
: Clear instructions for creating a new Supabase table.This new section provides step-by-step guidance on creating the necessary table in Supabase, including screenshots for visual aid. This addition enhances the guide's clarity and usability.
🧰 Tools
🪛 LanguageTool
[style] ~79-~79: Consider a more expressive alternative.
Context: ...re the video URL and transcription. To do this, click on 'Table Editor' <Icon ico...(DO_ACHIEVE)
[duplication] ~79-~79: Possible typo: you repeated a word
Context: ...n. To do this, click on 'Table Editor' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~85-~85: Possible typo: you repeated a word
Context: ...calledvideo_url
with the typetext
<Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~85-~85: Possible typo: you repeated a word
Context: ...anscription, also with the type
text` <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
289-347
: Clear instructions for creating and deploying the Supabase Edge Function.This section provides a step-by-step guide for setting up the Supabase Edge Function, including:
- Adding the Trigger.dev secret key to Supabase
- Creating the Edge Function using the Supabase CLI
- Deploying the Edge Function
The provided code for the Edge Function looks correct and includes necessary type definitions and imports.
🧰 Tools
🪛 LanguageTool
[duplication] ~297-~297: Possible typo: you repeated a word
Context: ... to use, navigate to 'Project settings' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~297-~297: Possible typo: you repeated a word
Context: ...lor="A8FF53" />, click 'Edge Functions' <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~297-~297: Possible typo: you repeated a word
Context: ...nu, and then click the 'Add new secret' <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~299-~299: Possible typo: you repeated a word
Context: ...3" /> button. AddTRIGGER_SECRET_KEY
<Icon icon="circle-4" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
353-390
: Detailed guide for creating the Database Webhook.This section offers clear instructions on setting up the Database Webhook in Supabase, including screenshots for visual guidance. The steps are well-explained and cover all necessary configurations.
🧰 Tools
🪛 LanguageTool
[duplication] ~355-~355: Possible typo: you repeated a word
Context: ...ect dashboard, click 'Project settings' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~355-~355: Possible typo: you repeated a word
Context: ...} color="A8FF53" />, then the 'API' tab <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~355-~355: Possible typo: you repeated a word
Context: ...anon
public
API key from the table <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~359-~359: Possible typo: you repeated a word
Context: ...se-api-key.png) Then, go to 'Database' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~359-~359: Possible typo: you repeated a word
Context: ...} color="A8FF53" /> click on 'Webhooks' <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~359-~359: Possible typo: you repeated a word
Context: ... />, and then click 'Create a new hook' <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
392-414
: Comprehensive instructions for triggering and verifying the workflow.The final section provides a clear guide on how to test the entire workflow by inserting a new row into the Supabase table and verifying the results in both Trigger.dev and Supabase. This helps users confirm that their setup is working correctly.
🧰 Tools
🪛 LanguageTool
[style] ~396-~396: Consider a more expressive alternative.
Context: ... yourvideo_transcriptions
table. To do this, go back to your Supabase project ...(DO_ACHIEVE)
[duplication] ~396-~396: Possible typo: you repeated a word
Context: ...ject dashboard, click on 'Table Editor' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~396-~396: Possible typo: you repeated a word
Context: ...ick on thevideo_transcriptions
table <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~396-~396: Possible typo: you repeated a word
Context: ..., and then click 'Insert', 'Insert Row' <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~400-~400: Possible typo: you repeated a word
Context: ...rvideo_url
, with a public video url. <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~406-~406: Possible typo: you repeated a word
Context: .../cloud.trigger.dev) project 'Runs' list <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~406-~406: Possible typo: you repeated a word
Context: ...processingvideoProcessAndUpdate
task <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
Line range hint
1-414
: Excellent comprehensive guide with clear instructions and visual aids.This guide has been significantly improved and now provides a thorough, step-by-step tutorial for integrating Supabase with Trigger.dev to create a video transcription workflow. The instructions are clear, well-structured, and include helpful screenshots and code snippets. The addition of specific sections for each component (Supabase table, Trigger.dev task, Edge Function, and Database Webhook) makes the guide easy to follow and implement.
Minor suggestions for improvement:
- Consider adding a troubleshooting section to address common issues users might encounter.
- It might be helpful to include a brief explanation of the benefits of this integration at the beginning of the guide to further motivate users.
Overall, this guide is of high quality and will be very useful for developers looking to implement this integration.
🧰 Tools
🪛 LanguageTool
[typographical] ~20-~20: Consider adding two commas here.
Context: ...ebhook triggers an Edge Function when a row including a video URL is inserted into a table - The Edge Fun...(UNLIKE_COMMA)
[style] ~79-~79: Consider a more expressive alternative.
Context: ...re the video URL and transcription. To do this, click on 'Table Editor' <Icon ico...(DO_ACHIEVE)
[duplication] ~79-~79: Possible typo: you repeated a word
Context: ...n. To do this, click on 'Table Editor' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~85-~85: Possible typo: you repeated a word
Context: ...calledvideo_url
with the typetext
<Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~85-~85: Possible typo: you repeated a word
Context: ...anscription, also with the type
text` <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[style] ~265-~265: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...s to your Trigger.dev project You will need to add yourDEEPGRAM_SECRET_KEY
, `SUPABA...(REP_NEED_TO_VB)
[duplication] ~297-~297: Possible typo: you repeated a word
Context: ... to use, navigate to 'Project settings' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~297-~297: Possible typo: you repeated a word
Context: ...lor="A8FF53" />, click 'Edge Functions' <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~297-~297: Possible typo: you repeated a word
Context: ...nu, and then click the 'Add new secret' <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~299-~299: Possible typo: you repeated a word
Context: ...3" /> button. AddTRIGGER_SECRET_KEY
<Icon icon="circle-4" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~355-~355: Possible typo: you repeated a word
Context: ...ect dashboard, click 'Project settings' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~355-~355: Possible typo: you repeated a word
Context: ...} color="A8FF53" />, then the 'API' tab <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~355-~355: Possible typo: you repeated a word
Context: ...anon
public
API key from the table <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~359-~359: Possible typo: you repeated a word
Context: ...se-api-key.png) Then, go to 'Database' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~359-~359: Possible typo: you repeated a word
Context: ...} color="A8FF53" /> click on 'Webhooks' <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~359-~359: Possible typo: you repeated a word
Context: ... />, and then click 'Create a new hook' <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[style] ~396-~396: Consider a more expressive alternative.
Context: ... yourvideo_transcriptions
table. To do this, go back to your Supabase project ...(DO_ACHIEVE)
[duplication] ~396-~396: Possible typo: you repeated a word
Context: ...ject dashboard, click on 'Table Editor' <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~396-~396: Possible typo: you repeated a word
Context: ...ick on thevideo_transcriptions
table <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~396-~396: Possible typo: you repeated a word
Context: ..., and then click 'Insert', 'Insert Row' <Icon icon="circle-3" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~400-~400: Possible typo: you repeated a word
Context: ...rvideo_url
, with a public video url. <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~406-~406: Possible typo: you repeated a word
Context: .../cloud.trigger.dev) project 'Runs' list <Icon icon="circle-1" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~406-~406: Possible typo: you repeated a word
Context: ...processingvideoProcessAndUpdate
task <Icon icon="circle-2" iconType="solid" size={20} c...(ENGLISH_WORD_REPEAT_RULE)
docs/triggering.mdx (1)
745-748
: Regarding the repetitive sentence beginnings (static analysis hint)The static analysis tool flagged that three successive sentences begin with the same word. In this context, where we're listing and introducing different options in a documentation file, this repetition is intentional and beneficial. It provides a consistent structure that makes the documentation easier to scan and understand. Therefore, I recommend keeping the current structure as is.
🧰 Tools
🪛 LanguageTool
[style] ~746-~746: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...r more information. ###maxDuration
View our [maxDuration doc](/runs/max-duratio...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.v3.$projectParam.runs.$runParam.spans.$spanParam/route.tsx (1)
Line range hint
1-1023
: Overall, good addition with room for improvement.The addition of the "Max duration" property is a valuable improvement to the
RunBody
component, providing users with more comprehensive information about the run. The implementation is correct and consistent with the existing code style.However, there are opportunities to enhance the code further:
- Adding a tooltip to the "Max duration" property for clarity.
- Reordering the properties for better logical grouping.
- Refactoring the code for improved organization and reusability.
These suggestions would improve the overall quality of the code, enhance maintainability, and potentially provide a better user experience.
Despite these suggestions for improvement, the current changes are approved as they add value without introducing any issues. The suggested improvements can be considered for future refactoring efforts.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.runs.$runParam/route.tsx (2)
83-83
: New import added for impersonation functionality.The addition of
getImpersonationId
from~/services/impersonation.server
suggests that impersonation features are being introduced or expanded in this component.
119-126
: Impersonation feature added to loader function.The changes implement a feature to show deleted logs when a user is impersonating another account. This is a useful addition for debugging or support purposes.
However, it's crucial to ensure that this feature is properly secured and only available to authorized users.
Let's verify the security measures for the impersonation feature:
apps/webapp/app/v3/services/enqueueDelayedRun.server.ts (3)
1-1
: LGTM!The import of
parseNaturalLanguageDuration
from"@trigger.dev/core/v3/apps"
is appropriate and ensures that the duration is parsed correctly.
6-6
: LGTM!Adding the import for
ExpireEnqueuedRunService
enhances the code by encapsulating the expiration logic within a dedicated service.
55-55
: LGTM!The call to
ExpireEnqueuedRunService.enqueue(run.id, expireAt, tx);
correctly utilizes the new service to handle run expiration within the transaction.apps/webapp/app/db.server.ts (3)
1-8
: Consolidate Imports for Improved MaintainabilityThe updated import statements now import necessary types directly from
@trigger.dev/database
, which enhances code maintainability and clarity.
17-21
: Organized Type Exports Enhance ReadabilityExporting the types in a consolidated statement improves the organization and readability of the code.
29-42
: Verify Compatibility oftransac
Function ParametersEnsure that the
transac
function imported from@trigger.dev/database
accepts the error handling function and options as arguments in the order used. There might be a mismatch iftransac
expects parameters in a different sequence.Run the following script to confirm the function signature of
transac
:✅ Verification successful
Run additional verification scripts to check the
transac
function signature.
Run additional scripts to locate the
transac
function definition and its usage.
Run updated scripts with correct TypeScript file type identifiers.
Compatibility of
transac
Function Parameters Verified Successfully.The
transac
function is an alias for$transaction
and correctly accepts(prisma, fn, onError, options)
in that order.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the parameters of the `transac` function. # Test: Search for the `transac` function definition. Expect: It accepts (prisma, fn, onError, options). ast-grep --lang typescript --pattern 'export function $transaction($_, $_, $_, $_) { $$$ }'Length of output: 93
Script:
#!/bin/bash # Description: Verify the parameters of the `transac` function. # Test: Search for the `transac` function definition. Expect: It accepts (prisma, fn, onError, options). ast-grep --lang typescript --pattern 'export function $transac($_, $_, $_, $_) { $$$ }'Length of output: 89
Script:
#!/bin/bash # Description: Search for all occurrences of `transac` to identify its definition and usage. # Search for any export or function declarations of `transac` rg 'export .*transac' --type typescript # Search for all definitions of `transac` rg 'function transac' --type typescript # Search for `transac` as a method within classes or objects rg '\.transac\s*\(' --type typescript # Search for import statements related to `transac` rg 'import .*transac' --type typescriptLength of output: 305
Script:
#!/bin/bash # Description: Search for all occurrences of `transac` to identify its definition and usage. # Search for any export or function declarations of `transac` rg 'export .*transac' --type ts # Search for all definitions of `transac` rg 'function transac' --type ts # Search for `transac` as a method within classes or objects rg '\.transac\s*\(' --type ts # Search for import statements related to `transac` rg 'import .*transac' --type tsLength of output: 6607
apps/webapp/app/v3/services/finalizeTaskRun.server.ts (1)
10-10
: ImportingExpireEnqueuedRunService
The import statement correctly includes
ExpireEnqueuedRunService
, which is necessary for handling the expiration of enqueued runs.apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (4)
12-12
: LGTM!The
Badge
component is correctly imported from"~/components/primitives/Badge"
. This import is necessary for the usage of theBadge
component later in the code.
18-18
: LGTM!The
SimpleTooltip
component is properly imported from"~/components/primitives/Tooltip"
. This import enables the use of tooltips within the table for enhanced user experience.
45-49
: LGTM!The additional functions and components
descriptionForTaskRunStatus
,filterableTaskRunStatuses
, andTaskRunStatusCombo
are correctly imported from"./TaskRunStatus"
. These imports support the new functionality introduced for displaying task run statuses with descriptions.
315-319
: Ensure thedisableHoverableContent
prop is set appropriately.In the
SimpleTooltip
component wrapping theTaskRunStatusCombo
, thedisableHoverableContent
prop is used. Please verify that this aligns with the desired user interaction. SettingdisableHoverableContent
totrue
means the tooltip content will not be interactive. If the tooltip content requires user interaction, consider removing this prop or setting it tofalse
.apps/webapp/app/services/worker.server.ts (6)
1-1
: Simplify import path forDeliverEmailSchema
The import statement for
DeliverEmailSchema
has been simplified:-import { DeliverEmailSchema } from "./emails/makeDeliverEmail.server"; +import { DeliverEmailSchema } from "emails";Ensure that the module resolution correctly handles the new import path and that the "emails" package is properly configured in your project dependencies.
3-3
: Update import path forZodWorker
The
ZodWorker
is now imported from@internal/zod-worker
:-import { ZodWorker } from "../external/zod-worker"; +import { ZodWorker } from "@internal/zod-worker";Confirm that the module
@internal/zod-worker
is accessible and correctly set up in your project. This change may impact bundling or module resolution if not properly configured.
57-57
: Importlogger
for enhanced loggingThe
logger
has been imported:+import { logger } from "./logger.server";
Incorporating a logger improves observability and aids in debugging. This is a beneficial addition to the codebase.
283-283
: Addreplica
andlogger
toworkerQueue
configurationIn the
getWorkerQueue()
function,replica
andlogger
have been added to theZodWorker
configuration:return new ZodWorker({ name: "workerQueue", prisma, + replica: $replica, runnerOptions: { connectionString: env.DATABASE_URL, concurrency: env.WORKER_CONCURRENCY, pollInterval: env.WORKER_POLL_INTERVAL, noPreparedStatements: env.DATABASE_URL !== env.DIRECT_URL, schema: env.WORKER_SCHEMA, maxPoolSize: env.WORKER_CONCURRENCY + 1, }, + logger: logger, shutdownTimeoutInMs: env.GRACEFUL_SHUTDOWN_TIMEOUT, schema: workerCatalog, recurringTasks: { // ... }, tasks: { // ... }, });Considerations:
replica: $replica
: Ensure that$replica
is properly defined and that theZodWorker
can utilize it effectively for database operations. Verify that adding the replica does not introduce unexpected behavior or performance issues.
logger: logger
: Passing thelogger
improves logging capabilities. Confirm that the logger is correctly integrated and that log messages are appropriately handled without causing performance overhead.Also applies to: 292-292
738-739
: Includereplica
andlogger
inexecutionWorker
configurationIn the
getExecutionWorkerQueue()
function,replica
andlogger
have been added:return new ZodWorker({ name: "executionWorker", prisma, + replica: $replica, + logger: logger, runnerOptions: { connectionString: env.DATABASE_URL, concurrency: env.EXECUTION_WORKER_CONCURRENCY, pollInterval: env.EXECUTION_WORKER_POLL_INTERVAL, noPreparedStatements: env.DATABASE_URL !== env.DIRECT_URL, schema: env.WORKER_SCHEMA, maxPoolSize: env.EXECUTION_WORKER_CONCURRENCY + 1, }, shutdownTimeoutInMs: env.GRACEFUL_SHUTDOWN_TIMEOUT, schema: executionWorkerCatalog, rateLimiter: executionRateLimiter, tasks: { // ... }, });Considerations:
Ensure that the
executionWorker
correctly utilizes thereplica
for any read operations and that it's compatible with the worker's tasks.Verify that the logger integration works as expected, providing useful logs without adversely affecting performance.
794-795
: EnhancetaskOperationWorker
withreplica
andlogger
In the
getTaskOperationWorkerQueue()
function, the following additions have been made:return new ZodWorker({ name: "taskOperationWorker", prisma, + replica: $replica, + logger: logger, runnerOptions: { connectionString: env.DATABASE_URL, concurrency: env.TASK_OPERATION_WORKER_CONCURRENCY, pollInterval: env.TASK_OPERATION_WORKER_POLL_INTERVAL, noPreparedStatements: env.DATABASE_URL !== env.DIRECT_URL, schema: env.WORKER_SCHEMA, maxPoolSize: env.TASK_OPERATION_WORKER_CONCURRENCY + 1, }, shutdownTimeoutInMs: env.GRACEFUL_SHUTDOWN_TIMEOUT, schema: taskOperationWorkerCatalog, tasks: { // ... }, });Considerations:
As with the other workers, confirm that
replica
is correctly integrated and that any read operations are appropriately directed.Ensure that the logger provides valuable insights during the execution of task operations and does not introduce logging noise or performance penalties.
.changeset/config.json
Outdated
"kubernetes-provider", | ||
"@trigger.dev/otlp-importer" | ||
], | ||
"ignore": ["webapp", "proxy", "coordinator", "docker-provider", "kubernetes-provider"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Packages removed from the ignore
array are present in the project.
The packages "emails"
, "@trigger.dev/database"
, and "@trigger.dev/otlp-importer"
still exist in the project directories. Removing them from the ignore
array means they will now be included in the changeset process, which could impact versioning and changelog generation.
🔗 Analysis chain
Please clarify the removal of entries from the ignore
array.
The ignore
array has been modified to remove several entries: "emails"
, "@trigger.dev/database"
, and "@trigger.dev/otlp-importer"
. This change means that these packages will now be included in the changeset process, potentially affecting versioning and changelog generation.
Could you please explain the rationale behind this change? It's important to understand:
- Why these specific packages are no longer being ignored?
- What impact this might have on the project's versioning and release process?
- How does this change align with the PR objectives, which don't mention updates to the changeset configuration?
To help assess the impact, let's check if these packages are still present in the project:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the removed packages still exist in the project
echo "Checking for 'emails' directory:"
fd -t d '^emails$'
echo "Checking for '@trigger.dev/database' package:"
fd -t f 'package.json' | xargs grep -l '"name": "@trigger.dev/database"'
echo "Checking for '@trigger.dev/otlp-importer' package:"
fd -t f 'package.json' | xargs grep -l '"name": "@trigger.dev/otlp-importer"'
Length of output: 689
apps/webapp/tsconfig.json
Outdated
"@trigger.dev/database": ["../../internal-packages/database/src/index"], | ||
"@trigger.dev/database/*": ["../../internal-packages/database/src/*"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Outdated import paths detected
The following files still import modules using the old paths:
./tests/utils.ts
./apps/webapp/prisma/seedCloud.ts
./apps/webapp/app/db.server.ts
- (and many others as listed in the script output)
Please update these import statements to reflect the new internal-packages
paths to ensure consistency and prevent potential build or runtime issues.
🔗 Analysis chain
Path updates reflect project restructuring
The changes to these module paths indicate a shift from packages
to internal-packages
for several modules:
@trigger.dev/database
@trigger.dev/otlp-importer
emails
This restructuring suggests these packages are now considered internal to the project. Ensure that all imports throughout the codebase have been updated to reflect these new paths.
To verify that all imports have been updated correctly, run the following script:
Also applies to: 31-34
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining imports using the old package paths
echo "Checking for old import paths..."
rg --type typescript --type javascript "from ['\"]@trigger\.dev/(database|otlp-importer)" ./app
rg --type typescript --type javascript "from ['\"]emails" ./app
echo "If no results are shown above, all imports have been updated correctly."
Length of output: 759
Script:
#!/bin/bash
# Description: Check for any remaining imports using the old package paths
echo "Checking for old import paths in TypeScript and JavaScript files..."
rg --type ts --type js "from ['\"]@trigger\.dev/(database|otlp-importer)" ./app
rg --type ts --type js "from ['\"]emails" ./app
echo "If no results are shown above, all imports have been updated correctly."
Length of output: 525
Script:
#!/bin/bash
# Description: Check for any remaining imports using the old package paths across the entire repository
echo "Checking for old import paths in all TypeScript and JavaScript files..."
rg --type ts --type js "from ['\"]@trigger\.dev/(database|otlp-importer)" .
rg --type ts --type js "from ['\"]emails" .
echo "If no results are shown above, all imports have been updated correctly."
Length of output: 21782
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
Outdated
Show resolved
Hide resolved
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
Outdated
Show resolved
Hide resolved
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (1)
307-314
: Dialog added for plan downgrade in TierFree componentA new dialog has been implemented to capture user feedback when downgrading from a paid plan to the free plan. The dialog includes checkboxes for common reasons and a text area for additional feedback.
The implementation looks good, but there's a minor improvement that could be made:
Consider extracting the list of reasons into a constant array at the top of the file. This would make it easier to maintain and potentially reuse the list if needed elsewhere.
const DOWNGRADE_REASONS = [ "Subscription or usage costs too expensive", "Bugs or technical issues", "No longer need the service", "Found a better alternative", "Lacking features I need", ]; // Then in the component: {DOWNGRADE_REASONS.map((label, index) => ( // ... existing code ))}Also applies to: 375-494
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (16 hunks)
🧰 Additional context used
🔇 Additional comments (7)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (7)
2-2
: New imports added to enhance functionalityThe new imports introduce additional icons, components, and utilities that are used in the updated plan selection process. These additions seem appropriate for the new features being implemented.
Also applies to: 8-8, 11-11, 20-21, 25-27, 35-35, 38-38, 41-41, 43-43, 45-45
56-57
: Schema and action function updated to handle user feedbackThe schema and action function have been modified to include optional
reasons
andmessage
fields for user feedback when downgrading plans. This is a good addition to gather user insights.Also applies to: 66-75
295-295
: New prop added to TierFree componentThe
periodEnd
prop has been added to the TierFree component. This is used to display the date when the user will lose access to their current plan's features when downgrading.Also applies to: 301-301
515-519
: Dialog added for plan downgrade in TierHobby componentSimilar to the TierFree component, a dialog has been added to the TierHobby component for handling plan downgrades. The implementation is consistent with the TierFree component, which is good for maintaining a uniform user experience.
Also applies to: 527-591
634-634
: Minor updates to TierPro componentThe TierPro component has been updated to use the
form
attribute on the Button component. This change ensures that the correct form is submitted when the button is clicked.Also applies to: 642-642
799-820
: TierLimit component updated with tooltipThe TierLimit component has been enhanced with a tooltip that provides more information about compute pricing. This is a good addition for improving user understanding of the pricing structure.
Line range hint
1-958
: Overall improvements to plan selection and user feedbackThe changes in this file significantly enhance the plan selection process by adding confirmation dialogs and user feedback collection when downgrading plans. These additions will likely improve user experience and provide valuable insights to the business. The implementation is consistent across different plan tiers and follows React best practices.
A few minor suggestions were made to improve code maintainability and robustness, but overall, the changes are well-implemented and achieve the desired functionality.
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (2)
Line range hint
67-186
: Improve condition for thread creationThe new error handling and feedback submission logic aligns well with the PR objectives. However, there's a potential issue in the condition for creating a thread:
if (reasons.length > 0 || (message && message.toString().trim() !== "")) { // ... create thread logic }This condition might not work as expected if
reasons
is a string (which is allowed by the schema).Consider modifying the condition to handle both array and string cases:
- if (reasons.length > 0 || (message && message.toString().trim() !== "")) { + if ((Array.isArray(reasons) && reasons.length > 0) || + (typeof reasons === 'string' && reasons.trim() !== '') || + (message && message.toString().trim() !== "")) { // ... create thread logic }
Line range hint
565-639
: LGTM! TierHobby component updated with downgrade confirmationThe addition of the downgrade confirmation dialog for the Hobby plan aligns well with the PR objectives. The implementation is clear and provides a good user experience.
For consistency with the TierFree component, consider adding a brief feedback collection mechanism to this dialog as well. This could provide valuable insights into why users are downgrading from Pro to Hobby.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (18 hunks)
🧰 Additional context used
🔇 Additional comments (4)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (4)
2-2
: LGTM! New imports added for enhanced functionality.The new imports suggest the addition of new UI components and functionality, which aligns with the PR objectives of implementing new confirmation modals and tooltips. The change from
requireUserId
torequireUser
indicates a shift in how user data is retrieved, which may provide more comprehensive user information.Also applies to: 8-8, 11-12, 21-22, 26-28, 36-36, 39-39, 42-42, 44-44, 46-46
255-255
: LGTM! PricingPlans component updated with periodEnd propThe addition of the
periodEnd
prop to the PricingPlans component and its propagation to the TierFree component aligns with the PR objectives. This change likely enables the display of information about when the current plan ends, enhancing user understanding of their subscription status.Also applies to: 264-264, 274-274
682-682
: LGTM! Minor improvements to TierPro componentThe changes to the TierPro component, including updating the form ID to "subscribe-pro" and adding the form attribute to the Button component, are appropriate. These improvements ensure a proper connection between the form and the submit button.
Also applies to: 690-690
847-868
: LGTM! Enhanced TierLimit function with informative tooltipThe modifications to the TierLimit function, including the addition of the SimpleTooltip component, align well with the PR objectives. The new tooltip provides valuable information about compute pricing when users hover over the usage limit link, enhancing the overall user experience and clarity of the pricing information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (1)
426-426
: Remove duplicate key prop from Dialog component.The Dialog component has a duplicate
key
prop, which is unnecessary and may cause a React warning.Remove the
key
prop from the Dialog component:- <Dialog open={isDialogOpen} onOpenChange={setIsDialogOpen} key="cancel"> + <Dialog open={isDialogOpen} onOpenChange={setIsDialogOpen}>This change removes the redundant
key
prop, as it's not needed in this context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (19 hunks)
🧰 Additional context used
🔇 Additional comments (5)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (5)
2-2
: LGTM: New imports are appropriate for the added functionality.The new imports, including icons, PlainClient, and local components/utilities, align well with the changes made to the file. They provide the necessary tools for implementing the new features and UI elements.
Also applies to: 8-8, 11-12, 21-22, 26-28, 36-36, 39-39, 42-42, 44-44, 46-46
67-70
: LGTM: Improved user identification and feedback collection.The changes to the action function enhance user identification by using
requireUser
and add robust functionality for collecting and submitting feedback when users downgrade their plans. The integration with PlainClient for customer data management is well-implemented.Also applies to: 90-186
296-315
: LGTM: Enhanced TierFree component with downgrade dialog and feedback form.The TierFree component has been significantly improved with the addition of a downgrade confirmation dialog and a feedback form. These changes align well with the PR objectives, providing a better user experience when changing subscription plans.
Also applies to: 376-533
573-577
: LGTM: TierHobby component updated with downgrade dialog.The TierHobby component has been enhanced with a downgrade confirmation dialog, consistent with the changes made to the TierFree component. These modifications improve the user experience when changing plans and align well with the PR objectives.
Also applies to: 585-647
855-874
: LGTM: New TierLimit component with informative tooltip.The new TierLimit component is a great addition, providing users with more information about usage limits through an informative tooltip. This enhancement aligns perfectly with the PR objective of adding a tooltip for compute allowance, improving the overall user experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (19 hunks)
🧰 Additional context used
🔇 Additional comments (11)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (11)
Line range hint
1-58
: LGTM: Imports and schema updates align with new featuresThe new imports and schema updates are consistent with the PR objectives, including the addition of new icons, components, and form fields for user feedback. These changes support the implementation of the new confirmation modal and tooltip features.
Line range hint
60-206
: LGTM: Action function improvementsThe action function has been significantly enhanced to handle user feedback, improve error handling, and integrate with the Plain API for customer data management. These changes align well with the PR objectives and provide a more robust implementation.
Line range hint
257-287
: LGTM: PricingPlans component updatesThe PricingPlans component has been successfully updated to include new props and render all tier components (Free, Hobby, Pro, and Enterprise). These changes provide a comprehensive view of all available plans and align with the PR objectives.
Line range hint
289-552
: LGTM: TierFree component enhancementsThe TierFree component has been significantly enhanced with new features, including a confirmation dialog for downgrading to the free plan and GitHub verification logic. These changes align well with the PR objectives and improve the user experience when selecting or changing plans.
Line range hint
554-647
: LGTM: TierHobby component updatesThe TierHobby component has been successfully updated with new features, including a confirmation dialog for downgrading to the Hobby plan. These changes align with the PR objectives and provide a consistent user experience across different plan tiers.
Line range hint
649-738
: LGTM: TierPro component consistencyThe TierPro component has been updated with minor changes that maintain consistency with other tier components. This ensures a uniform user experience across all plan options.
Line range hint
740-805
: LGTM: TierEnterprise componentThe TierEnterprise component remains consistent with the existing design and provides clear information about the Enterprise plan features. No significant changes were required, and it aligns well with the overall pricing plan structure.
Line range hint
807-1007
: LGTM: Utility components and functions enhancementsThe utility components and functions have been successfully updated and enhanced. The new TierLimit component with a tooltip is a notable addition that aligns with the PR objectives, providing users with more information about compute pricing. These changes contribute to an improved user experience on the pricing plans page.
Line range hint
1-1007
: Overall LGTM: Comprehensive improvements to pricing plans functionalityThis PR introduces significant enhancements to the pricing plans functionality, including:
- Implementation of confirmation modals for plan downgrades
- Integration with Plain API for customer feedback
- Improved error handling and logging
- Addition of tooltips for better user information
- Consistent updates across all plan tier components
These changes align well with the PR objectives and greatly improve the user experience when selecting or changing subscription plans. The code structure is maintainable and follows good practices. Minor suggestions for improvements have been provided in previous comments.
68-76
:⚠️ Potential issueRemove console.log statements in production code
The use of
console.log
statements to log form data may inadvertently expose sensitive user information. It's recommended to remove these logging statements or ensure they are only used in development environments.Consider removing or conditionally executing these log statements:
- console.log("All form data:", Object.fromEntries(formData));
Likely invalid or redundant comment.
133-133
:⚠️ Potential issueImprove condition for creating feedback thread
The current condition
reasons.length > 0 || (message && message.toString().trim() !== "")
may not work as expected ifreasons
is a string (which is allowed by the schema).Consider modifying the condition to handle both array and string cases:
- if (reasons.length > 0 || (message && message.toString().trim() !== "")) { + if ((Array.isArray(reasons) && reasons.length > 0) || + (typeof reasons === 'string' && reasons.trim() !== '') || + (message && message.toString().trim() !== "")) {Likely invalid or redundant comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx (1)
40-42
: LGTM! Consider adding a comment for clarity.The implementation of
periodEnd
is correct. It sets the date to one month from now, which is likely used for subscription or plan duration purposes.Consider adding a brief comment explaining the purpose of
periodEnd
for better code readability:const periodEnd = new Date(); periodEnd.setMonth(periodEnd.getMonth() + 1); +// Set periodEnd to one month from now for subscription duration
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (2)
Line range hint
60-206
: Enhance error handling for PlainClient operationsThe current error handling for PlainClient operations is generic. Consider adding more specific error handling to provide better diagnostics and user feedback.
For example:
try { // PlainClient operations } catch (e) { if (e instanceof PlainAPIError) { logger.error("PlainClient API error", { error: e.message, code: e.code }); throw redirectWithErrorMessage(form.callerPath, request, `API Error: ${e.message}`); } else { logger.error("Unknown error during Plain operations", { error: e }); throw redirectWithErrorMessage(form.callerPath, request, "An unexpected error occurred"); } }This approach provides more detailed error logging and user-friendly error messages.
Line range hint
291-533
: Great addition of downgrade dialog and feedback collectionThe new dialog for downgrading to the free plan and the implementation of feedback collection align perfectly with the PR objectives. This enhancement provides valuable insights into why users are downgrading and improves the overall user experience.
Consider using useCallback for dialog state management
To optimize performance, consider using the
useCallback
hook for thesetIsDialogOpen
function:const handleDialogOpenChange = useCallback((open: boolean) => { setIsDialogOpen(open); }, []); useEffect(() => { handleDialogOpenChange(false); }, [subscription]); // Use handleDialogOpenChange instead of setIsDialogOpen in the Dialog componentThis change can help prevent unnecessary re-renders, especially in larger components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- apps/webapp/app/routes/app.orgs.$organizationSlug.select-plan/route.tsx (2 hunks)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (19 hunks)
🧰 Additional context used
🔇 Additional comments (7)
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx (2)
43-43
: LGTM! Consistent with previous changes.The addition of
periodEnd
to thetypedjson
return object is correct and consistent with the previous changes. This makes theperiodEnd
value available to the client-side code.
47-48
: LGTM! Verify PricingPlans component prop usage.The changes are consistent with the modifications in the loader function. The
periodEnd
value is correctly destructured and passed as a prop to thePricingPlans
component.Ensure that the
PricingPlans
component is updated to handle the newperiodEnd
prop correctly. Run the following script to verify:Also applies to: 59-59
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (5)
Line range hint
1-46
: LGTM: Import statements updated appropriatelyThe new imports align well with the added features and UI enhancements. The inclusion of new icons, PlainClient, and additional components supports the implementation of dialogs, feedback forms, and improved user interface elements.
Line range hint
249-289
: LGTM: PricingPlans component updated appropriatelyThe PricingPlans component has been updated to include the new
periodEnd
prop, which is then passed down to the TierFree component. This change aligns well with the new functionality for handling plan downgrades and providing more accurate information to users.
Line range hint
558-647
: LGTM: TierHobby component enhanced with downgrade dialogThe addition of the downgrade dialog in the TierHobby component is a great improvement. It provides a consistent user experience across different plan tiers and aligns well with the changes made in the TierFree component. The comprehensive handling of different subscription statuses ensures that users are presented with appropriate options based on their current plan.
Line range hint
674-734
: LGTM: TierPro component updated consistentlyThe TierPro component has been updated to maintain consistency with the other tier components. The changes to button text and disabled state handling improve the user experience by providing clear and accurate information about the current plan and available actions.
855-876
: Great addition of tooltip to TierLimit componentThe new tooltip in the TierLimit component is a valuable addition. It enhances the user experience by providing more context about the compute pricing information when users hover over the usage limit text. This improvement aligns well with the overall goal of making the pricing information more transparent and accessible.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements
Documentation