-
-
Notifications
You must be signed in to change notification settings - Fork 729
Added initial repo cursor rules and other docs for AI #1783
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
|
WalkthroughSeveral new documentation and configuration files have been added to the monorepo. Guidelines now cover how to execute commands using Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
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
🧹 Nitpick comments (11)
.cursorignore (1)
1-7
: Well-structured ignore configurationThe
.cursorignore
file correctly lists directories and files to be excluded from cursor tracking, which helps improve performance and focus. The configuration includes specific app directories and common configuration files.However, consider adding a brief comment at the top of the file explaining its purpose for future contributors.
+# Directories and files to be ignored by cursor apps/docker-provider/ apps/kubernetes-provider/ apps/proxy/ apps/coordinator/ packages/rsc/ .changeset .zed
.cursor/rules/writing-tests.mdc (1)
1-6
: Clear and concise test guidelines referenceThis rule effectively directs users to the detailed test documentation while providing the basic context. The use of the
mdc:
link format ensures proper navigation within the monorepo's documentation system.For enhanced usability, consider adding a brief overview of the key testing principles before referring to the detailed guide.
--- description: How to write tests in the monorepo globs: alwaysApply: true --- +We use Vitest as our testing framework and prefer real integrations over mocks. + Follow our [tests.md](mdc:ai/references/tests.md) guide for how to write tests in the monorepo..cursor/rules/repo.mdc (1)
1-6
: Effective monorepo structure documentation linkThe rule correctly points users to the detailed documentation about the repository structure. The
alwaysApply: true
setting ensures all users will benefit from this knowledge.To improve immediate usefulness, consider adding a brief overview of the repo's main components directly in this file.
--- description: understanding the structure of the monorepo globs: alwaysApply: true --- +Our monorepo contains: +- Apps: webapp (Remix), supervisor +- Public packages: trigger-sdk, cli-v3, core, etc. +- Internal packages: database, run-engine, redis-worker, etc. + We've documented the structure of our monorepo here: [repo.md](mdc:ai/references/repo.md).cursor/rules/webapp.mdc (1)
24-26
: Informative historical context for the Run EngineThe explanation of the legacy run engine versus Run Engine 2.0 provides valuable context for developers. However, there's a minor grammatical error in the first sentence.
## Legacy run engine vs Run Engine 2.0 -We originally the Trigger.dev "Run Engine" not as a single system, but just spread out all over the codebase, with no real separate or encapsulation. And we didn't even call it a "Run Engine". With Run Engine 2.0, we've completely rewritten big parts of the way the system works, and moved it over to an internal package called `@internal/run-engine`. So we've retroactively named the previous run engine "Legacy run engine". We're focused almost exclusively now on moving to Run Engine 2.0 and will be deprecating and removing the legacy run engine code eventually. +We originally implemented the Trigger.dev "Run Engine" not as a single system, but just spread out all over the codebase, with no real separation or encapsulation. And we didn't even call it a "Run Engine". With Run Engine 2.0, we've completely rewritten big parts of the way the system works, and moved it over to an internal package called `@internal/run-engine`. So we've retroactively named the previous run engine "Legacy run engine". We're focused almost exclusively now on moving to Run Engine 2.0 and will be deprecating and removing the legacy run engine code eventually.ai/references/tests.md (3)
26-26
: Improve Wording for Clarity on Mock Statement.
The phrase “almost NEVER mock anything” may sound overly emphatic and wordy. Consider replacing it with a more concise alternative such as “rarely mock anything” or “avoid using mocks.”🧰 Tools
🪛 LanguageTool
[style] ~26-~26: ‘almost NEVER’ might be wordy. Consider a shorter alternative.
Context: ...ng Tests We use vitest for testing. We almost NEVER mock anything. Start with a top-level "...(EN_WORDINESS_PREMIUM_ALMOST_NEVER)
73-78
: Reduce Repetitive List Wording.
The bullet list items all start with “Do not …”, which makes the section feel repetitive. Consider combining these points into a single, consolidated statement (for example, “Avoid using mocks, spies, stubs, fakes, or sinon in tests”) to improve readability.🧰 Tools
🪛 LanguageTool
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nything. - Do not use mocks in tests. - Do not use spies in tests. - Do not use st...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~76-~76: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n tests. - Do not use spies in tests. - Do not use stubs in tests. - Do not use fa...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~77-~77: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n tests. - Do not use stubs in tests. - Do not use fakes in tests. - Do not use si...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~78-~78: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n tests. - Do not use fakes in tests. - Do not use sinon in tests. - Structure eac...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
80-80
: Rephrase Informal Language.
The phrase “Feel free to write long test names” can be reworded to a more formal suggestion, such as “Consider using descriptive test names to improve clarity.”🧰 Tools
🪛 LanguageTool
[style] ~80-~80: The phrase ‘feel free to’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others and make it sound more professional.
Context: ...a setup, action, and assertion style. - Feel free to write long test names. - If there is an...(FEEL_FREE_TO_STYLE_ME)
ai/references/repo.md (4)
16-16
: Add Comma for Clarity.
In the sentence describing the react-hooks package, consider adding a comma after “react hooks” to improve clarity. For example:- defines some useful react hooks like our realtime hooks. + defines some useful react hooks, like our realtime hooks.🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: Possible missing comma found.
Context: ...s/react-hooks defines some useful react hooks like our realtime hooks. See our [Realt...(AI_HYDRA_LEO_MISSING_COMMA)
20-20
: Improve Punctuation for Readability.
A comma after “usually” would help the sentence flow better. For example:- used internally only, not published, and usually they have a tsc build step + used internally only, not published, and usually, they have a tsc build step🧰 Tools
🪛 LanguageTool
[typographical] ~20-~20: Consider adding a comma after ‘usually’ for more clarity.
Context: ...sed internally only, not published, and usually they have a tsc build step and are used...(RB_LY_COMMA)
22-22
: Correct Possessive Form.
Replace “it's lifecycle” with “its lifecycle” to correctly indicate the possessive form. For example:- handles moving a run all the way through it's lifecycle + handles moving a run all the way through its lifecycle🧰 Tools
🪛 LanguageTool
[uncategorized] ~22-~22: Did you mean “its” (the possessive pronoun)?
Context: ...andles moving a run all the way through it's lifecycle - /internal-packages/re...(ITS_PREMIUM)
23-23
: Fix Typo in "system".
There is a typo in “sytem” which should be corrected to “system” for accuracy. For example:- implements a custom background job/worker sytem powered by redis + implements a custom background job/worker system powered by redis
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.cursor/rules/executing-commands.mdc
(1 hunks).cursor/rules/repo.mdc
(1 hunks).cursor/rules/webapp.mdc
(1 hunks).cursor/rules/writing-tests.mdc
(1 hunks).cursorignore
(1 hunks)ai/references/repo.md
(1 hunks)ai/references/tests.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
ai/references/repo.md
[uncategorized] ~16-~16: Possible missing comma found.
Context: ...s/react-hooks defines some useful react hooks like our realtime hooks. See our [Realt...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~20-~20: Consider adding a comma after ‘usually’ for more clarity.
Context: ...sed internally only, not published, and usually they have a tsc build step and are used...
(RB_LY_COMMA)
[uncategorized] ~22-~22: Did you mean “its” (the possessive pronoun)?
Context: ...andles moving a run all the way through it's lifecycle - /internal-packages/re...
(ITS_PREMIUM)
ai/references/tests.md
[style] ~26-~26: ‘almost NEVER’ might be wordy. Consider a shorter alternative.
Context: ...ng Tests We use vitest for testing. We almost NEVER mock anything. Start with a top-level "...
(EN_WORDINESS_PREMIUM_ALMOST_NEVER)
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nything. - Do not use mocks in tests. - Do not use spies in tests. - Do not use st...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~76-~76: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n tests. - Do not use spies in tests. - Do not use stubs in tests. - Do not use fa...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~77-~77: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n tests. - Do not use stubs in tests. - Do not use fakes in tests. - Do not use si...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~78-~78: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n tests. - Do not use fakes in tests. - Do not use sinon in tests. - Structure eac...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~80-~80: The phrase ‘feel free to’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others and make it sound more professional.
Context: ...a setup, action, and assertion style. - Feel free to write long test names. - If there is an...
(FEEL_FREE_TO_STYLE_ME)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
.cursor/rules/webapp.mdc (4)
1-5
: Good configuration setup for webapp rulesThe metadata configuration correctly specifies which files this rule applies to with appropriate glob patterns. Setting
alwaysApply: false
is appropriate since these rules are specific to webapp files only.
6-13
: Comprehensive webapp overview with useful subsystem descriptionsThis section effectively describes the main webapp architecture and its dependencies. The links to specific schema files and package configurations are particularly valuable.
14-22
: Excellent testing guidance with practical examplesThis section provides clear guidance on environment variable usage and testing best practices. The service/configuration separation principle is well explained with concrete examples.
28-37
: Helpful code location references for common functionalityThis section provides valuable pointers to important parts of the codebase. The detailed listings of API endpoints, configuration files, and service locations will help developers navigate the complex webapp structure.
ai/references/repo.md (1)
1-38
: Overall Documentation is Clear and Informative.
The repo overview is well-structured and provides comprehensive details on the repository’s structure and components. Nice work on outlining both public and internal packages alongside other key resources.🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: Possible missing comma found.
Context: ...s/react-hooks defines some useful react hooks like our realtime hooks. See our [Realt...(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~20-~20: Consider adding a comma after ‘usually’ for more clarity.
Context: ...sed internally only, not published, and usually they have a tsc build step and are used...(RB_LY_COMMA)
[uncategorized] ~22-~22: Did you mean “its” (the possessive pronoun)?
Context: ...andles moving a run all the way through it's lifecycle - /internal-packages/re...(ITS_PREMIUM)
--- | ||
description: how to run commands in the monorepo | ||
globs: | ||
alwaysApply: true | ||
--- | ||
Almost all commands in the monorepo should be executed when `pnpm run ...` from the root of the monorepo. For example, running tests for the `@internal/run-engine` internal package: | ||
|
||
``` | ||
pnpm run dev --filter webapp | ||
``` | ||
|
||
But often, when running tests, it's better to `cd` into the directory and then run tests: | ||
|
||
``` | ||
cd apps/webapp | ||
pnpm run test | ||
``` | ||
|
||
This way you can run for a single file easily: | ||
|
||
``` | ||
cd internal-packages/run-engine | ||
pnpm run test ./src/engine/tests/ttl.test.ts | ||
``` |
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.
🛠️ Refactor suggestion
Good command execution guidance with minor inconsistency
This documentation effectively explains how to run commands in the monorepo with helpful examples. However, there's an inconsistency in the first example - it mentions running tests for the @internal/run-engine
package but the example shows pnpm run dev --filter webapp
.
---
description: how to run commands in the monorepo
globs:
alwaysApply: true
---
-Almost all commands in the monorepo should be executed when `pnpm run ...` from the root of the monorepo. For example, running tests for the `@internal/run-engine` internal package:
+Almost all commands in the monorepo should be executed using `pnpm run ...` from the root of the monorepo. For example, running the development server for the webapp:
pnpm run dev --filter webapp
+For running tests for a specific package from the root:
+
+```
+pnpm run test --filter @internal/run-engine
+```
+
But often, when running tests, it's better to `cd` into the directory and then run tests:
cd apps/webapp
pnpm run test
This way you can run for a single file easily:
cd internal-packages/run-engine
pnpm run test ./src/engine/tests/ttl.test.ts
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
--- | |
description: how to run commands in the monorepo | |
globs: | |
alwaysApply: true | |
--- | |
Almost all commands in the monorepo should be executed when `pnpm run ...` from the root of the monorepo. For example, running tests for the `@internal/run-engine` internal package: | |
``` | |
pnpm run dev --filter webapp | |
``` | |
But often, when running tests, it's better to `cd` into the directory and then run tests: | |
``` | |
cd apps/webapp | |
pnpm run test | |
``` | |
This way you can run for a single file easily: | |
``` | |
cd internal-packages/run-engine | |
pnpm run test ./src/engine/tests/ttl.test.ts | |
``` | |
--- | |
description: how to run commands in the monorepo | |
globs: | |
alwaysApply: true | |
--- | |
Almost all commands in the monorepo should be executed using `pnpm run ...` from the root of the monorepo. For example, running the development server for the webapp: | |
Summary by CodeRabbit
Documentation
Chores