fix(orm): make orderBy nulls optional

[ymc9]

Summary

• Prisma's SortOrderInput defines nulls as optional, but ZenStack's Zod schema required it, so valid inputs like orderBy: { name: { sort: 'asc' } } were rejected by validation before reaching the dialect.
• Make nulls optional in the orderBy Zod schema.
• Update applyScalarOrderBy in the base dialect to handle the nulls-less object form by falling back to query.orderBy(fieldRef, sort).

Test plan

• Added a regression case in tests/e2e/orm/client-api/find.test.ts covering orderBy: { name: { sort: 'desc' } }.
• pnpm vitest run orm/client-api/find.test.ts — all 18 tests pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • orderBy now allows omitting the nulls option for optional scalar fields.

• Chores

  • Filter options updated: threshold is now optional and unaccent is required.
  • OpenAPI baseline updated to reflect adjusted ordering/nulls schema branches.

• Tests

  • Added test asserting sorting behavior when nulls is omitted.

[coderabbitai]

📝 Walkthrough

Walkthrough

Makes nulls optional for scalar orderBy objects in the zod schema and dialect: the schema omits nulls, the dialect falls back to field ordering when nulls is absent, updates FuzzyFilterOptions, adds an e2e test case, and reformats OpenAPI baseline ordering sections.

Changes

Optional nulls in orderBy

Layer / File(s)	Summary
Schema generation with optional nulls packages/orm/src/client/zod/factory.ts	makeOrderBySchema now emits orderBy objects where nulls is optional for scalar fields.
Dialect implementation for optional nulls packages/orm/src/client/crud/dialects/base-dialect.ts	applyScalarOrderBy keys on value.sort and applies NULLS FIRST/LAST only when value.nulls is exactly 'first' or 'last'; otherwise it orders by the field using the computed sort.
Type updates and test verification packages/orm/src/client/crud/dialects/base-dialect.ts, tests/e2e/orm/client-api/find.test.ts	FuzzyFilterOptions now has threshold?: number and unaccent: boolean; e2e test adds a case asserting orderBy: { name: { sort: 'desc' } } returns a defined result.
OpenAPI baseline reformatting packages/server/test/openapi/baseline/rpc.baseline.yaml	Reformats multiple nulls/required sections in User/Post orderBy input schema blocks (placement of const: last and required: - sort).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through schemas, tidy and spry,
Took nulls off the path and let sorts fly,
Tests gave a nod, the dialect agreed,
OpenAPI tidied each ordering seed,
A rabbit's small fix — quick, neat, and spry.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(orm): make orderBy nulls optional' accurately describes the primary change: making the nulls field optional in orderBy schemas, which aligns with the Prisma SortOrderInput definition and the main code changes throughout the PR.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/orderby-nulls-optional

---

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

---

Code Review: Make orderBy nulls optional

Tasks:

• Read and analyze changed files
• Review Zod schema changes in factory.ts
• Review dialect implementation in base-dialect.ts
• Review test coverage in find.test.ts
• Check for potential bugs or edge cases
• Provide overall feedback and recommendations

---

Analysis

This PR successfully addresses a validation mismatch between ZenStack and Prisma's SortOrderInput type. The changes are well-targeted and maintain backward compatibility.

✅ Strengths

1. Correct Problem Identification: The issue correctly identifies that Prisma's SortOrderInput makes nulls optional, but ZenStack's Zod validation was requiring it.

2. Minimal, Focused Changes:

   • factory.ts:1525: Simply adds .optional() to the nulls field in the Zod schema
   • base-dialect.ts:1244-1246: Properly handles the case where nulls is undefined by falling back to basic query.orderBy(fieldRef, sort)

3. Backward Compatibility: Existing code using { sort: 'asc', nulls: 'first' } continues to work unchanged.

4. Test Coverage: Added regression test in find.test.ts:150-155 covering the { sort: 'desc' } syntax without nulls.

🔍 Code Quality Assessment

factory.ts (packages/orm/src/client/zod/factory.ts:1525)

nulls: z.union([z.literal('first'), z.literal('last')]).optional()

• Clean, straightforward change that aligns with Prisma's type definition
• Maintains type safety while allowing optional nulls

base-dialect.ts (packages/orm/src/client/crud/dialects/base-dialect.ts:1244-1246)

if (value.nulls === 'first' || value.nulls === 'last') {
    return this.buildOrderByField(query, fieldRef, sort, value.nulls);
} else {
    return query.orderBy(fieldRef, sort);
}

