feat(cli): implement watch mode for generate #554
Conversation
merge dev to main (v3.0.0-beta.28)
merge dev to main (v3.0.0-beta.29)
merge dev to main (v3.0.0-beta.30)
merge dev to main (v3.0.0-beta.31)
merge dev to main (v3.0.0-beta.32)
merge dev to main (v3.0.0-beta.33)
merge dev to main (v3.0.0-beta.34)
merge dev to main (v3.0.0)
chore: update CI sample update script (zenstackhq#531)
merge dev to main
📝 WalkthroughWalkthroughAdds CLI generate watch mode (new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/cli/src/index.ts (1)
224-228: Consider a more robust approach for detecting watch mode.Checking
program.argsfor raw argument strings is fragile - it won't work if the user combines short flags (e.g.,-wsfor--watch --silent) or uses--watch=true. A more robust approach would be to have the generate action return or signal its running state.🔎 Alternative approach
One option is to have
generateActionreturn a boolean or the action itself track whether it's in persistent mode:-const generateAction = async (options: Parameters<typeof actions.generate>[0]): Promise<void> => { - await telemetry.trackCommand('generate', () => actions.generate(options)); -}; +let isWatchMode = false; +const generateAction = async (options: Parameters<typeof actions.generate>[0]): Promise<void> => { + isWatchMode = options.watch; + await telemetry.trackCommand('generate', () => actions.generate(options)); +};Then check
isWatchModeinstead of parsing args:- if (program.args.includes('generate') && (program.args.includes('-w') || program.args.includes('--watch'))) { + if (isWatchMode) { // Prevent the process from terminating in watch mode return; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/cli/package.jsonpackages/cli/src/actions/generate.tspackages/cli/src/index.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Database migrations should use Prisma CLI under the hood via ZenStack commands
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/cli/src/index.tspackages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Use `pnpm` with workspaces for package management, pinned to version `pnpm10.12.1`
Applied to files:
packages/cli/package.json
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Applied to files:
packages/cli/src/actions/generate.ts
🧬 Code graph analysis (1)
packages/cli/src/actions/generate.ts (3)
packages/language/src/module.ts (1)
ZModelLanguageMetaData(27-27)packages/language/src/generated/ast.ts (5)
Model(559-563)Model(565-565)isDataModel(391-393)DataModel(377-387)DataModel(389-389)packages/language/src/ast.ts (1)
DataModel(49-54)
🔇 Additional comments (4)
packages/cli/src/actions/generate.ts (2)
16-23: LGTM!The
watchoption is correctly added to the Options type.
114-144: LGTM!The
pureGeneratehelper cleanly separates the generation logic and appropriately suppresses the usage guidance during watch-triggered regenerations.packages/cli/src/index.ts (1)
71-71: LGTM!The
-w, --watchflag follows the established pattern for CLI options.packages/cli/package.json (1)
38-38: LGTM!Chokidar is a well-established file watching library and an appropriate choice for this feature. Version 5.0.0 is current.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/cli/src/index.ts (1)
224-227: Fragile arg-based watch detection; consider checking parsed option or exitCode.The raw string check on
program.argsis brittle and doesn't account for error scenarios. If generation fails (exitCode !== 0), returning early leaves the process hanging without proper exit signaling.Consider a cleaner approach where the generate action signals watch mode is active (e.g., via a shared flag or promise), or at minimum guard against errors:
🔎 Suggested improvement
- if (program.args.includes('generate') && (program.args.includes('-w') || program.args.includes('--watch'))) { + if (exitCode === 0 && program.args.includes('generate') && (program.args.includes('-w') || program.args.includes('--watch'))) { // A "hack" way to prevent the process from terminating because we don't want to stop it. return; }packages/cli/src/actions/generate.ts (2)
38-50: Consider extracting helper for cleaner path resolution.The
getModelAllPathsclosure works correctly, but the chained optionals with non-null assertions on line 49 rely on implicit guarantees from the filter. This is safe but could be clearer.🔎 Optional: Extract to a simpler helper
- const getModelAllPaths = (model: Model) => new Set( - ( - model.declarations.filter( - (v) => - isDataModel(v) && - v.$cstNode?.parent?.element.$type === 'Model' && - !!v.$cstNode.parent.element.$document?.uri?.fsPath, - ) as DataModel[] - ).map((v) => v.$cstNode!.parent!.element.$document!.uri!.fsPath), - ); + const getModelAllPaths = (model: Model): Set<string> => { + const paths: string[] = []; + for (const decl of model.declarations) { + if (!isDataModel(decl)) continue; + const fsPath = decl.$cstNode?.parent?.element.$document?.uri?.fsPath; + if (fsPath && decl.$cstNode?.parent?.element.$type === 'Model') { + paths.push(fsPath); + } + } + return new Set(paths); + };
98-112: Consider graceful shutdown handling.The watcher runs indefinitely without cleanup on process termination. While functional, adding SIGINT/SIGTERM handlers would allow graceful shutdown and proper resource cleanup.
🔎 Optional enhancement
// After watcher setup const cleanup = () => { watcher.close(); process.exit(0); }; process.on('SIGINT', cleanup); process.on('SIGTERM', cleanup);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/cli/package.jsonpackages/cli/src/actions/generate.tspackages/cli/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/package.json
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/cli/src/index.tspackages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
packages/cli/src/actions/generate.ts
🧬 Code graph analysis (1)
packages/cli/src/actions/generate.ts (3)
packages/language/src/module.ts (1)
ZModelLanguageMetaData(27-27)packages/language/src/generated/ast.ts (5)
Model(559-563)Model(565-565)isDataModel(391-393)DataModel(377-387)DataModel(389-389)packages/language/src/ast.ts (1)
DataModel(49-54)
🔇 Additional comments (4)
packages/cli/src/index.ts (1)
71-71: LGTM!The watch option is correctly defined with appropriate short and long flags and a sensible default.
packages/cli/src/actions/generate.ts (3)
52-67: LGTM! Previous review feedback addressed.The dynamic import of chokidar is appropriate,
schemaExtensionsis correctly kept as an array, and theignoredcallback properly uses.some()with.endsWith()for extension matching.
69-96: Debouncing and dynamic path tracking look good.The 500ms debounce prevents rapid regeneration on bulk saves. The fix at line 89 correctly adds new paths to the
watchedPathsSet before adding to the watcher, addressing the previous review feedback.One minor note: if
pureGeneratefails, the error is logged but watch continues—this is reasonable resilience.
115-145: Clean extraction of generation logic.The
pureGeneratehelper nicely encapsulates the generation workflow, enabling reuse for both initial and watch-triggered generations. ThefromWatchparameter correctly suppresses the usage guidance during watch regenerations, which is good UX.
ymc9
left a comment
ymc9
left a comment
There was a problem hiding this comment.
Hi @DoctorFTB , thanks a lot for working on this feature! I've personally wanted it for quite a while 😄.
Please take a look at my review comments when you have time.
|
The pnpm lockfile needs to be updated to fix the build. |
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @packages/cli/src/actions/generate.ts:
- Around line 93-101: The removed-paths branch is wrong: when removeModelPaths
is non-empty, change the log text from "Added file(s) to watch:" to "Removed
file(s) from watch:" (use logsEnabled and logPaths as before), replace
watchedPaths.add(at) with watchedPaths.delete(at) when iterating
removeModelPaths, and replace watcher.add(removeModelPaths) with
watcher.unwatch(removeModelPaths) so removed files are actually unwatched; keep
using the same variables removeModelPaths, logsEnabled, watchedPaths, watcher
and preserve existing logging formatting.
🧹 Nitpick comments (2)
packages/cli/src/actions/generate.ts (2)
51-51: Consider using a static import for chokidar.The dynamic import provides minimal benefit in a CLI context and adds unnecessary complexity. A previous review comment asked about the specific reason for the dynamic import. Unless there's a compelling reason (e.g., significant startup time impact), a static import would be simpler and more conventional.
🔎 Proposed refactor to use static import
Add to the top of the file with other imports:
+import { watch } from 'chokidar'; import { invariant } from '@zenstackhq/common-helpers'; import { ZModelLanguageMetaData } from '@zenstackhq/language';Then remove the dynamic import:
- const { watch } = await import('chokidar'); - const watchedPaths = getRootModelWatchPaths(model);
68-106: Consider using a dedicated debounce library for more robust handling.The manual
setTimeout-based debouncing is functional but could be improved. A previous review comment suggested using a package likep-debouncefor more reliable throttling. This would provide better handling of promise-based async operations, prevent overlapping regenerations if one takes longer than the debounce period, and offer additional configuration options (leading/trailing edge).For reference, using
p-debounce:import pDebounce from 'p-debounce'; const reGenerateSchema = pDebounce(async () => { if (logsEnabled) { console.log('Got changes, run generation!'); } try { const newModel = await pureGenerate(options, true); // ... rest of logic } catch (e) { console.error(e); } }, 500);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/cli/src/actions/generate.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/cli/src/actions/generate.ts
🧬 Code graph analysis (1)
packages/cli/src/actions/generate.ts (3)
packages/language/src/module.ts (1)
ZModelLanguageMetaData(27-27)packages/sdk/src/prisma/prisma-builder.ts (1)
Model(115-164)packages/language/src/generated/ast.ts (4)
Model(559-563)Model(565-565)AbstractDeclaration(87-87)AbstractDeclaration(89-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (7)
packages/cli/src/actions/generate.ts (7)
2-3: LGTM!The new imports are appropriate for the watch mode implementation.
ZModelLanguageMetaDataprovides schema file extensions for filtering, and the AST types enable CST traversal to extract source file paths.
20-20: LGTM!The
watchoption is a clean addition to the Options type for enabling watch mode.
29-38: LGTM!The initial generation is correctly performed before setting up the watch mode. The use of
schemaExtensionsas an array (rather than a joined string) properly addresses the previous review feedback.
53-66: LGTM!The watcher initialization is well-structured. The
ignoredcallback correctly uses.some()with.endsWith()for precise extension matching, which properly addresses previous review feedback about fragile substring matching.
108-121: LGTM!The watcher event handlers are correctly implemented. The
unlinkhandler properly removes the deleted file from tracking before triggering regeneration, and thechangehandler efficiently triggers the debounced regeneration.
125-155: LGTM!The
pureGeneratefunction is a clean refactoring that extracts the core generation logic. ThefromWatchparameter appropriately controls whether to display client usage guidance (avoiding repetition on every regeneration), and returning theModelenables the watch mode to track source file paths via CST traversal.
41-49: All declaration types are correctly captured for watching.The function filters
model.declarationswhich contains allAbstractDeclarationtypes: Attribute, DataModel, DataSource, Enum, FunctionDecl, GeneratorDecl, Plugin, Procedure, and TypeDef. The CST filterv.$cstNode?.parent?.element.$type === 'Model'applies to all these types since they are direct children of Model, ensuring that enums, types, functions, and other declarations are watched regardless of type.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/cli/src/actions/generate.ts (1)
93-101: Minor typo in log message.Line 96 says "Removed file(s) to watch:" but should be "Removed file(s) from watch:" for grammatical correctness and consistency with line 110.
🔎 Proposed fix
if (removeModelPaths.length) { if (logsEnabled) { const logPaths = [...removeModelPaths].map((at) => `- ${at}`).join('\n'); - console.log(`Removed file(s) to watch:\n${logPaths}`); + console.log(`Removed file(s) from watch:\n${logPaths}`); } removeModelPaths.forEach((at) => watchedPaths.delete(at)); watcher.unwatch(removeModelPaths); }
🧹 Nitpick comments (1)
packages/cli/src/actions/generate.ts (1)
41-49: Consider removing redundant type assertion.The
as AbstractDeclaration[]cast on line 47 is unnecessary sincemodel.declarationsis alreadyArray<AbstractDeclaration>andfilter()preserves the element type. This is a minor style nit.🔎 Proposed simplification
const getRootModelWatchPaths = (model: Model) => new Set<string>( - ( - model.declarations.filter( - (v) => - v.$cstNode?.parent?.element.$type === 'Model' && - !!v.$cstNode.parent.element.$document?.uri?.fsPath, - ) as AbstractDeclaration[] - ).map((v) => v.$cstNode!.parent!.element.$document!.uri!.fsPath), + model.declarations + .filter( + (v) => + v.$cstNode?.parent?.element.$type === 'Model' && + !!v.$cstNode.parent.element.$document?.uri?.fsPath, + ) + .map((v) => v.$cstNode!.parent!.element.$document!.uri!.fsPath), );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/cli/src/actions/generate.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/cli/src/actions/generate.ts
🧬 Code graph analysis (1)
packages/cli/src/actions/generate.ts (2)
packages/language/src/module.ts (1)
ZModelLanguageMetaData(27-27)packages/language/src/generated/ast.ts (4)
Model(559-563)Model(565-565)AbstractDeclaration(87-87)AbstractDeclaration(89-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (4)
packages/cli/src/actions/generate.ts (4)
2-3: LGTM!The new imports are appropriate for the watch mode functionality -
ZModelLanguageMetaDataprovides file extensions, andAbstractDeclarationsupports the declaration filtering logic.
16-23: LGTM!The
watchoption is appropriately added to the Options type.
61-121: Watch mode implementation looks solid.The watcher setup with proper extension filtering, debounced regeneration with error handling, and dynamic path management (add/remove) is well-implemented. The 500ms debounce is reasonable for handling rapid file saves.
125-155: LGTM!The
pureGeneratefunction cleanly extracts the generation logic, usesfromWatchto suppress verbose guidance during watch mode, and returns the model for path tracking. Good refactoring.
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @packages/cli/src/actions/generate.ts:
- Around line 93-101: Update the console.log message in the block that handles
removeModelPaths so it reads "Removed file(s) from watch:" instead of "Removed
file(s) to watch:"; locate the code using the variables removeModelPaths,
logsEnabled, logPaths and the console.log call (and related watcher.unwatch /
watchedPaths.delete) and change the string literal to use "from" for consistency
with the other log "Removed file from watch:".
🧹 Nitpick comments (1)
packages/cli/src/actions/generate.ts (1)
61-66: Consider adding error event handler for the watcher.Chokidar can emit
errorevents for permission issues or other filesystem problems. Adding an error handler would improve debuggability in edge cases.🔎 Suggested addition after watcher initialization
watcher.on('error', (error) => { console.error(colors.red(`Watcher error: ${error.message}`)); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/cli/src/actions/generate.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/cli/src/actions/generate.ts
🧬 Code graph analysis (1)
packages/cli/src/actions/generate.ts (2)
packages/language/src/module.ts (1)
ZModelLanguageMetaData(27-27)packages/language/src/generated/ast.ts (4)
Model(559-563)Model(565-565)AbstractDeclaration(87-87)AbstractDeclaration(89-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (4)
packages/cli/src/actions/generate.ts (4)
16-23: LGTM!The
Optionstype extension with thewatchproperty is clean and correctly typed.
41-49: The CST-based path extraction is well-structured.The filtering logic correctly identifies declarations with valid document URIs through the CST parent chain. The non-null assertions are safe given the preceding filter conditions that ensure these properties exist.
108-121: LGTM!The event handlers are correctly implemented:
unlinkimmediately removes the path from tracking before regeneration, which is appropriate since the file no longer exists.changetriggers debounced regeneration.The potential for
unwatchbeing called twice (once inunlink, once ifremoveModelPathsincludes it) is harmless as chokidar handles redundant unwatch calls gracefully.
125-155: Clean extraction of generation logic.The
pureGeneratefunction properly separates the generation logic from the watch orchestration. ThefromWatchparameter correctly suppresses the one-time client guidance during watch-triggered regenerations while still showing generation timing.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/cli/src/actions/generate.ts:
- Around line 68-114: The debounce currently drops changes that occur while
generationInProgress is true; modify reGenerateSchema to use a pending flag
(e.g., pendingGeneration) alongside reGenerateSchemaTimeout and
generationInProgress: when reGenerateSchema is invoked and generationInProgress
is true, set pendingGeneration = true and return (do not silently discard), and
after a generation finishes (where generationInProgress is set false) check
pendingGeneration; if true, clear pendingGeneration and call reGenerateSchema
(or schedule it) so one additional generation runs to capture accumulated
changes; ensure the timeout clearing/setting logic still uses
reGenerateSchemaTimeout so debouncing remains intact.
🧹 Nitpick comments (2)
tests/e2e/orm/client-api/typed-json-fields.test.ts (1)
124-124: Consider updating the similar assertion in the update test for consistency.The assertion now checks for the specific error path
data.identity.providers[0].id, which aligns with the improved granular error reporting. However, the similar test case at line 212 (works with update) still uses the generic/invalid/ipattern for the same validation scenario (missing requiredidfield).For consistency and better test coverage of the new error reporting, consider updating line 212 as well:
🔎 Suggested change for line 212
- ).rejects.toThrow(/invalid/i); + ).rejects.toThrow('data.identity.providers[0].id');tests/regression/test/issue-558.test.ts (1)
5-18: Add cleanup to prevent potential resource leaks.The test creates a client but doesn't disconnect it after the test completes. Other E2E tests in the codebase (e.g.,
typed-json-fields.test.ts) follow the pattern of disconnecting inafterEach. This ensures proper cleanup and prevents potential resource leaks when running the test suite.🔎 Suggested fix to add cleanup
describe('Regression for issue #558', () => { + let db: any; + + afterEach(async () => { + await db?.$disconnect(); + }); + it('verifies issue 558', async () => { - const db = await createTestClient(` + db = await createTestClient(` type Foo { x Int } model Model { id String @id @default(cuid()) foo Foo @json } `); await expect(db.model.create({ data: { foo: { x: 'hello' } } })).rejects.toThrow('data.foo.x'); }); });You'll also need to add
afterEachto the import:-import { describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it } from 'vitest';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/cli/src/actions/generate.tspackages/orm/src/client/crud/validator/index.tstests/e2e/orm/client-api/typed-json-fields.test.tstests/regression/test/issue-558.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/client-api/typed-json-fields.test.ts
tests/**/type*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure TypeScript inference and type coverage are validated through type coverage tests
Files:
tests/e2e/orm/client-api/typed-json-fields.test.ts
🧠 Learnings (9)
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/regression/test/issue-558.test.tstests/e2e/orm/client-api/typed-json-fields.test.tspackages/orm/src/client/crud/validator/index.tspackages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/regression/test/issue-558.test.tstests/e2e/orm/client-api/typed-json-fields.test.tspackages/orm/src/client/crud/validator/index.ts
📚 Learning: 2025-12-30T15:07:10.092Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:10.092Z
Learning: In the zenstackhq/zenstack-v3 codebase, ts-expect-error should be heavily avoided in production code (packages/*). It is acceptable in test files but production code should use proper type annotations, type assertions, or refactoring to resolve type errors instead.
Applied to files:
tests/regression/test/issue-558.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/orm/src/client/crud/validator/index.tspackages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
Applied to files:
packages/orm/src/client/crud/validator/index.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
packages/orm/src/client/crud/validator/index.ts
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/orm/src/client/crud/validator/index.tspackages/cli/src/actions/generate.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Applied to files:
packages/cli/src/actions/generate.ts
🧬 Code graph analysis (2)
tests/regression/test/issue-558.test.ts (1)
packages/testtools/src/client.ts (1)
createTestClient(101-248)
packages/cli/src/actions/generate.ts (2)
packages/language/src/module.ts (1)
ZModelLanguageMetaData(27-27)packages/language/src/generated/ast.ts (4)
Model(559-563)Model(565-565)AbstractDeclaration(87-87)AbstractDeclaration(89-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: build-test (22.x, postgresql)
🔇 Additional comments (6)
packages/orm/src/client/crud/validator/index.ts (2)
385-393: LGTM! Good improvement for granular error reporting.The switch from
z.customwith boolean return toz.any().superRefinecorrectly enables per-issue error propagation. This ensures that validation errors for typed JSON fields include specific paths (e.g.,data.foo.x) rather than generic messages.The
as anycast on line 388 is acceptable here as it bridges a minor type compatibility gap between Zod's parsed issue types and the context'saddIssueparameter type.
502-502: Good improvement for developer experience.Adding a descriptive error message for the
$exprfield validation helps developers quickly understand what went wrong when they provide an invalid value. The Zod 4errorparameter is used correctly here.packages/cli/src/actions/generate.ts (4)
1-24: LGTM!The new imports and Options type extension appropriately support the watch mode feature. All added imports are used correctly throughout the implementation.
32-66: Watch mode setup looks solid.The initialization correctly:
- Extracts schema file paths from CST nodes for all declaration types
- Sets up chokidar with appropriate filtering by schema extensions
- Maintains a Set for efficient path lookups
Note: As ymc9 mentioned, chokidar provides
watcher.getWatched(), but maintaining thewatchedPathsSet is reasonable for O(1) membership checks and simpler path diffing logic.
116-129: Event handlers implemented correctly.Both
unlinkandchangehandlers appropriately trigger regeneration. Theunlinkhandler properly maintains thewatchedPathsSet and unwatches the removed file.Note: These handlers will work correctly once the concurrency issue in
reGenerateSchemais resolved.
133-163: LGTM!The refactoring to
pureGenerateis clean and correct. ThefromWatchparameter appropriately controls client guidance printing, avoiding repetitive output during watch mode while ensuring the model is returned for path tracking.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/cli/src/actions/generate.ts (2)
52-52: Consider using chokidar's built-ingetWatched()API.As noted in a previous review comment, chokidar provides a
getWatched()method that returns the currently watched paths. This could eliminate the need to maintain a separatewatchedPathsSet and keep the watcher as the single source of truth.However, the current implementation is functional and correct. This is a nice-to-have simplification.
102-116: Consider adding graceful shutdown handling.The watcher is created but never closed. While the process terminating will clean up resources, explicitly closing the watcher on SIGINT/SIGTERM would be cleaner, especially if watch mode is embedded in a larger workflow.
🔎 Example pattern
const cleanup = () => { watcher.close(); process.exit(0); }; process.on('SIGINT', cleanup); process.on('SIGTERM', cleanup);This is optional since CLI tools typically rely on process termination for cleanup.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/cli/src/actions/generate.tspackages/common-helpers/src/index.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/common-helpers/src/index.tspackages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Applied to files:
packages/cli/src/actions/generate.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (4)
packages/cli/src/actions/generate.ts (3)
1-11: LGTM!Imports are well-organized and appropriate for the watch mode implementation.
119-149: LGTM!Clean extraction of generation logic into
pureGenerate. ThefromWatchparameter appropriately suppresses redundant client usage guidance during watch mode regenerations.
67-100:singleDebounceimplementation properly prevents race conditions.The implementation correctly:
- Prevents concurrent
pureGenerateexecutions via theinProgressflag- Queues pending regenerations through the
pendingInProgressflag and recursiverun()call after completion- Enables trailing execution with the third parameter (
true) to capture accumulated changesThe previous race condition concern is resolved.
packages/common-helpers/src/index.ts (1)
7-7: LGTM!The re-export follows the existing pattern and correctly exposes the
singleDebounceutility, which is a debounce function that handles concurrent execution scenarios.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/common-helpers/src/single-debounce.ts:
- Around line 6-10: The current run() logic only returns early when inProgress
&& reRunOnInProgressCall, which allows concurrent executions when inProgress is
true and reRunOnInProgressCall is false; change the check to always return if
inProgress but only set pendingInProgress when reRunOnInProgressCall is true
(e.g., if (inProgress) { if (reRunOnInProgressCall) pendingInProgress = true;
return; }) so the callback never runs concurrently but still queues a rerun when
requested.
🧹 Nitpick comments (1)
packages/common-helpers/src/single-debounce.ts (1)
1-1: Typo in parameter name:debounceMc→debounceMs.The parameter name suggests "Mc" but the standard abbreviation for milliseconds is "Ms".
🔎 Proposed fix
-export function singleDebounce(cb: () => void | PromiseLike<void>, debounceMc: number, reRunOnInProgressCall: boolean = false) { +export function singleDebounce(cb: () => void | PromiseLike<void>, debounceMs: number, reRunOnInProgressCall: boolean = false) { let timeout: ReturnType<typeof setTimeout> | undefined;Also update line 29:
- timeout = setTimeout(run, debounceMc); + timeout = setTimeout(run, debounceMs);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/common-helpers/src/single-debounce.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/common-helpers/src/single-debounce.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (1)
packages/common-helpers/src/single-debounce.ts (1)
26-30: LGTM on the returned debounce trigger.The pattern of clearing the existing timeout before setting a new one correctly implements debounce semantics.
Minor style note: missing semicolon after the closing brace on line 30, but this is non-critical.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/common-helpers/src/single-debounce.ts (1)
1-1: Rename parameterdebounceMctodebounceMs.The parameter name appears to represent milliseconds but uses the non-standard abbreviation
Mc. Standard convention isMsfor milliseconds.📝 Suggested rename
-export function singleDebounce(cb: () => void | PromiseLike<void>, debounceMc: number, reRunOnInProgressCall: boolean = false) { +export function singleDebounce(cb: () => void | PromiseLike<void>, debounceMs: number, reRunOnInProgressCall: boolean = false) {Also update line 32:
- timeout = setTimeout(run, debounceMc); + timeout = setTimeout(run, debounceMs);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/common-helpers/src/single-debounce.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
📚 Learning: 2025-12-30T15:07:06.254Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.
Applied to files:
packages/common-helpers/src/single-debounce.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (3)
packages/common-helpers/src/single-debounce.ts (3)
6-13: LGTM! Concurrency guard correctly implemented.The fix from previous review comments has been correctly applied. The function now always returns early when
inProgressis true (preventing concurrent execution), while conditionally queueing a rerun only whenreRunOnInProgressCallis true.
29-33: LGTM! Clean debounce wrapper implementation.The returned wrapper correctly implements the debounce pattern by clearing any existing timeout and scheduling a new execution.
18-19: No action needed. The callback passed tosingleDebouncealready wraps all logic in a try-catch block and logs errors withconsole.error, ensuring that generation failures do not cause unhandled rejections or crash the watch mode process.
ymc9
left a comment
ymc9
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the quick follow-up. I'm merging it now.
2 participants
For #514
Idk about tests, cuz its not force call process.exit
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.