fix(orm): select/include nullability

[ymc9]

Summary by CodeRabbit

• Refactor
  • Unified field selection handling across database operation types (Create, Update, Delete, Upsert). CRUD argument signatures have been restructured for consistency. Review documentation if you customize these operations.

[coderabbitai]

Warning

Rate limit exceeded

@ymc9 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 44 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 7b1a778 and a13ec7c.

📒 Files selected for processing (4)

• packages/orm/src/client/crud-types.ts (7 hunks)
• packages/orm/src/client/crud/operations/create.ts (1 hunks)
• packages/orm/src/client/crud/operations/update.ts (1 hunks)
• packages/orm/src/client/crud/validator/index.ts (7 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

A single file refactoring consolidates scattered inline select, include, and omit field declarations across multiple CRUD operation argument types (Create, Update, Upsert, Delete, and their Many/Return variants) into a unified SelectIncludeOmit type wrapper for centralized semantics handling.

Changes

Cohort / File(s)	Change Summary
CRUD Type Consolidation packages/orm/src/client/crud-types.ts	Replaced inline select, include, omit declarations in CreateArgs, UpdateArgs, UpsertArgs, and DeleteArgs with & SelectIncludeOmit<Schema, Model, true>. Refactored CreateManyAndReturnArgs and UpdateManyAndReturnArgs to use Omit<SelectIncludeOmit<Schema, Model, false>, 'include'> to exclude include while preserving select and omit.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify that SelectIncludeOmit type definition adequately handles all combinations of selection/inclusion/omission across different argument types
• Confirm the true/false parameter distinction between standard CRUD operations and Many/Return variants is semantically correct
• Ensure Omit<SelectIncludeOmit<..., false>, 'include'> correctly excludes the include property while preserving expected behavior for select and omit
• Check for any breaking changes to exported type signatures that might affect downstream consumers

Possibly related PRs

• fix(orm): allow null for select/include/omit #402: Modifies the same SelectIncludeOmit type definition and its usage in packages/orm/src/client/crud-types.ts to widen the wrapper and update validators.
• refactor(orm): simplify ORM query args #396: Refactors the same CRUD argument type signatures in packages/orm/src/client/crud-types.ts to add a Simplify flag and SimplifiedModelResult.

Poem

🐰 Select, include, omit scattered wide,
Now unified with wrapper pride!
One SelectIncludeOmit to guide,
Less duplication, cleaner inside. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title references 'select/include nullability' but the changes involve refactoring these selectors into a unified SelectIncludeOmit wrapper, not addressing nullability issues.	Update the title to reflect the actual change, such as 'refactor(orm): unify select/include/omit handling' or 'refactor(orm): consolidate selection type definitions'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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(orm): select/include nullability

Review Tasks:

• Read and analyze the modified file
• Check code quality and best practices
• Identify potential bugs or issues
• Review performance considerations
• Check security concerns
• Assess test coverage needs
• Provide final review feedback

---

Summary

This PR refactors type definitions in packages/orm/src/client/crud-types.ts:698 to use the centralized SelectIncludeOmit type instead of inline duplicate type definitions. This is a follow-up to PR #402 that added null support to select/include/omit fields.

Code Quality ✅

Excellent refactoring that follows DRY principles:

• Eliminates 24 lines of duplicate code by reusing the SelectIncludeOmit type
• Consolidates type definitions that were previously scattered across multiple CRUD operation types
• Maintains consistent nullability handling across all operations

Type Safety ✅

The changes preserve type safety while improving consistency:

• SelectIncludeOmit<Schema, Model, true> correctly allows null values for select/include/omit
• SelectIncludeOmit<Schema, Model, false> properly restricts the type for operations that don't allow counts
• The Omit<..., 'include'> for CreateManyAndReturnArgs and UpdateManyAndReturnArgs is appropriate since these operations don't support includes

Performance ✅

No performance impact:

• Type-level changes only - no runtime performance implications
• May slightly improve TypeScript compilation speed by reducing duplicate type computations

Security ✅

No security concerns identified.

Test Coverage Assessment ⚠️

Recommendation: Verify test coverage includes null handling

• While the refactoring preserves existing behavior, the previous commit (fix(orm): allow null for select/include/omit #402) added null support
• Ensure integration tests verify that select: null, include: null, and omit: null work correctly across all affected operations:
  • CreateArgs, UpdateArgs, UpsertArgs, DeleteArgs
  • CreateManyAndReturnArgs, UpdateManyAndReturnArgs

Minor Suggestions

1. Consider adding a comment explaining why include is omitted from the "AndReturn" operations:

   // CreateManyAndReturn doesn't support include operations, only select/omit
   > = SimplifyIf<CreateManyInput<Schema, Model> & Omit<SelectIncludeOmit<Schema, Model, false>, 'include'>, Simplify>;

Verdict: ✅ APPROVED

This is a well-executed refactoring that:

• ✅ Reduces code duplication significantly
• ✅ Improves maintainability by centralizing type definitions
• ✅ Preserves existing type safety and behavior
• ✅ Follows established patterns in the codebase
• ✅ Has no breaking changes

The code is ready to merge after ensuring adequate test coverage for the null handling feature introduced in the previous commit.

---

[Copilot]

Pull Request Overview

This PR refactors the ORM type definitions to use a unified SelectIncludeOmit helper type and makes the select, include, and omit fields explicitly nullable by adding | null to their type definitions. This change affects mutation operations (Create, Update, Upsert, Delete) and their variants.

Key changes:

• Introduction of SelectIncludeOmit helper type that defines select, include, and omit as nullable (| null)
• Refactored CreateArgs, UpdateArgs, UpsertArgs, and DeleteArgs to use the SelectIncludeOmit helper
• Refactored CreateManyAndReturnArgs and UpdateManyAndReturnArgs to use SelectIncludeOmit with the include property omitted

---

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

[claude]

Claude finished @ymc9's task —— View job

---

PR Review: fix(orm): select/include nullability

Review Tasks:

• Read and analyze the modified file
• Check code quality and best practices
• Identify potential bugs or issues
• Review performance considerations
• Check security concerns
• Assess test coverage needs
• Provide final review feedback

---

Summary

This PR refactors type definitions in packages/orm/src/client/crud-types.ts:698 to use the centralized SelectIncludeOmit type instead of inline duplicate type definitions. This is a follow-up to PR #402 that added null support to select/include/omit fields.

Code Quality ✅

Excellent refactoring that follows DRY principles:

• Eliminates 24 lines of duplicate code by reusing the SelectIncludeOmit type
• Consolidates type definitions that were previously scattered across multiple CRUD operation types
• Maintains consistent nullability handling across all operations

Type Safety ✅

The changes preserve type safety while improving consistency:

• SelectIncludeOmit<Schema, Model, true> correctly allows null values for select/include/omit
• SelectIncludeOmit<Schema, Model, false> properly restricts the type for operations that don't allow counts
• The Omit<..., 'include'> for CreateManyAndReturnArgs and UpdateManyAndReturnArgs is appropriate since these operations don't support includes

Performance ✅

No performance impact:

• Type-level changes only - no runtime performance implications
• May slightly improve TypeScript compilation speed by reducing duplicate type computations

Security ✅

No security concerns identified.

Critical Issues ❌

Validator Schema Inconsistency - Runtime Failures Expected:

The Copilot review correctly identified a serious runtime issue. The type definitions now allow null values for select, include, and omit fields via SelectIncludeOmit, but the validator schemas only mark these fields as .optional() without .nullable():

1. CreateArgs (packages/orm/src/client/crud/validator/index.ts:867-869): Missing .nullable()
2. UpdateArgs (packages/orm/src/client/crud/validator/index.ts:1145-1147): Missing .nullable()
3. UpsertArgs (packages/orm/src/client/crud/validator/index.ts:1177-1179): Missing .nullable()
4. DeleteArgs (packages/orm/src/client/crud/validator/index.ts:1294-1295): Missing .nullable() AND missing omit field entirely
5. CreateManyAndReturnArgs (packages/orm/src/client/crud/validator/index.ts:883-884): Missing .nullable()
6. UpdateManyAndReturnArgs (packages/orm/src/client/crud/validator/index.ts:1165-1166): Missing .nullable()

Test Coverage Assessment ⚠️

Recommendation: Verify test coverage includes null handling

• While the refactoring preserves existing behavior, the previous commit (fix(orm): allow null for select/include/omit #402) added null support
• Ensure integration tests verify that select: null, include: null, and omit: null work correctly across all affected operations:
  • CreateArgs, UpdateArgs, UpsertArgs, DeleteArgs
  • CreateManyAndReturnArgs, UpdateManyAndReturnArgs

Minor Suggestions

1. Consider adding a comment explaining why include is omitted from the "AndReturn" operations:

   // CreateManyAndReturn doesn't support include operations, only select/omit
   > = SimplifyIf<CreateManyInput<Schema, Model> & Omit<SelectIncludeOmit<Schema, Model, false>, 'include'>, Simplify>;

Verdict: ❌ NEEDS WORK

While this is a well-executed refactoring that improves code organization, the validator schema inconsistencies will cause runtime failures. Users will be unable to pass null values for select/include/omit fields despite the TypeScript types allowing it.

Required fixes before merge:

1. Update all validator schemas to use .optional().nullable() for select/include/omit fields
2. Add missing omit field to DeleteArgs validator schema
3. Add integration tests for null value handling

The code quality is excellent, but the runtime functionality is broken due to validator mismatches.

---

[claude]

Claude finished @ymc9's task —— View job

---

PR Review: fix(orm): select/include nullability

Review Tasks:

• Read and analyze the modified files
• Check code quality and best practices
• Identify potential bugs or issues
• Review performance considerations
• Check security concerns
• Assess test coverage needs
• Provide final review feedback

---

Summary

This PR refactors type definitions in packages/orm/src/client/crud-types.ts:698 to use the centralized SelectIncludeOmit type instead of inline duplicate type definitions. This is a follow-up to PR #402 that added null support to select/include/omit fields.

Code Quality ✅

Excellent refactoring that follows DRY principles:

• Eliminates 24 lines of duplicate code by reusing the SelectIncludeOmit type
• Consolidates type definitions that were previously scattered across multiple CRUD operation types
• Maintains consistent nullability handling across all operations

Type Safety ✅

The changes preserve type safety while improving consistency:

• SelectIncludeOmit<Schema, Model, true> correctly allows null values for select/include/omit
• SelectIncludeOmit<Schema, Model, false> properly restricts the type for operations that don't allow counts
• The Omit<..., 'include'> for CreateManyAndReturnArgs and UpdateManyAndReturnArgs is appropriate since these operations don't support includes

Performance ✅

No performance impact:

• Type-level changes only - no runtime performance implications
• May slightly improve TypeScript compilation speed by reducing duplicate type computations

Security ✅

No security concerns identified.

Critical Issues ❌

Validator Schema Inconsistency - Runtime Failures Expected:

The Copilot review correctly identified a serious runtime issue. The type definitions now allow null values for select, include, and omit fields via SelectIncludeOmit, but the validator schemas only mark these fields as .optional() without .nullable():

1. CreateArgs (packages/orm/src/client/crud/validator/index.ts:867-869): Missing .nullable()
2. UpdateArgs (packages/orm/src/client/crud/validator/index.ts:1145-1147): Missing .nullable()
3. UpsertArgs (packages/orm/src/client/crud/validator/index.ts:1177-1179): Missing .nullable()
4. DeleteArgs (packages/orm/src/client/crud/validator/index.ts:1294-1295): Missing .nullable() AND missing omit field entirely
5. CreateManyAndReturnArgs (packages/orm/src/client/crud/validator/index.ts:883-884): Missing .nullable()
6. UpdateManyAndReturnArgs (packages/orm/src/client/crud/validator/index.ts:1165-1166): Missing .nullable()

Test Coverage Assessment ⚠️

Recommendation: Verify test coverage includes null handling

• While the refactoring preserves existing behavior, the previous commit (fix(orm): allow null for select/include/omit #402) added null support
• Ensure integration tests verify that select: null, include: null, and omit: null work correctly across all affected operations:
  • CreateArgs, UpdateArgs, UpsertArgs, DeleteArgs
  • CreateManyAndReturnArgs, UpdateManyAndReturnArgs

Minor Suggestions

1. Consider adding a comment explaining why include is omitted from the "AndReturn" operations:

   // CreateManyAndReturn doesn't support include operations, only select/omit
   > = SimplifyIf<CreateManyInput<Schema, Model> & Omit<SelectIncludeOmit<Schema, Model, false>, 'include'>, Simplify>;

Verdict: ❌ NEEDS WORK

While this is a well-executed refactoring that improves code organization, the validator schema inconsistencies will cause runtime failures. Users will be unable to pass null values for select/include/omit fields despite the TypeScript types allowing it.

Required fixes before merge:

1. Update all validator schemas to use .optional().nullable() for select/include/omit fields
2. Add missing omit field to DeleteArgs validator schema
3. Add integration tests for null value handling

The code quality is excellent, but the runtime functionality is broken due to validator mismatches.

---

[claude]

Claude finished @ymc9's task —— View job

---

PR Review: fix(orm): select/include nullability

Review Tasks:

• Read and analyze the modified files
• Check code quality and best practices
• Identify potential bugs or issues
• Review performance considerations
• Check security concerns
• Assess test coverage needs
• Provide final review feedback

---

Summary

This PR refactors type definitions in packages/orm/src/client/crud-types.ts:698 to use the centralized SelectIncludeOmit type instead of inline duplicate type definitions. This is a follow-up to PR #402 that added null support to select/include/omit fields.

Code Quality ✅

Excellent refactoring that follows DRY principles:

• Eliminates 24 lines of duplicate code by reusing the SelectIncludeOmit type
• Consolidates type definitions that were previously scattered across multiple CRUD operation types
• Maintains consistent nullability handling across all operations

Type Safety ✅

The changes preserve type safety while improving consistency:

• SelectIncludeOmit<Schema, Model, true> correctly allows null values for select/include/omit
• SelectIncludeOmit<Schema, Model, false> properly restricts the type for operations that don't allow counts
• The Omit<..., 'include'> for CreateManyAndReturnArgs and UpdateManyAndReturnArgs is appropriate since these operations don't support includes

Performance ✅

No performance impact:

• Type-level changes only - no runtime performance implications
• May slightly improve TypeScript compilation speed by reducing duplicate type computations

Security ✅

No security concerns identified.

Critical Issues ❌

Validator Schema Inconsistency - Runtime Failures Expected:

The Copilot review correctly identified a serious runtime issue. The type definitions now allow null values for select, include, and omit fields via SelectIncludeOmit, but the validator schemas only mark these fields as .optional() without .nullable():

1. CreateArgs (packages/orm/src/client/crud/validator/index.ts:876-878): Missing .nullable()
2. UpdateArgs (packages/orm/src/client/crud/validator/index.ts:1154-1156): Missing .nullable()
3. UpsertArgs (packages/orm/src/client/crud/validator/index.ts:1186-1188): Missing .nullable()
4. DeleteArgs (packages/orm/src/client/crud/validator/index.ts:1303-1304): Missing .nullable() AND missing omit field entirely
5. CreateManyAndReturnArgs (packages/orm/src/client/crud/validator/index.ts:892-893): Missing .nullable()
6. UpdateManyAndReturnArgs (packages/orm/src/client/crud/validator/index.ts:1174-1175): Missing .nullable()

Test Coverage Assessment ⚠️

Recommendation: Verify test coverage includes null handling

• While the refactoring preserves existing behavior, the previous commit (fix(orm): allow null for select/include/omit #402) added null support
• Ensure integration tests verify that select: null, include: null, and omit: null work correctly across all affected operations:
  • CreateArgs, UpdateArgs, UpsertArgs, DeleteArgs
  • CreateManyAndReturnArgs, UpdateManyAndReturnArgs

Minor Suggestions

1. Consider adding a comment explaining why include is omitted from the "AndReturn" operations:

   // CreateManyAndReturn doesn't support include operations, only select/omit
   > = SimplifyIf<CreateManyInput<Schema, Model> & Omit<SelectIncludeOmit<Schema, Model, false>, 'include'>, Simplify>;

Verdict: ❌ NEEDS WORK

While this is a well-executed refactoring that improves code organization, the validator schema inconsistencies will cause runtime failures. Users will be unable to pass null values for select/include/omit fields despite the TypeScript types allowing it.

Required fixes before merge:

1. Update all validator schemas to use .optional().nullable() for select/include/omit fields
2. Add missing omit field to DeleteArgs validator schema
3. Add integration tests for null value handling

The code quality is excellent, but the runtime functionality is broken due to validator mismatches.

---