• Proper defensive programming - explicitly checks for nulls presence
• Graceful fallback to standard ordering when nulls is undefined
• Uses existing, well-tested code paths

find.test.ts (tests/e2e/orm/client-api/find.test.ts:150-155)

// object sort without nulls
await expect(
    client.user.findFirst({
        orderBy: { name: { sort: 'desc' } },
    }),
).resolves.toMatchObject({ email: 'u2@test.com' });

• Tests the exact use case mentioned in the issue
• Integration test verifies end-to-end functionality

🚀 Performance Considerations

• No impact: Changes are purely structural/validation-related
• Falls back to standard Kysely orderBy() method when nulls is omitted
• No additional overhead introduced

🔒 Security Assessment

• Low risk: Changes are limited to schema validation relaxation
• No user input handling changes
• Maintains existing validation for when nulls is provided

⚠️ Potential Considerations

1. Database-Specific Behavior: When nulls is omitted, ordering behavior becomes database-dependent:

   • PostgreSQL: NULLs typically sort after non-NULLs in ASC, before in DESC
   • SQLite: NULLs sort before non-NULLs regardless of sort direction
   • This matches Prisma's behavior, so it's correct, but users should be aware

2. Documentation: Consider updating documentation to clarify that nulls is optional and explain default behavior across databases.

📊 Test Coverage Assessment

Excellent coverage for the core functionality:

• ✅ Basic { sort: 'desc' } syntax (new test)
• ✅ Full { sort: 'asc', nulls: 'first' } syntax (existing tests at lines 137-148)
• ✅ String syntax 'asc'/'desc' (existing tests)

Could be enhanced with:

• Cross-database behavior verification for null ordering
• Edge cases with computed/virtual fields

🎯 Recommendation: APPROVE

This is a well-implemented fix that:

• ✅ Solves the immediate validation issue
• ✅ Maintains full backward compatibility
• ✅ Follows established patterns in the codebase
• ✅ Includes appropriate test coverage
• ✅ Aligns ZenStack behavior with Prisma's specification

The implementation is clean, minimal, and follows defensive programming principles. The changes are exactly what's needed - no more, no less.

---

[coderabbitai]

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 `@packages/orm/src/client/crud/dialects/base-dialect.ts`:
- Around line 1240-1246: The conditional that checks sort on the incoming value
can throw when value is null; update the if-condition in the method in
BaseDialect to guard against null by adding value !== null before using 'in' and
property accesses (so the branch that calls this.negateSort(value.sort, negated)
and either this.buildOrderByField(query, fieldRef, sort, value.nulls) or
query.orderBy(fieldRef, sort) only runs when value is not null). Locate the
block that references negateSort, buildOrderByField, and query.orderBy and
modify the predicate to include value !== null while preserving existing checks
for value.sort and value.nulls.

🪄 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

Run ID: a29adcc7-1247-43d3-8f21-e9b2be235c8d

📥 Commits

Reviewing files that changed from the base of the PR and between e492c93 and 5bbfbba.

📒 Files selected for processing (3)

• packages/orm/src/client/crud/dialects/base-dialect.ts
• packages/orm/src/client/zod/factory.ts
• tests/e2e/orm/client-api/find.test.ts

[claude]

Claude finished @ymc9's task —— View job

---

Code Review: Make orderBy nulls optional

Tasks:

• Read and analyze changed files
• Review Zod schema changes in factory.ts
• Review dialect implementation in base-dialect.ts
• Review test coverage in find.test.ts
• Check for potential bugs or edge cases
• Provide overall feedback and recommendations

---

Analysis

This PR successfully addresses a validation mismatch between ZenStack and Prisma's SortOrderInput type. The changes are well-targeted and maintain backward compatibility.

✅ Strengths

1. Correct Problem Identification: The issue correctly identifies that Prisma's SortOrderInput makes nulls optional, but ZenStack's Zod validation was requiring it.

2. Minimal, Focused Changes:

   • factory.ts:1363: Simply adds .optional() to the nulls field in the Zod schema
   • base-dialect.ts:1244-1246: Properly handles the case where nulls is undefined by falling back to basic query.orderBy(fieldRef, sort)

3. Backward Compatibility: Existing code using { sort: 'asc', nulls: 'first' } continues to work unchanged.

