Uh oh!
There was an error while loading. Please reload this page.
fix(sdk): unnamed relation resolving to the wrong opposite field - #2769
Merged
Conversation
When two relations connect the same pair of models and only one pair
carries `@relation("name")`, the unnamed pair could resolve to the named
pair's field. The generated schema then had e.g.
`Category.products.relation.opposite = "featuredIn"`, so relation filters
and nested reads traversed the wrong FK column and silently returned
wrong results.
`getOppositeRelationField` matched relation names asymmetrically: a named
field required a name match, but an unnamed field accepted the first
back-pointing field regardless of its name, making resolution depend on
field declaration order. Match names on both sides, including the case
where neither side is named - the same rule the language validator
already applies.
Regenerating the e2e schemas surfaces one real-world instance of this:
trigger.dev's `Project.workerGroups` now correctly resolves to
`WorkerInstanceGroup.project` instead of the `ProjectDefaultWorkerGroup`
relation's `defaultForProjects`.
Fixes#2757
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Contributor
📝 WalkthroughWalkthroughThe schema generator now requires symmetric relation-name matches when selecting opposite fields. The generated schema fixture is updated, and regression tests cover unnamed and named relations through nested reads and relation filters. ChangesRelation resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/regression/test/issue-2757.test.ts`:
- Around line 46-58: Extend the relation-filter regression coverage in
issue-2757 by seeding a second product with opposing category and featuredIn
values, then add category.findMany assertions for products.every and
featured.every. Ensure the expected category results validate both
every-relation paths alongside the existing some and none cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d2dee002-59eb-4269-a9de-9c0e3e92763f
📒 Files selected for processing (3)
packages/sdk/src/ts-schema-generator.tstests/e2e/github-repos/trigger.dev/schema.tstests/regression/test/issue-2757.test.ts
Comment on lines
+46
to
+58
| // relation filters traverse the right column | ||
| await expect(db.category.findMany({ where: { products: { none: {} } } })).resolves.toEqual([ | ||
| expect.objectContaining({ id: c2.id }), | ||
| ]); | ||
| await expect(db.category.findMany({ where: { products: { some: {} } } })).resolves.toEqual([ | ||
| expect.objectContaining({ id: c1.id }), | ||
| ]); | ||
| await expect(db.category.findMany({ where: { featured: { none: {} } } })).resolves.toEqual([ | ||
| expect.objectContaining({ id: c1.id }), | ||
| ]); | ||
| await expect(db.category.findMany({ where: { featured: { some: {} } } })).resolves.toEqual([ | ||
| expect.objectContaining({ id: c2.id }), | ||
| ]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover the every relation-filter path.
The stated regression scope includes every, but these assertions only exercise some and none. Seed a second product with opposing category/featuredIn values and assert every for both relations; otherwise a regression specific to every can ship undetected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/regression/test/issue-2757.test.ts` around lines 46 - 58, Extend the
relation-filter regression coverage in issue-2757 by seeding a second product
with opposing category and featuredIn values, then add category.findMany
assertions for products.every and featured.every. Ensure the expected category
results validate both every-relation paths alongside the existing some and none
cases.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#2757
Problem
When two relations connect the same pair of models and only one pair carries
@relation("name"), the unnamed pair could resolve to the named pair's field. The generated schema then had e.g.Category.products.relation.opposite = "featuredIn", so relation filters (some/none/every) and nested reads traversed the wrong FK column and silently returned wrong results — no error, types still compile. Prisma resolves the same schema correctly.category.findMany({ where: { products: { none: {} } } })joined onfeaturedInIdinstead ofcategoryId.Cause
TsSchemaGenerator.getOppositeRelationFieldmatched relation names asymmetrically — a named field required a name match, but an unnamed field returned the first back-pointing field regardless of its relation name. Resolution therefore depended on field declaration order, which is exactly the ordering sensitivity the reporter observed.Fix
Require the relation name to match on both sides, including the case where neither side is named. This is the same rule the language validator already applies when pairing relation fields (
datamodel-validator.ts), so the generator and validation now agree, and declaration order no longer matters. Schemas that name only one side are already rejected by validation, so nothing that previously validated changes meaning.Nothing else computes the opposite field — the ORM runtime only reads
relation.oppositefrom the generated schema.Verification
tests/regression/test/issue-2757.test.tscovers both declaration orders, nested reads, andsome/nonefilters on both sides. The "named relation declared first" case failed before this change.Project.workerGroupsresolved todefaultForProjects(the@relation("ProjectDefaultWorkerGroup")pair) and now correctly resolves toWorkerInstanceGroup.project, the field carryingprojectId.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests