fix(orm): fix query errors with many-to-many relations between delegate models

[ymc9]

fixes #505

Summary by CodeRabbit

• Bug Fixes

  • Fixed an issue with many-to-many relationships when using model inheritance, ensuring proper resolution of join table naming, foreign key ordering, and ID field handling.

• Tests

  • Added regression test to validate many-to-many relationships work correctly with inherited models.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This pull request fixes a query execution error when handling many-to-many relations between delegate (inherited) models. The fix modifies the getManyToManyRelation function to properly derive base models using originModel and determines join table naming and foreign key ordering for inherited relations, with an accompanying regression test validating the fix.

Changes

Cohort / File(s)	Summary
M2M relation logic for inherited models packages/orm/src/client/query-utils.ts	Updates getManyToManyRelation to use originModel for deriving the base model of inherited m2m fields, then uses realModel for join table naming, FK ordering, and ID field resolution. Ensures correct handling when relations are not self-referential.
Regression test for inherited model relations tests/regression/test/issue-505.test.ts	Adds test scenario exercising many-to-many relations between delegate models (Media/TelegramPhoto and Message/TelegramMessage), validating that creating linked records via delegation and querying relationships works correctly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Logic complexity: Requires understanding how delegation/inheritance affects m2m relation resolution, join table naming conventions, and foreign key assignment.
• Focus area: The getManyToManyRelation function changes warrant careful attention to verify correct model resolution for inherited relations and impact on join table construction.
• Test coverage: Regression test provides clear validation of the specific scenario, but review should confirm it covers edge cases with delegation.

Poem

🐰 A rabbit hops through tables joined,
Where delegates once caused much pain,
But now the base models realigned,
Relations flow through inheritance's lane,
No phantom joins—the fix has reigned! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main fix: resolving query errors with many-to-many relations between delegate models.
Linked Issues check	✅ Passed	The PR directly addresses issue #505 by fixing the getManyToManyRelation logic to correctly handle inherited delegate models, resolving the join table naming and FK issues.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing many-to-many relation handling for delegate models: core logic in query-utils.ts and a regression test.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/issue-505

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe109c8 and 1357c3e.

📒 Files selected for processing (2)

• packages/orm/src/client/query-utils.ts (1 hunks)
• tests/regression/test/issue-505.test.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (7)

📓 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 packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization

📚 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-505.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 tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests

Applied to files:

• tests/regression/test/issue-505.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 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-505.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 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/query-utils.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/query-utils.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/orm/src/client/query-utils.ts

🧬 Code graph analysis (1)

tests/regression/test/issue-505.test.ts (1)

packages/testtools/src/client.ts (1)

• createTestClient (101-248)

⏰ 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, sqlite)
• GitHub Check: build-test (20.x, postgresql)

🔇 Additional comments (4)

packages/orm/src/client/query-utils.ts (2)

235-237: LGTM! Correct handling of inherited many-to-many fields.

The logic correctly derives the base model for inherited relation fields, ensuring join table naming and FK references align with the delegate base model rather than the derived model. This is essential for Prisma's convention where the join table is named after the base models.

---

245-258: LGTM! Consistent application of realModel throughout.

All references to the model for join table naming, self-relation detection, FK ordering, and ID field retrieval correctly use realModel. This ensures consistent behavior when handling inherited many-to-many relations.

tests/regression/test/issue-505.test.ts (2)

1-34: LGTM! Well-structured regression test setup.

The schema correctly reproduces the issue scenario with delegate models and a many-to-many relation between the base models. Using usePrismaPush: true ensures the full integration is tested, including Prisma schema generation and database migration.

---

36-68: LGTM! Comprehensive validation of the fix.

The test operations thoroughly validate the fix by:

1. Creating instances of derived delegate models
2. Connecting them via the inherited many-to-many relation (the operation that was failing)
3. Verifying the bidirectional relation works correctly
4. Confirming the discriminator field is properly populated

