-
-
Notifications
You must be signed in to change notification settings - Fork 12
merge dev to main (v3.0.0-alpha.26) #177
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
Co-authored-by: ymc9 <[email protected]>
* fix: relation selection input validation issue * update * fix test
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughVersion bumps across many package.json files to 3.0.0-alpha.26. Runtime FindArgs type moves Distinct/Cursor into the collection branch. Validator refactors relation select/include schema via a helper to add filtering/pagination for relations. Extensive nested-relation tests added and an error expectation updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Claude finished @ymc9's task —— View job Code Review Complete ✅I've reviewed PR #177 (dev → main merge) and here's my comprehensive analysis: Overall AssessmentThis is a well-structured release PR that bumps version from Key Changes📦 Version Bumps (20 files)
🔧 Runtime Validation Improvements
Code Quality Analysis✅ Strengths:
🔍 Key Technical Improvements: validator.ts (
find.test.ts (
Security & Performance
Test Coverage
Recommendations
SummaryThis PR represents a quality release that:
Approval Status: ✅ APPROVED - Well-executed release with important improvements. |
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.
Pull Request Overview
This PR merges development branch changes to main, primarily involving a version bump from 3.0.0-alpha.25 to 3.0.0-alpha.26 across all packages, along with significant improvements to relation filtering and query validation.
- Version bump across all packages from
3.0.0-alpha.25to3.0.0-alpha.26 - Enhanced relation filtering capabilities for optional to-one and to-many relations
- Improved query validation with more specific error messages and comprehensive test coverage
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json (root) | Version bump to 3.0.0-alpha.26 |
| packages/*/package.json | Version bump across all packages |
| packages/runtime/src/client/crud/validator.ts | Refactored relation schema validation with new filtering capabilities |
| packages/runtime/src/client/crud-types.ts | Restructured type definitions for better query argument organization |
| packages/runtime/test/client-api/find.test.ts | Added comprehensive tests for new relation filtering features |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
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: 2
🧹 Nitpick comments (4)
packages/testtools/package.json (1)
3-3: Test tools version bump approvedLooks consistent with the monorepo release. Since this is a published package, ensure CHANGELOG/release notes reflect the new pre-release if applicable.
package.json (1)
3-3: Root version bump confirmed.Matches the monorepo release plan. Consider tagging the release and updating release notes when merging dev to main.
packages/runtime/test/client-api/find.test.ts (2)
640-659: Solid coverage: nested to-many selection with where/orderBy/distinct in select/include.These tests validate the extended nested relation args well and mirror both select and include paths. Consider adding one negative test asserting that nested objects cannot have both select and include once the validator enforces that rule.
I can add a failing test now and follow up with the validator refinement. Want me to open a PR with both?
Also applies to: 661-680
872-873: Assertion updated to new validation message; consider asserting on error type for stability.The message "Invalid find args" is fine, but string-based matching can be brittle. If feasible, assert on InputValidationError to make the test less sensitive to message changes.
For example:
await expect( client.post.findFirstOrThrow({ // @ts-expect-error include: { author: { where: { email: user.email } } }, }) ).rejects.toThrow(InputValidationError);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
package.json(1 hunks)packages/cli/package.json(1 hunks)packages/common-helpers/package.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/dialects/sql.js/package.json(1 hunks)packages/eslint-config/package.json(1 hunks)packages/ide/vscode/package.json(1 hunks)packages/language/package.json(1 hunks)packages/runtime/package.json(1 hunks)packages/runtime/src/client/crud-types.ts(1 hunks)packages/runtime/src/client/crud/validator.ts(3 hunks)packages/runtime/test/client-api/find.test.ts(2 hunks)packages/sdk/package.json(1 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(1 hunks)packages/typescript-config/package.json(1 hunks)packages/vitest-config/package.json(1 hunks)packages/zod/package.json(1 hunks)samples/blog/package.json(1 hunks)tests/e2e/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
tests/e2e/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
E2E tests are in
tests/e2e/directory
Files:
tests/e2e/package.json
{packages,samples,tests}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
tests/e2e/package.jsonsamples/blog/package.jsonpackages/ide/vscode/package.jsonpackages/eslint-config/package.jsonpackages/tanstack-query/package.jsonpackages/zod/package.jsonpackages/create-zenstack/package.jsonpackages/dialects/sql.js/package.jsonpackages/sdk/package.jsonpackages/runtime/package.jsonpackages/cli/package.jsonpackages/language/package.jsonpackages/vitest-config/package.jsonpackages/common-helpers/package.jsonpackages/testtools/package.jsonpackages/runtime/test/client-api/find.test.tspackages/typescript-config/package.jsonpackages/runtime/src/client/crud/validator.tspackages/runtime/src/client/crud-types.ts
🧠 Learnings (2)
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: `zenstack generate` compiles ZModel to TypeScript schema (`schema.ts`)
Applied to files:
packages/language/package.json
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: Schema-first approach with ZModel DSL extension of Prisma schema language
Applied to files:
packages/runtime/src/client/crud/validator.ts
🧬 Code Graph Analysis (1)
packages/runtime/src/client/crud/validator.ts (1)
packages/sdk/src/schema/schema.ts (1)
FieldDef(59-74)
⏰ 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: claude-review
- GitHub Check: build-test (20.x)
🔇 Additional comments (21)
packages/cli/package.json (2)
6-6: LGTM: version bumped to 3.0.0-alpha.26Monorepo CLI package metadata-only change looks good.
6-6: Monorepo versions & internal deps verified — no action requiredI ran the checks: every package.json version is 3.0.0-alpha.26, there are no occurrences of 3.0.0-alpha.25, and all @zenstackhq/* dependencies are set to "workspace:*".
packages/sdk/package.json (1)
3-3: LGTM: version bumped to 3.0.0-alpha.26SDK package version-only change; exports and dependencies unchanged. No concerns.
packages/tanstack-query/package.json (1)
3-3: LGTM: version bumped to 3.0.0-alpha.26No functional changes. Peer dependency range for @tanstack/react-query remains ^5, which matches current package intent.
packages/zod/package.json (1)
3-3: LGTM: version bumped to 3.0.0-alpha.26Metadata-only update; dependencies and exports intact.
packages/ide/vscode/package.json (1)
4-4: LGTM: version bumped to 3.0.0-alpha.26VS Code extension manifest version updated; no other changes. Scripts and engine constraints unchanged.
tests/e2e/package.json (1)
3-3: Version bump aligns with the release trainLooks good. This keeps e2e package in sync with the repo-wide 3.0.0-alpha.26 release.
packages/runtime/package.json (1)
3-3: Runtime version bump verified — all packages updated to 3.0.0-alpha.26Ran the provided script: every package.json with a version field is 3.0.0-alpha.26 and no references to "3.0.0-alpha.25" were found.
packages/vitest-config/package.json (1)
4-4: LGTM: metadata-only version bumpNo behavior changes here; keeping in lockstep with the rest of the repo is good.
packages/typescript-config/package.json (1)
3-3: Typescript config version bump is consistentPrivate package; safe and aligned with the release train.
packages/create-zenstack/package.json (1)
3-3: Version bump to 3.0.0-alpha.26 looks good.No API or config changes; aligns with the repo-wide release bump.
packages/eslint-config/package.json (1)
3-3: Version bump acknowledged.Private package; safe metadata-only change.
samples/blog/package.json (1)
3-3: Sample app version updated correctly.Dependencies reference workspace and catalog; no action needed.
package.json (1)
1-41: All package.json files have been bumped to 3.0.0-alpha.26 — no action required.I ran the verification script; every package.json in the repo (root, packages/*, samples, tests) reports version 3.0.0-alpha.26.
packages/language/package.json (1)
4-4: Language package version bump is consistent.No other changes in scripts/exports; nothing else to verify here.
packages/common-helpers/package.json (1)
3-3: LGTM: version bump only.Version updated to 3.0.0-alpha.26 with no other changes. No issues.
packages/dialects/sql.js/package.json (1)
3-3: LGTM: version bump only.Version updated to 3.0.0-alpha.26 with no other changes. No issues.
packages/runtime/src/client/crud/validator.ts (2)
592-592: Good reuse: relation select/include shape now centralized.Replacing inline unions with makeRelationSelectIncludeSchema reduces duplication and keeps relation args consistent across select/include.
675-675: Consistent application: include schema switched to centralized relation helper.Keeps select/include relation shapes in sync and easier to evolve.
packages/runtime/test/client-api/find.test.ts (2)
682-696: Good: nested to-one relation projection.Covers selecting author.email via both select and include on a to-one relation. This aligns with the new relation helper.
714-730: Great: optional to-one filtering semantics verified.Asserting profile is object/null based on where filters demonstrates the conditional where support added for optional to-one relations.
|
Claude finished @ymc9's task —— View job Code Review Complete ✅I've thoroughly reviewed PR #177 (dev → main merge for v3.0.0-alpha.26). Here's my comprehensive analysis: Overall AssessmentThis is a well-structured release PR that properly bumps version across all packages while introducing important runtime validation improvements for relation filtering. The changes are backward-compatible and enhance the developer experience. Key Changes Summary📦 Version Management (20 files)
🔧 Core Runtime Improvements
Code Quality & Best Practices ✅Strengths:
Minor Improvements Needed:
Potential Issues 🔍Type/Validation Alignment Concern:
Recommendation: Update Nested Relation Validation Gap:
Performance Considerations ✅Positive:
No Concerns:
Security Assessment ✅No Security Issues:
Test Coverage Assessment ✅Excellent Coverage:
Key Test Scenarios Covered:
Specific File Analysis
Recommendations
Final Verdict: ✅ APPROVEDThis is a quality release that:
The identified issues are minor and can be addressed in follow-up PRs without blocking this release. ** |
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: 2
♻️ Duplicate comments (1)
packages/runtime/src/client/crud/validator.ts (1)
629-655: Enforce nested select/include/omit mutual exclusivity and update comment to include 'distinct'Currently, nested relation objects allow
select+includeorselect+omittogether. Add local refinements so nested relations follow top-level rules. Also, the comment omitsdistinctthough it’s supported.private makeRelationSelectIncludeSchema(fieldDef: FieldDef) { - return z.union([ - z.boolean(), - z.strictObject({ + const relObject = z.strictObject({ ...(fieldDef.array || fieldDef.optional ? { // to-many relations and optional to-one relations are filterable where: z.lazy(() => this.makeWhereSchema(fieldDef.type, false)).optional(), } : {}), select: z.lazy(() => this.makeSelectSchema(fieldDef.type)).optional(), include: z.lazy(() => this.makeIncludeSchema(fieldDef.type)).optional(), omit: z.lazy(() => this.makeOmitSchema(fieldDef.type)).optional(), ...(fieldDef.array ? { - // to-many relations can be ordered, skipped, taken, and cursor-located + // to-many relations can be ordered, skipped, taken, cursor-located, and distinct-selected orderBy: z.lazy(() => this.makeOrderBySchema(fieldDef.type, true, false)).optional(), skip: this.makeSkipSchema().optional(), take: this.makeTakeSchema().optional(), cursor: this.makeCursorSchema(fieldDef.type).optional(), distinct: this.makeDistinctSchema(fieldDef.type).optional(), } : {}), - }), - ]); + }) + .refine((v) => !(v.select && v.include), '"select" and "include" cannot be used together') + .refine((v) => !(v.select && v.omit), '"select" and "omit" cannot be used together'); + return z.union([z.boolean(), relObject]); }
🧹 Nitpick comments (1)
packages/runtime/src/client/plugin.ts (1)
49-49: Consider re-exporting AllCrudOperation from this module for plugin authorsYou already re-export
CrudOperationhere. Re-exportingAllCrudOperationavoids deep import paths for consumers.Apply at Line 49:
-export { type CrudOperation } from './crud/operations/base'; +export { type CrudOperation, type AllCrudOperation } from './crud/operations/base';Also applies to: 64-64
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/runtime/src/client/crud/operations/base.ts(1 hunks)packages/runtime/src/client/crud/validator.ts(6 hunks)packages/runtime/src/client/plugin.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{packages,samples,tests}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/runtime/src/client/crud/operations/base.tspackages/runtime/src/client/plugin.tspackages/runtime/src/client/crud/validator.ts
🧠 Learnings (2)
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: Schema is used to instantiate `ZenStackClient` with type-safe CRUD operations
Applied to files:
packages/runtime/src/client/plugin.ts
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: Schema-first approach with ZModel DSL extension of Prisma schema language
Applied to files:
packages/runtime/src/client/crud/validator.ts
🧬 Code Graph Analysis (3)
packages/runtime/src/client/crud/operations/base.ts (1)
packages/runtime/src/client/plugin.ts (1)
CrudOperation(49-49)
packages/runtime/src/client/plugin.ts (1)
packages/runtime/src/client/crud/operations/base.ts (1)
AllCrudOperation(71-71)
packages/runtime/src/client/crud/validator.ts (3)
packages/runtime/src/client/crud/operations/base.ts (1)
options(94-96)packages/runtime/src/client/executor/zenstack-query-executor.ts (1)
options(54-56)packages/sdk/src/schema/schema.ts (1)
FieldDef(59-74)
⏰ 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 (20.x)
- GitHub Check: claude-review
🔇 Additional comments (5)
packages/runtime/src/client/plugin.ts (1)
5-5: Switch to AllCrudOperation import — LGTMThis aligns the plugin surface with the extended operation union.
packages/runtime/src/client/crud/validator.ts (4)
199-228: Move cursor/distinct under collection (!unique) — LGTM and consistent with typesGating
skip/take/orderBy/cursor/distincton!options.uniquecorrectly reflects collection-only semantics.
592-597: Select: unified relation schema helper usage — LGTMReplacing inline unions with
makeRelationSelectIncludeSchemareduces duplication and keeps select/include/omit consistent.
674-675: Include: unified relation schema helper usage — LGTMKeeps select/include/omit parity for relation fields and centralizes future adjustments.
47-49: validateFindArgs narrowed to { unique } — callers verifiedThe only caller is FindOperationHandler which passes operation === 'findUnique' as the unique flag. client-impl maps the *OrThrow variants to the same internal operation strings, so unique=true is used for findUnique/findUniqueOrThrow and unique=false for findMany/findFirst/findFirstOrThrow.
- packages/runtime/src/client/crud/operations/find.ts — validateFindArgs(this.model, operation === 'findUnique', normalizedArgs)
- packages/runtime/src/client/client-impl.ts — createPromise(...) uses 'findUnique' for both findUnique and findUniqueOrThrow, and 'findFirst' for findFirst and findFirstOrThrow (findMany uses 'findMany')
- packages/runtime/src/client/crud/validator.ts — makeFindSchema(model, { unique }) (schema respects unique)
No change required.
Summary by CodeRabbit
New Features
Refactor
Tests
Chores