merge dev to main (v3.2.1) #590
Conversation
* fix(zmodel): improve attribute argument assignability check fixes #584 * fix JSON array check * update * update ts schema generator
Co-authored-by: ymc9 <[email protected]>
📝 WalkthroughWalkthroughThis PR bumps package versions from 3.2.0 to 3.2.1 across the monorepo and introduces targeted functional improvements: refactored attribute parameter validation with discriminated union errors and JSON default handling, conditional _count field typing in ORM queries, refined read-back filtering in update operations, explicit null default handling in schema generation, and new regression tests for _count and JSON validation scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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 |
|
Claude finished @ymc9's task —— View job Code Review for v3.2.1 ReleaseReviewing the dev -> main merge for version 3.2.1. I've analyzed the changes across 33 files focusing on code quality, potential bugs, performance, security, and test coverage. Review Tasks:
🔍 Review SummaryThis PR represents a clean v3.2.1 release containing important bug fixes and improvements. The changes are well-structured and follow good engineering practices. ✅ Positive FindingsVersion Management:
Bug Fixes & Improvements:
Code Quality:
Testing:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
package.jsonpackages/auth-adapters/better-auth/package.jsonpackages/cli/package.jsonpackages/clients/client-helpers/package.jsonpackages/clients/tanstack-query/package.jsonpackages/common-helpers/package.jsonpackages/config/eslint-config/package.jsonpackages/config/typescript-config/package.jsonpackages/config/vitest-config/package.jsonpackages/create-zenstack/package.jsonpackages/ide/vscode/package.jsonpackages/language/package.jsonpackages/language/src/validators/attribute-application-validator.tspackages/orm/package.jsonpackages/orm/src/client/crud-types.tspackages/orm/src/client/crud/operations/update.tspackages/plugins/policy/package.jsonpackages/schema/package.jsonpackages/sdk/package.jsonpackages/sdk/src/ts-schema-generator.tspackages/server/package.jsonpackages/testtools/package.jsonpackages/zod/package.jsonsamples/orm/package.jsontests/e2e/orm/client-api/find.test.tstests/e2e/orm/policy/crud/update.test.tstests/e2e/orm/schemas/typing/typecheck.tstests/e2e/package.jsontests/regression/package.jsontests/regression/test/issue-584.test.tstests/regression/test/issue-586.test.tstests/runtimes/bun/package.jsontests/runtimes/edge-runtime/package.json
🧰 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/policy/crud/update.test.tstests/e2e/orm/client-api/find.test.tstests/e2e/orm/schemas/typing/typecheck.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/schemas/typing/typecheck.ts
🧠 Learnings (11)
📚 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:
packages/clients/tanstack-query/package.jsonpackages/common-helpers/package.jsonpackage.jsonpackages/zod/package.jsonpackages/config/eslint-config/package.jsonpackages/config/typescript-config/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 packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
samples/orm/package.jsonpackages/orm/src/client/crud-types.tstests/e2e/orm/policy/crud/update.test.tspackages/orm/src/client/crud/operations/update.tstests/e2e/orm/client-api/find.test.tspackages/orm/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/schema/package.jsonpackages/language/package.jsonpackages/zod/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 tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/regression/test/issue-584.test.tstests/regression/test/issue-586.test.tstests/e2e/orm/policy/crud/update.test.tstests/e2e/orm/client-api/find.test.tstests/e2e/orm/schemas/typing/typecheck.tstests/e2e/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 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-584.test.tstests/regression/test/issue-586.test.tspackages/orm/src/client/crud-types.tstests/e2e/orm/policy/crud/update.test.tstests/e2e/orm/client-api/find.test.tstests/e2e/orm/schemas/typing/typecheck.tspackages/orm/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 tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests
Applied to files:
tests/regression/test/issue-584.test.tstests/e2e/orm/schemas/typing/typecheck.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-types.tstests/e2e/orm/policy/crud/update.test.tspackages/orm/package.json
📚 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-types.tspackages/orm/src/client/crud/operations/update.tspackages/sdk/src/ts-schema-generator.tspackages/language/src/validators/attribute-application-validator.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/cli/**/*.test.{ts,tsx} : CLI package tests should focus on action-specific tests for each command
Applied to files:
tests/e2e/orm/policy/crud/update.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: Use `pnpm` with workspaces for package management, pinned to version `pnpm10.12.1`
Applied to files:
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 ide/vscode/**/{package.json,version.ts} : The VSCode IDE extension package should maintain a different version from other packages to comply with VSCode Marketplace requirements
Applied to files:
packages/ide/vscode/package.jsonpackages/config/typescript-config/package.json
🧬 Code graph analysis (5)
tests/regression/test/issue-584.test.ts (1)
packages/testtools/src/client.ts (1)
createTestClient(101-248)
tests/regression/test/issue-586.test.ts (1)
packages/testtools/src/client.ts (1)
createPolicyTestClient(258-269)
packages/orm/src/client/crud/operations/update.ts (1)
packages/orm/src/client/query-utils.ts (1)
getIdValues(181-187)
tests/e2e/orm/client-api/find.test.ts (1)
tests/e2e/orm/client-api/utils.ts (2)
createUser(6-21)createPosts(23-32)
packages/sdk/src/ts-schema-generator.ts (1)
packages/language/src/generated/ast.ts (1)
isNullExpr(593-595)
⏰ 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). (5)
- GitHub Check: Agent
- GitHub Check: build-test (22.x, postgresql)
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (33)
packages/config/eslint-config/package.json (1)
3-3: Version bump is consistent with the coordinated release.The version update from 3.2.0 to 3.2.1 aligns with the PR objective and other package version updates in the monorepo.
packages/testtools/package.json (1)
3-3: Version bump is consistent with the coordinated release.The version update from 3.2.0 to 3.2.1 aligns with the PR objective and other package version updates in the monorepo. Dependencies remain unchanged.
samples/orm/package.json (1)
3-3: Version bump is consistent with the coordinated release.The version update from 3.2.0 to 3.2.1 aligns with the PR objective and other package version updates in the monorepo.
packages/server/package.json (1)
3-3: Version bump is appropriate.Patch version update from 3.2.0 to 3.2.1 with no dependency changes. Dependency versions remain stable across the board.
tests/e2e/package.json (1)
3-3: Version bump aligns with monorepo release.Patch version update with no test infrastructure or dependency changes. Workspace dependencies will automatically reference updated versions.
packages/ide/vscode/package.json (1)
4-4: No action needed—VSCode extension versioning is correct.The VSCode extension maintains synchronized versioning with the rest of the monorepo (3.2.1), which is the current and appropriate approach. No separate versioning mechanism exists, and the standard VSCode Extension CLI (vsce) is used for publishing without requiring custom version handling. The zenstack-v3 extension is properly configured as a separate extension on the VSCode Marketplace for ZenStack v3.x, coexisting alongside the original extension for v2.x.
Likely an incorrect or invalid review comment.
packages/plugins/policy/package.json (1)
3-3: VSCode IDE extension requires separate versioning; do not use 3.2.1 for packages/ide/vscode/package.json.The other package.json files correctly bump to 3.2.1, but packages/ide/vscode/package.json must maintain a separate version to comply with VSCode Marketplace requirements. Per the codebase guidelines, the VSCode IDE extension should not align with the monorepo's standard versioning scheme. Update packages/ide/vscode/package.json to use its own version number (e.g., a separate sequence) and verify compatibility with VSCode Marketplace release processes before merging.
⛔ Skipped due to 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 ide/vscode/**/{package.json,version.ts} : The VSCode IDE extension package should maintain a different version from other packages to comply with VSCode Marketplace requirementspackages/orm/package.json (1)
3-3: LGTM: Version bump aligns with the patch release.The version increment from 3.2.0 to 3.2.1 is appropriate for this patch release merge to main.
packages/auth-adapters/better-auth/package.json (1)
3-3: LGTM: Version bump aligns with the patch release.The version increment from 3.2.0 to 3.2.1 is consistent with the monorepo-wide version update.
package.json (1)
3-3: LGTM: Root package version bump for v3.2.1 release.The version increment correctly reflects the patch release being merged from dev to main.
packages/clients/tanstack-query/package.json (1)
3-3: LGTM: Version bump aligns with the patch release.The version increment from 3.2.0 to 3.2.1 maintains consistency across the monorepo packages.
packages/sdk/package.json (1)
3-3: LGTM: Version bump aligns with the patch release.The version increment from 3.2.0 to 3.2.1 is consistent with the v3.2.1 release objectives.
packages/language/package.json (1)
4-4: Version bump to 3.2.1 looks consistent with the release PR.packages/common-helpers/package.json (1)
3-3: LGTM: version bumped to 3.2.1 only.tests/runtimes/bun/package.json (1)
3-3: LGTM: bun-e2e version bumped to 3.2.1 (no other changes).tests/regression/package.json (1)
3-3: LGTM: regression package version bumped to 3.2.1 only.tests/runtimes/edge-runtime/package.json (1)
3-3: LGTM: edge-runtime-e2e version bumped to 3.2.1 (no other changes).packages/zod/package.json (1)
3-3: LGTM: Version bump is consistent with the release.The version increment from 3.2.0 to 3.2.1 aligns with the PR objectives for the v3.2.1 release.
packages/config/typescript-config/package.json (1)
3-3: LGTM: Version bump is consistent with the release.The version increment from 3.2.0 to 3.2.1 aligns with the PR objectives for the v3.2.1 release.
packages/schema/package.json (1)
3-3: LGTM: Version bump is consistent with the release.The version increment from 3.2.0 to 3.2.1 aligns with the PR objectives for the v3.2.1 release.
packages/cli/package.json (1)
6-6: LGTM: Version bump is consistent with the release.The version increment from 3.2.0 to 3.2.1 aligns with the PR objectives for the v3.2.1 release.
packages/config/vitest-config/package.json (1)
4-4: LGTM! Clean version bump.The version increment from 3.2.0 to 3.2.1 is consistent with the PR's objectives.
packages/orm/src/client/crud/operations/update.ts (1)
46-47: LGTM! Refined read-back filter.The read-back filter now uses only ID fields via
getIdValues(), which prevents extraneous fields from the underlying executor result from polluting the filter. This is a more precise approach and aligns well with the intent to read back the specific entity by its identifier.tests/regression/test/issue-584.test.ts (1)
1-140: LGTM! Comprehensive JSON default validation tests.The tests thoroughly validate JSON default value handling for both scalar Json/Json? types (requiring JSON string literals) and Json[] array types (requiring arrays of JSON string literals). The test cases cover edge cases including null, plain strings, nested objects, and mixed value types.
tests/regression/test/issue-586.test.ts (1)
1-87: LGTM! Well-structured regression test.The test verifies that JSON array updates with a custom mutation plugin don't trigger false "cannot-read-back" errors. The test setup is comprehensive with proper auth context, schema definition, and cleanup in the finally block. This test effectively validates the fix in the update operation handler.
packages/orm/src/client/crud-types.ts (1)
217-221: LGTM! Precise conditional typing for _count.The conditional augmentation correctly adds the _count field to the result type only when it's explicitly included and not disabled. This type-level refinement improves IDE autocomplete and type safety by avoiding unnecessary _count fields in query results.
packages/sdk/src/ts-schema-generator.ts (2)
610-665: LGTM! Explicit null default handling is correctly implemented.The null default value handling is well-integrated:
- Line 610-613: Checks for null and emits
ExpressionUtils._null()- Line 614: Properly excludes arrays from the object branch
- Lines 773-774:
getMappedValuecorrectly returns null forNullExpr- Type signatures (lines 731-739, 752) properly include
| nullThe logic correctly separates null, non-array objects, and arrays into distinct branches.
776-776: Improved error message provides better context.Changing from "Unsupported default value type" to "Unsupported expression type" is more accurate since this function processes expressions, not just default values.
packages/language/src/validators/attribute-application-validator.ts (3)
396-411: Excellent refactor: Discriminated union enables precise error reporting.The change from boolean return to
{ result: true } | { result: false; error: string }allows specific error messages to be propagated to users instead of generic "invalid argument type" messages. This significantly improves the developer experience.
619-629: JSON validation helper is concise and correct.The
isLiteralJsonStringhelper properly validates JSON by:
- Checking if the value is a string literal
- Attempting
JSON.parseto validate JSON structure- Catching parse errors and returning false
This is an appropriate validation strategy for JSON literals.
450-547: Consistent error propagation throughout the function.All branches now return structured errors with specific messages:
- Line 466: "expected array/non-array"
- Line 482: "expected an array of field references"
- Line 488: "expected a field reference"
- Line 509: "expected array/non-array" (for enum mismatch)
The consistent use of the discriminated union pattern improves maintainability and error clarity.
tests/e2e/orm/client-api/find.test.ts (1)
1125-1169: Comprehensive test coverage for _count inside include.The test validates three key scenarios:
- Lines 1130-1142: _count with select inside include
- Lines 1145-1155: _count with boolean true inside include
- Lines 1158-1168: _count with filtered posts inside include
Each scenario verifies:
- Runtime behavior (correct counts)
- TypeScript typing (comment line 1140 confirms type checking)
- Both posts data and _count are accessible
The test aligns with the coding guideline to validate real-world schema compatibility.
Based on learnings, E2E tests should validate real-world schema compatibility with established projects.
tests/e2e/orm/schemas/typing/typecheck.ts (1)
165-182: TypeScript type validation for _count inside include is correctly implemented.The type checks validate that:
- Line 169:
_count: { select: { posts: true } }is properly typed- Line 172-173: Both
posts[0]?.titleand_count.postsare accessible- Lines 175-182: Boolean
_count: truesyntax also works correctlyThese compile-time checks complement the runtime tests in
find.test.tsand ensure type inference is correct. The console.log statements serve as type assertions that will fail compilation if types are incorrect.Based on learnings, TypeScript inference and type coverage should be validated through type coverage tests.
There was a problem hiding this comment.
Pull request overview
This PR merges the dev branch into main for the v3.2.1 release. The changes include bug fixes for JSON field validation and update operations, a new feature for _count support inside include queries, and version bumps across all packages.
Changes:
- Version bumps from 3.2.0 to 3.2.1 across all packages
- Fixed JSON field default value validation to require JSON string literals (issue #584)
- Fixed read-back issue in update operations with mutation plugins (issue #586)
- Added TypeScript support for
_countinsideincludequeries
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/language/src/validators/attribute-application-validator.ts | Enhanced validation for JSON default values with improved error messages |
| packages/sdk/src/ts-schema-generator.ts | Added proper handling of null default values in schema generation |
| packages/orm/src/client/crud/operations/update.ts | Fixed read-back logic to use only ID fields for querying after update |
| packages/orm/src/client/crud-types.ts | Added TypeScript type support for _count property inside include |
| tests/regression/test/issue-584.test.ts | Comprehensive tests for JSON default value validation |
| tests/regression/test/issue-586.test.ts | Test for update read-back fix with mutation plugins |
| tests/e2e/orm/client-api/find.test.ts | Tests for new _count inside include feature |
| tests/e2e/orm/schemas/typing/typecheck.ts | Type-checking validation for _count in include |
| tests/e2e/orm/policy/crud/update.test.ts | Import ordering cleanup |
| 29 package.json files | Consistent version bump to 3.2.1 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary by CodeRabbit
Chores
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.