4. Test Coverage: Added regression test in find.test.ts:150-155 covering the { sort: 'desc' } syntax without nulls.

🔍 Code Quality Assessment

factory.ts (packages/orm/src/client/zod/factory.ts:1363)

nulls: z.union([z.literal('first'), z.literal('last')]).optional()

• Clean, straightforward change that aligns with Prisma's type definition
• Maintains type safety while allowing optional nulls

base-dialect.ts (packages/orm/src/client/crud/dialects/base-dialect.ts:1244-1246)

if (value.nulls === 'first' || value.nulls === 'last') {
    return this.buildOrderByField(query, fieldRef, sort, value.nulls);
} else {
    return query.orderBy(fieldRef, sort);
}

• Proper defensive programming - explicitly checks for nulls presence
• Graceful fallback to standard ordering when nulls is undefined
• Uses existing, well-tested code paths

find.test.ts (tests/e2e/orm/client-api/find.test.ts:150-155)

// object sort without nulls
await expect(
    client.user.findFirst({
        orderBy: { name: { sort: 'desc' } },
    }),
).resolves.toMatchObject({ email: 'u2@test.com' });

• Tests the exact use case mentioned in the issue
• Integration test verifies end-to-end functionality

🚀 Performance Considerations

• No impact: Changes are purely structural/validation-related
• Falls back to standard Kysely orderBy() method when nulls is omitted
• No additional overhead introduced

🔒 Security Assessment

• Low risk: Changes are limited to schema validation relaxation
• No user input handling changes
• Maintains existing validation for when nulls is provided

⚠️ Potential Issue Found

Critical Bug in base-dialect.ts:1240: The conditional check has a potential null pointer exception:

