fix(cli): add missing opposite relation fields during db pull when multiple FKs target the same model#2652
fix(cli): add missing opposite relation fields during db pull when multiple FKs target the same model#2652svetch wants to merge 1 commit intozenstackhq:devfrom
Conversation
…ltiple FKs target the same model Back-reference relation fields (the opposite side of a relation, with @relation but no `fields` arg) were silently skipped during the merge phase of `db pull` when no matching field existed in the original schema. This caused models like `Users` that are referenced by many tables (e.g., via `user_created`/`user_updated` FKs) to be missing their back-reference fields after pulling. The fix adds relation-name-based matching as a new step in the field matching algorithm, and removes the blanket early-skip that discarded all unmatched back-references. Named back-references that don't match any existing field are now correctly added as new fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR modifies relation name handling in the database pull functionality. It replaces ChangesRelation Name Handling Enhancement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Pull request overview
Fixes db pull merge behavior so that named opposite-side relation fields (back-references) are no longer dropped when multiple foreign keys point to the same target model, ensuring schemas are fully restored/preserved in these multi-FK scenarios.
Changes:
- Added extraction of
@relationpositional relation name (getRelationName) to enable reliable matching of back-reference fields. - Updated the field matching priority during
db pullmerge to include relation-name-based matching and removed the blanket skip for unmatched back-references. - Added regression tests covering restore/preserve behavior when multiple models reference the same target model via multiple FKs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/cli/test/db/pull.test.ts | Adds regression tests to ensure opposite relation fields are restored/preserved when multiple FKs target the same model. |
| packages/cli/src/actions/pull/utils.ts | Introduces getRelationName helper to read the first positional argument of @relation. |
| packages/cli/src/actions/db.ts | Extends merge matching logic to use relation-name matching and allows adding previously skipped named back-references. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/cli/test/db/pull.test.ts (1)
195-230: 💤 Low valueTest placement: "preserve" test sits under "Pull from zero" describe.
This test does not zero out the schema between push and pull, so semantically it belongs to the "Pull with existing schema - preserve schema features" describe block (Line 330) rather than "Pull from zero - restore complete schema from database" (Line 11). The first new test (Lines 155-193) is correctly placed under "Pull from zero". Reorganizing improves discoverability and aligns with the existing pattern (e.g., the analogous "preserve" test for self-referencing models at Lines 132-153 has the same minor placement issue, so this is consistent — but worth fixing for both).
🤖 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 `@packages/cli/test/db/pull.test.ts` around lines 195 - 230, Move the "should preserve opposite relation fields when multiple models have FKs to the same target" test out of the "Pull from zero" describe and into the "Pull with existing schema - preserve schema features" describe block (i.e., relocate the entire it(...) block so it runs alongside the other "preserve" tests); ensure you also relocate the analogous self-referencing "preserve" test if present so both preservation tests live under the same describe, keeping the test name and behavior unchanged (look for the it(...) with that exact description and the matching test that checks self-referencing models).packages/cli/src/actions/pull/utils.ts (1)
131-137: 💤 Low valueMinor: redundant
as StringLiteralcast.The check on Line 135 already narrows
firstPositionalArg.valuetoStringLiteral, so the cast on Line 136 is unnecessary. Not a functional issue.Optional cleanup
export function getRelationName(decl: DataField): string | undefined { const relationAttr = decl?.attributes?.find((a) => a.decl?.ref?.name === '@relation'); if (!relationAttr) return undefined; const firstPositionalArg = relationAttr.args.find((a) => !a.name); if (!firstPositionalArg || firstPositionalArg.value?.$type !== 'StringLiteral') return undefined; - return (firstPositionalArg.value as StringLiteral).value; + return firstPositionalArg.value.value; }🤖 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 `@packages/cli/src/actions/pull/utils.ts` around lines 131 - 137, In getRelationName, remove the redundant type cast "as StringLiteral" on the return line: after you already checked firstPositionalArg.value?.$type === 'StringLiteral', directly access and return firstPositionalArg.value.value (or assign firstPositionalArg.value to a const typed variable for clarity) so the explicit cast is unnecessary; update the return accordingly in the getRelationName function.
🤖 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.
Nitpick comments:
In `@packages/cli/src/actions/pull/utils.ts`:
- Around line 131-137: In getRelationName, remove the redundant type cast "as
StringLiteral" on the return line: after you already checked
firstPositionalArg.value?.$type === 'StringLiteral', directly access and return
firstPositionalArg.value.value (or assign firstPositionalArg.value to a const
typed variable for clarity) so the explicit cast is unnecessary; update the
return accordingly in the getRelationName function.
In `@packages/cli/test/db/pull.test.ts`:
- Around line 195-230: Move the "should preserve opposite relation fields when
multiple models have FKs to the same target" test out of the "Pull from zero"
describe and into the "Pull with existing schema - preserve schema features"
describe block (i.e., relocate the entire it(...) block so it runs alongside the
other "preserve" tests); ensure you also relocate the analogous self-referencing
"preserve" test if present so both preservation tests live under the same
describe, keeping the test name and behavior unchanged (look for the it(...)
with that exact description and the matching test that checks self-referencing
models).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dbfb51ad-0e00-4e56-b68a-78e5be718f3b
📒 Files selected for processing (3)
packages/cli/src/actions/db.tspackages/cli/src/actions/pull/utils.tspackages/cli/test/db/pull.test.ts
Back-reference relation fields (the opposite side of a relation, with @relation but no
fieldsarg) were silently skipped during the merge phase ofdb pullwhen no matching field existed in the original schema. This caused models likeUsersthat are referenced by many tables (e.g., viauser_created/user_updatedFKs) to be missing their back-reference fields after pulling.The fix adds relation-name-based matching as a new step in the field matching algorithm, and removes the blanket early-skip that discarded all unmatched back-references. Named back-references that don't match any existing field are now correctly added as new fields.
Summary by CodeRabbit
Bug Fixes
Tests