The use of toMatchObject for assertions is appropriate, allowing resilience to additional fields while ensuring the critical relationships are correct.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude finished @ymc9's task —— View job

---

PR Review: Fix for Many-to-Many Relations Between Delegate Models

I've reviewed this pull request that addresses query errors with many-to-many relations between delegate models. Here's my comprehensive feedback:

Review Tasks:

• Read and analyze the modified query-utils.ts file
• Read and analyze the new test file
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Evaluate test coverage

---

Code Quality & Best Practices ✅

Positive aspects:

• Clean, focused fix: The change is minimal and surgical, addressing exactly the issue at hand
• Good variable naming: realModel clearly indicates its purpose
• Consistent code style: Follows existing patterns in the codebase
• Proper null coalescing: Uses ?? operator appropriately for fallback logic

Suggestions:

• The inline comment at query-utils.ts:236-237 is helpful but could be more specific about why we need the base model (related to delegate inheritance and join table naming)

Bug Analysis ✅

Root cause properly addressed:

• The bug occurred because many-to-many join table names are generated based on model names, but for delegate models, the function was using the derived model name instead of the base model name
• This caused inconsistencies in join table resolution between delegate models sharing the same base

Fix validation:

• Lines 245, 247, 249, 258: All correctly changed from model to realModel
• The fallback logic fieldDef.originModel ?? model ensures backward compatibility for non-delegate models

Performance Considerations ✅

Minimal performance impact:

• The change adds only one property lookup (fieldDef.originModel)
• No additional database queries or complex computations
• The fix actually prevents potential query errors that would be more expensive to handle

Security Concerns ✅

No security issues identified:

• The change doesn't introduce any new attack vectors
• Input validation remains the same
• No exposure of sensitive data
• Follows existing error handling patterns

Test Coverage ✅

Comprehensive test scenario:
The new test in tests/regression/test/issue-505.test.ts validates:

• ✅ Creation of delegate models (TelegramPhoto extends Media, TelegramMessage extends Message)
• ✅ Many-to-many relationship creation between delegate models
• ✅ Bidirectional relationship queries (both directions of the M2M relation)
• ✅ Proper data inclusion and object matching

Test quality:

• Uses realistic schema with delegate inheritance (@@delegate(type))
• Tests both connect operation and include queries
• Validates actual data integrity with expect.objectContaining()
• Covers the exact scenario that was failing before the fix

Overall Assessment 🎯

This is a high-quality fix that:

1. Correctly identifies and solves the root cause - delegate inheritance affecting join table naming
2. Maintains backward compatibility - non-delegate models continue to work as before
3. Includes excellent test coverage - comprehensive regression test validates the fix
4. Follows best practices - minimal, focused change with clear intent

Recommendations

1. ✅ Approve this PR - the fix is solid and well-tested
2. Consider adding a brief code comment explaining the delegate inheritance context for future maintainers
3. The test coverage is comprehensive and should prevent regression

Summary: This PR successfully fixes a specific edge case with many-to-many relations in delegate model hierarchies. The implementation is clean, safe, and well-validated through testing.

---

[Copilot]

Pull request overview

This PR fixes query errors that occur when using many-to-many relations between delegate models. The issue arose because when a concrete model (e.g., TelegramMessage) inherits a many-to-many relation field from its delegate base model (e.g., Message), the system was using the concrete model name instead of the base model name when computing join table names and foreign key references, causing query failures.

Key Changes:

• Modified getManyToManyRelation function to use originModel field for inherited relation fields
• Added comprehensive regression test verifying both directions of many-to-many relations between delegate models

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
packages/orm/src/client/query-utils.ts	Updated getManyToManyRelation to resolve base model name using originModel field for inherited relation fields, ensuring correct join table name and FK ordering
tests/regression/test/issue-505.test.ts	Added regression test with many-to-many relations between delegate models (Media/TelegramPhoto and Message/TelegramMessage), verifying create, connect, and include operations in both directions

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.