if (typeof value === 'object' && 'sort' in value && (value.sort === 'asc' || value.sort === 'desc')) {

Problem: typeof null === 'object' returns true, but the in operator throws a TypeError when used on null. This pattern appears throughout the codebase with proper null guards (see lines 177, 313, 1809), but line 1240 is missing the null check.

Fix needed: Add value !== null to the condition:

if (value !== null && typeof value === 'object' && 'sort' in value && (value.sort === 'asc' || value.sort === 'desc')) {

🎯 Additional Considerations

1. Database-Specific Behavior: When nulls is omitted, ordering behavior becomes database-dependent:

   • PostgreSQL: NULLs typically sort after non-NULLs in ASC, before in DESC
   • SQLite: NULLs sort before non-NULLs regardless of sort direction
   • This matches Prisma's behavior, so it's correct, but users should be aware

2. Documentation: Consider updating documentation to clarify that nulls is optional and explain default behavior across databases.

📊 Test Coverage Assessment

Excellent coverage for the core functionality:

• ✅ Basic { sort: 'desc' } syntax (new test)
• ✅ Full { sort: 'asc', nulls: 'first' } syntax (existing tests at lines 137-148)
• ✅ String syntax 'asc'/'desc' (existing tests)

Could be enhanced with:

• Cross-database behavior verification for null ordering
• Edge cases with computed/virtual fields

🎯 Recommendation: APPROVE with Minor Fix

This is a well-implemented fix that:

• ✅ Solves the immediate validation issue
• ✅ Maintains full backward compatibility
• ✅ Follows established patterns in the codebase
• ✅ Includes appropriate test coverage
• ✅ Aligns ZenStack behavior with Prisma's specification

The implementation is clean, minimal, and follows defensive programming principles. Please address the null check issue in base-dialect.ts:1240 before merging.

---

[claude]

Claude finished @ymc9's task —— View job

---

Code Review: Make orderBy nulls optional

Tasks:

• Read and analyze changed files
• Review Zod schema changes in factory.ts
• Review dialect implementation in base-dialect.ts
• Review test coverage in find.test.ts
• Check for potential bugs or edge cases
• Provide overall feedback and recommendations

---

Analysis

This PR successfully addresses a validation mismatch between ZenStack and Prisma's SortOrderInput type. The changes are well-targeted and maintain backward compatibility.

✅ Strengths

1. Correct Problem Identification: The issue correctly identifies that Prisma's SortOrderInput makes nulls optional, but ZenStack's Zod validation was requiring it.

2. Minimal, Focused Changes:

   • factory.ts:1363: Simply adds .optional() to the nulls field in the Zod schema
   • base-dialect.ts:1244-1246: Properly handles the case where nulls is undefined by falling back to basic query.orderBy(fieldRef, sort)

3. Backward Compatibility: Existing code using { sort: 'asc', nulls: 'first' } continues to work unchanged.

4. Test Coverage: Added regression test in find.test.ts:150-155 covering the { sort: 'desc' } syntax without nulls.

🔍 Code Quality Assessment

factory.ts (packages/orm/src/client/zod/factory.ts:1363)

nulls: z.union([z.literal('first'), z.literal('last')]).optional()

• Clean, straightforward change that aligns with Prisma's type definition
• Maintains type safety while allowing optional nulls

base-dialect.ts (packages/orm/src/client/crud/dialects/base-dialect.ts:1244-1246)

if (value.nulls === 'first' || value.nulls === 'last') {
    return this.buildOrderByField(query, fieldRef, sort, value.nulls);
} else {
    return query.orderBy(fieldRef, sort);
}

• Proper defensive programming - explicitly checks for nulls presence
• Graceful fallback to standard ordering when nulls is undefined
• Uses existing, well-tested code paths

find.test.ts (tests/e2e/orm/client-api/find.test.ts:150-155)

// object sort without nulls
await expect(
    client.user.findFirst({
        orderBy: { name: { sort: 'desc' } },
    }),
).resolves.toBeDefined();

• Tests the exact use case mentioned in the issue
• Integration test verifies end-to-end functionality

🚀 Performance Considerations

• No impact: Changes are purely structural/validation-related
• Falls back to standard Kysely orderBy() method when nulls is omitted
• No additional overhead introduced

🔒 Security Assessment

• Low risk: Changes are limited to schema validation relaxation
• No user input handling changes
• Maintains existing validation for when nulls is provided

⚠️ Potential Issue Found

Critical Bug in base-dialect.ts:1240: The conditional check has a potential null pointer exception:

if (typeof value === 'object' && 'sort' in value && (value.sort === 'asc' || value.sort === 'desc')) {

Problem: typeof null === 'object' returns true, but the in operator throws a TypeError when used on null. This pattern appears throughout the codebase with proper null guards (see lines 313, 1399, 1408, 1809), but line 1240 is missing the null check.

Fix needed: Add value !== null to the condition:

if (value !== null && typeof value === 'object' && 'sort' in value && (value.sort === 'asc' || value.sort === 'desc')) {

🎯 Additional Considerations

1. Database-Specific Behavior: When nulls is omitted, ordering behavior becomes database-dependent:

   • PostgreSQL: NULLs typically sort after non-NULLs in ASC, before in DESC
   • SQLite: NULLs sort before non-NULLs regardless of sort direction
   • This matches Prisma's behavior, so it's correct, but users should be aware

2. Documentation: Consider updating documentation to clarify that nulls is optional and explain default behavior across databases.

📊 Test Coverage Assessment

Excellent coverage for the core functionality:

• ✅ Basic { sort: 'desc' } syntax (new test)
• ✅ Full { sort: 'asc', nulls: 'first' } syntax (existing tests at lines 137-148)
• ✅ String syntax 'asc'/'desc' (existing tests)

Could be enhanced with:

• Cross-database behavior verification for null ordering
• Edge cases with computed/virtual fields

🎯 Recommendation: APPROVE with Minor Fix

This is a well-implemented fix that:

• ✅ Solves the immediate validation issue
• ✅ Maintains full backward compatibility
• ✅ Follows established patterns in the codebase
• ✅ Includes appropriate test coverage
• ✅ Aligns ZenStack behavior with Prisma's specification

The implementation is clean, minimal, and follows defensive programming principles. Please address the null check issue in base-dialect.ts:1240 before merging.

---

[coderabbitai]

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/e2e/orm/client-api/find.test.ts`:
- Around line 150-155: The test currently asserts that
client.user.findFirst({...}).resolves.toBeDefined(), which would incorrectly
pass if the result is null; replace the assertion with the stronger custom
matcher toResolveTruthy() so the promise must resolve to a truthy user. Locate
the call to client.user.findFirst with orderBy: { name: { sort: 'desc' } } and
change the expectation from .resolves.toBeDefined() to .toResolveTruthy() to
match the suite's pattern.

🪄 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

Run ID: af073876-324a-4f95-a928-a226553258bd

📥 Commits

Reviewing files that changed from the base of the PR and between 7e1a9f6 and e275f62.

📒 Files selected for processing (1)

• tests/e2e/orm/client-api/find.test.ts