Conversation
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
* fix(delegate): exclude uninheritable attributes properly * fix tests
WalkthroughThis PR updates many package versions from 3.0.0-beta.29 → 3.0.0-beta.30, adjusts attribute inheritance/filtering in the language and Prisma schema generator, narrows delegate-field inheritance checks, adds an index to an Asset test schema, and enables Prisma Push in a regression test. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 PR Review Complete ✅This PR merges dev → main for v3.0.0-beta.30, containing primarily version bumps and one important delegate inheritance fix. Review Analysis:
Overall Assessment: APPROVED ✅✅ Version Management
✅ Key Bug Fix (Commit eb1a0c2)Fix: Delegate inheritance properly excludes uninheritable attributes Changes analyzed:
Code Quality:
✅ Test Coverage
✅ Security & Performance
✅ Code Standards
Recommendations:
|
There was a problem hiding this comment.
Pull request overview
This PR merges the dev branch to main for version 3.0.0-beta.30, bumping the version from beta.29 to beta.30 across all packages. The key change is a refactoring of how non-inheritable model-level attributes are handled in delegate inheritance. Previously, only the @@map attribute was filtered out during Prisma schema generation. Now, the filtering is done more comprehensively at the language utilities level, preventing @@map, @@unique, and @@index attributes from being inherited from base delegate models.
- Refactored attribute inheritance logic to centralize filtering of non-inheritable attributes (
@@map,@@unique,@@index) in the language utilities layer - Added test coverage for delegate inheritance with multiple non-inheritable attributes
- Updated delegate schema examples to demonstrate proper behavior with non-inherited
@@indexattributes
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Version bump to 3.0.0-beta.30 |
| packages/auth-adapters/better-auth/package.json | Version bump to 3.0.0-beta.30 |
| packages/cli/package.json | Version bump to 3.0.0-beta.30 |
| packages/clients/tanstack-query/package.json | Version bump to 3.0.0-beta.30 |
| packages/common-helpers/package.json | Version bump to 3.0.0-beta.30 |
| packages/config/eslint-config/package.json | Version bump to 3.0.0-beta.30 |
| packages/config/typescript-config/package.json | Version bump to 3.0.0-beta.30 |
| packages/config/vitest-config/package.json | Version bump to 3.0.0-beta.30 |
| packages/create-zenstack/package.json | Version bump to 3.0.0-beta.30 |
| packages/language/package.json | Version bump to 3.0.0-beta.30 |
| packages/language/src/utils.ts | Added isNonInheritableAttribute function to filter @@Map, @@unique, and @@index from inherited attributes |
| packages/orm/package.json | Version bump to 3.0.0-beta.30 |
| packages/plugins/policy/package.json | Version bump to 3.0.0-beta.30 |
| packages/schema/package.json | Version bump to 3.0.0-beta.30 |
| packages/sdk/package.json | Version bump to 3.0.0-beta.30 |
| packages/sdk/src/prisma/prisma-schema-generator.ts | Removed isInheritedMapAttribute method; filtering now handled centrally in utils.ts |
| packages/server/package.json | Version bump to 3.0.0-beta.30 |
| packages/testtools/package.json | Version bump to 3.0.0-beta.30 |
| packages/zod/package.json | Version bump to 3.0.0-beta.30 |
| samples/next.js/package.json | Version bump to 3.0.0-beta.30 |
| samples/orm/package.json | Version bump to 3.0.0-beta.30 |
| tests/e2e/package.json | Version bump to 3.0.0-beta.30 |
| tests/e2e/orm/schemas/delegate/schema.zmodel | Added @@index([ownerId]) to Asset model to demonstrate non-inheritance |
| tests/e2e/orm/schemas/delegate/schema.ts | Generated schema updated with @@index attribute on Asset (not inherited by Video/Image) |
| tests/regression/package.json | Version bump to 3.0.0-beta.30 |
| tests/regression/test/v2-migrated/issue-1243.test.ts | Added usePrismaPush option to properly test delegate inheritance with non-inheritable attributes |
| tests/runtimes/bun/package.json | Version bump to 3.0.0-beta.30 |
| tests/runtimes/edge-runtime/package.json | Version bump to 3.0.0-beta.30 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Claude finished @ymc9's task —— View job PR Review - v3.0.0-beta.30 ReleaseReviewing this PR for code quality, potential bugs, performance, security, and test coverage. Review Tasks:
Overall Assessment: APPROVED ✅✅ Version Management
✅ Key Bug Fix (Commit eb1a0c2)Fix: Delegate inheritance properly excludes uninheritable attributes Changes analyzed:
Code Quality:
✅ Test Coverage
✅ Security & Performance
✅ Code Standards
Recommendations:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/language/src/validators/datamodel-validator.ts (1)
433-436: Doc/semantics mismatch: “directly or indirectly” vs actual check.
The helper currently checks only whether the field’s declaring container is a delegate model and differs fromcontextModel. If “indirectly” is intended (multi-level inheritance), that’s probably still fine as long as the declaring container is always the original delegate ancestor—but the comment should reflect the actual criterion (declared-on-delegate-and-not-context), not imply traversal.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/language/src/validators/datamodel-validator.ts(2 hunks)packages/language/test/delegate.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/language/test/delegate.test.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). (3)
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: claude-review
🔇 Additional comments (2)
packages/language/test/delegate.test.ts (1)
118-166: Nice regression coverage for delegate relation opposites.
The two-step schema (error then no-error) matches the validator behavior change and should prevent this from regressing.packages/language/src/validators/datamodel-validator.ts (1)
247-256: Good: opposite-relation check is now context-aware (but relies on$containersemantics).
PassingcontextModelintoisFieldInheritedFromDelegateModelseems to match the intended behavior change (delegate model’s own relation fields now require opposites; inherited ones don’t). Please double-check thatgetAllFields(dm)returns inheritedDataFields with$containerstill pointing at the base model (not the derived model), otherwise this condition can silently misclassify fields.#!/bin/bash # Verify how getAllFields constructs/returns inherited fields and whether it preserves DataField.$container. rg -n --type=ts "function getAllFields\\b|const getAllFields\\b|export function getAllFields\\b" -S rg -n --type=ts "getAllFields\\(" packages/language/src -S
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.