Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Nov 12, 2025

Summary by CodeRabbit

  • New Features

    • Enum name mapping for database fields and mapped enum value display
    • Enums support field-level attributes and per-variant mappings
  • Refactoring

    • Enum schema shape changed: enum members are now nested under a values property
    • Test client API simplified: schemaFile moved into options
    • Public constants updated to reference enum values collection
  • Bug Fixes

    • Improved enum validation and input handling
  • Chores

    • Renamed public schema attribute parameter from name to map

Copilot AI review requested due to automatic review settings November 12, 2025 06:38
@coderabbitai
Copy link

coderabbitai bot commented Nov 12, 2025

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

Restructures enum shape from flat mappings to objects with values, optional fields, and attributes; updates codegen, runtime enum handling, name-mapping, DB enum push, and tests. Also renames @@schema(_ name: String)@@schema(_ map: String) in stdlib.zmodel and consolidates test client schemaFile into options.

Changes

Cohort / File(s) Summary
Core Schema & Stdlib
packages/schema/src/schema.ts, packages/language/res/stdlib.zmodel
Introduces EnumField and new EnumDef shape (values, optional fields, optional attributes); tightens GetEnum to return .values. Renames attribute parameter: @@schema(_ name: String)@@schema(_ map: String).
SDK Codegen
packages/sdk/src/ts-schema-generator.ts
Emits enums as structured objects with a values map, optional fields objects (when field attributes exist), and optional enum-level attributes; internal access paths updated to use .values.
ORM Runtime & Validation
packages/orm/src/client/crud/validator/index.ts, packages/orm/src/client/executor/name-mapper.ts, packages/orm/src/client/helpers/schema-db-pusher.ts
Adds enum-aware validation (makeEnumSchema), uses getTypeDef accessor, switches some schemas to loose object, implements enum-aware name mapping for selects/inserts/updates, and builds Postgres enum values from enumDef.fields with @map handling.
Test Client API
packages/testtools/src/client.ts
Adds schemaFile?: string to CreateTestClientOptions and removes schemaFile as a separate positional parameter from createTestClient overloads and implementation.
Tests — Name Mapping & Client API
tests/e2e/orm/client-api/delegate.test.ts, tests/e2e/orm/client-api/name-mapping.test.ts
Update createTestClient call sites to use options.schemaFile; extend name-mapping tests to assert enum mapping behavior across CRUD, aggregations, and group-by.
Generated/Sample Schema & Models
samples/orm/zenstack/schema.ts, samples/orm/zenstack/models.ts, tests/e2e/orm/schemas/*/schema.ts, tests/e2e/orm/schemas/*/models.ts
Change generated enum definitions and usages to the new .values-based shape and update exported constants to reference .values. Includes name-mapping schema additions (enum Role with per-value @map, User.role field).
Misc Tests/Imports
tests/e2e/orm/query-builder/query-builder.test.ts
Minor import reorder / cleanup.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

  • Attention points:
    • packages/orm/src/client/executor/name-mapper.ts — complex enum-aware mapping across many query paths; verify alias/scope correctness and that case expressions preserve semantics.
    • packages/orm/src/client/crud/validator/index.ts — ensure enum validation and loose-object change do not widen validation unintentionally.
    • packages/schema/src/schema.ts & packages/sdk/src/ts-schema-generator.ts — confirm generated types and emitted accessors align and that downstream consumers reference .values consistently.
    • Test changes that migrate createTestClient usage and extensive name-mapping assertions.

Possibly related PRs

Poem

🐰
Enums tucked into values, snug and neat,
Fields get maps, names and defaults meet.
I hop through tests and map each role with care,
A dotted path of values everywhere.
Hooray — carrots for the build, and code that’s fair! 🥕

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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the primary change: adding enum name mapping support. The secondary mention of postgres schema fixes is referenced but is a minor aspect of the broader change.
✨ 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 feat/enum-mapping

📜 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 1be90f8 and 66ae12a.

📒 Files selected for processing (2)
  • packages/orm/src/client/helpers/schema-db-pusher.ts (1 hunks)
  • tests/e2e/orm/query-builder/query-builder.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/orm/src/client/helpers/schema-db-pusher.ts
⏰ 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). (3)
  • GitHub Check: claude-review
  • GitHub Check: build-test (20.x, postgresql)
  • GitHub Check: build-test (20.x, sqlite)
🔇 Additional comments (1)
tests/e2e/orm/query-builder/query-builder.test.ts (1)

2-2: LGTM! Clean import organization.

The import reordering improves code organization by removing duplication and grouping related imports together.


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

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

@claude
Copy link

claude bot commented Nov 12, 2025

Claude finished @ymc9's task —— View job


Code Review for PR #391

I've completed a comprehensive review of this pull request focusing on code quality, best practices, potential bugs, performance considerations, security concerns, and test coverage.

Review Tasks:

  • Analyze core validation changes in packages/orm/src/client/crud/validator/index.ts
  • Review name mapping enhancements in packages/orm/src/client/executor/name-mapper.ts
  • Examine schema generation updates in packages/sdk/src/ts-schema-generator.ts
  • Check test coverage and implementation
  • Review other supporting file changes
  • Provide comprehensive feedback and recommendations

Overall Assessment ⭐⭐⭐⭐⭐

This is a high-quality pull request that implements enum name mapping and stricter schema validation with excellent attention to detail. The code demonstrates strong software engineering practices and comprehensive testing.

Key Strengths

🏗️ Architecture & Design

  • Excellent separation of concerns: Clear division between validation, name mapping, and schema generation
  • Robust caching strategy: WeakMap-based schema caching in validator prevents memory leaks
  • Comprehensive enum mapping: Support for both field name mapping (@map) and enum value mapping (@map)
  • Backwards compatibility: Unmapped enum values pass through unchanged

🔍 Code Quality

  • Type safety: Excellent use of TypeScript with proper type guards and assertions
  • Error handling: Comprehensive error messages with context (file:line references)
  • Code organization: Well-structured classes with logical method grouping
  • Documentation: Good inline comments explaining complex logic

🧪 Testing Excellence

  • Comprehensive test coverage: Extensive tests covering CRUD operations, aggregate functions, groupBy
  • Real-world scenarios: Tests both ORM and raw Kysely query operations
  • Edge case coverage: Tests for mapped/unmapped enum values, complex joins, sub-queries

Technical Review

Validator Enhancements (packages/orm/src/client/crud/validator/index.ts)

Strengths:

  • Smart caching with stable stringification for cache keys
  • Proper handling of enum validation with name mappings
  • Excellent error contextualization with model information

Minor Suggestions:

  • Consider extracting complex validation logic into smaller helper methods for improved maintainability
  • The makeEnumFilterSchema method could benefit from early returns for readability

Name Mapping Engine (packages/orm/src/client/executor/name-mapper.ts)

Strengths:

  • Sophisticated scope management: Proper handling of nested queries and joins
  • Robust SQL generation: CASE-WHEN for enum mappings with proper type casting
  • Performance optimization: Selective expansion of SELECT * only when needed
  • Comprehensive mapping: Handles table names, column names, and enum values

Notable Implementation:

// Smart enum value mapping with CASE-WHEN
const finalExpr = caseWhen!.else(eb.cast(new ExpressionWrapper(node), 'text')).end();

This handles PostgreSQL's type inference issues elegantly.

Schema Generation (packages/sdk/src/ts-schema-generator.ts)

Strengths:

  • Conditional import generation: Only imports what's needed based on schema features
  • Memory-efficient: Tracks usage flags to minimize bundle size
  • Comprehensive type generation: Proper handling of computed fields, enums, and type definitions

Test Coverage

Comprehensive scenarios tested:

  • Basic CRUD with name mapping
  • Complex queries with joins and sub-selects
  • Aggregate functions and groupBy operations
  • Both mapped and unmapped enum values
  • Raw SQL queries alongside ORM operations

Security & Performance

🔒 Security

  • SQL injection protection: Proper use of parameterized queries
  • Input validation: Comprehensive Zod schema validation
  • No exposed internals: Clean API boundaries

Performance

  • Efficient caching: WeakMap usage prevents memory leaks
  • Minimal overhead: Name mapping only applied when needed
  • Query optimization: Smart SELECT * expansion only when mappings exist

Minor Recommendations

  1. Code Organization: Consider extracting the complex enum mapping logic in name-mapper.ts:674-712 into a separate utility class

  2. Error Messages: Add more context to enum mapping errors to help developers debug mapping issues

  3. Documentation: Consider adding JSDoc comments for the public methods in QueryNameMapper

  4. Type Safety: The getMappedValue method could use more specific return types instead of a large union type

Files Examined

  • packages/orm/src/client/crud/validator/index.ts - Enhanced validation with enum support
  • packages/orm/src/client/executor/name-mapper.ts - Comprehensive name mapping engine
  • packages/sdk/src/ts-schema-generator.ts - Schema generation with conditional imports
  • tests/e2e/orm/client-api/name-mapping.test.ts - Comprehensive test coverage
  • packages/language/src/validators/datasource-validator.ts - Schema validation improvements
  • packages/sdk/src/prisma/prisma-schema-generator.ts - Prisma schema compatibility

Conclusion

This PR represents excellent engineering work with:

  • ✅ Robust implementation with comprehensive error handling
  • ✅ Excellent test coverage across multiple scenarios
  • ✅ Clean, maintainable code with proper abstractions
  • ✅ Strong attention to performance and memory management
  • ✅ Backwards compatibility preservation

Recommendation: APPROVE

The implementation is production-ready and demonstrates high-quality software engineering practices. The comprehensive test suite provides confidence in the feature's reliability.


@claude
Copy link

claude bot commented Nov 12, 2025

Claude finished @ymc9's task —— View job


Code Review Complete ✅

Reviewing PR #391: fix: stricter default schema validation and proper transformation to Prisma schema

Todo List

  • Analyze core implementation changes
  • Review enum mapping functionality
  • Check validation logic changes
  • Assess test coverage
  • Review for potential bugs and security issues
  • Evaluate performance considerations
  • Provide comprehensive feedback

📋 Summary

This PR implements enum name mapping functionality and improves schema validation. The main changes include:

  1. Enum value mapping support - Allows @map attributes on enum values to customize database storage
  2. Enhanced name mapping - Comprehensive handling of field/table name mapping in queries
  3. Improved validation - Stricter schema validation with better error messages
  4. Comprehensive test coverage - Extensive tests for the new enum mapping features

✅ Code Quality

Strengths:

  • Well-structured implementation: Clear separation between name mapping logic and enum handling
  • Comprehensive test coverage: Extensive test suite covering CRUD operations, aggregations, and edge cases
  • Good error handling: Proper invariants and validation throughout the codebase
  • Performance conscious: Uses caching for schema validation to avoid redundant work

🔍 Detailed Analysis

Core Changes

packages/orm/src/client/executor/name-mapper.ts (+279/-42):

  • Major enhancement to QueryNameMapper class with enum value mapping support
  • Added comprehensive enum value transformation logic for both input and output
  • Proper handling of scope resolution and alias management
  • Uses CASE-WHEN expressions for enum value mapping in select queries

packages/orm/src/client/crud/validator/index.ts (+38/-21):

  • Enhanced enum validation logic in makeEnumSchema
  • Improved error handling for enum fields
  • Better integration with name mapping functionality

packages/sdk/src/ts-schema-generator.ts (+69/-7):

  • Extended schema generation to include enum field attributes
  • Proper handling of @map attributes on enum values
  • Enhanced enum object creation with field-level attributes

🔒 Security Considerations

✅ No Security Issues Found

  • No direct security vulnerabilities identified
  • Proper input validation and sanitization
  • No exposure of sensitive information in error messages
  • Uses parameterized queries through Kysely (SQL injection protection)

🚀 Performance Considerations

Optimizations:

  • Schema caching: schemaCache WeakMap prevents redundant validation schema creation
  • Lazy evaluation: Uses TypeScript lazy types for recursive schema definitions
  • Efficient enum mapping: Only applies CASE-WHEN transformations when enum mappings exist

Potential Concerns:

  • CASE-WHEN expressions for enum mapping add SQL complexity, but necessary for correct mapping
  • Large enum sets with many mapped values could result in complex SQL expressions

🧪 Test Coverage Assessment

Excellent Coverage:

  • tests/e2e/orm/client-api/name-mapping.test.ts provides comprehensive testing
  • Tests cover all CRUD operations with enum mapping
  • Includes edge cases like mixed mapped/unmapped enum values
  • Tests both ORM-level and raw Kysely query operations
  • Validates correct database storage vs. application-level representation

Test Scenarios Covered:

  • Create with enum defaults and explicit values
  • Find operations with enum filtering
  • Update with enum value changes
  • Delete operations
  • Count and aggregate operations with enums
  • GroupBy operations with enum fields

🐛 Potential Issues

Minor Concerns:

  1. name-mapper.ts:707: The explicit cast to 'text' for PostgreSQL CASE-WHEN expressions is a workaround for type inference issues. This is documented and appears to be the correct approach.

  2. Enum validation edge case: In makeEnumSchema, empty enum values result in z.never() schema, which might be too restrictive. Consider if this should allow null/undefined for optional fields.

  3. Performance with complex mappings: The CASE-WHEN approach for enum mapping could become unwieldy with very large enums (>50 values), though this is an edge case.

🔧 Recommendations

Code Quality:

  1. Good: Proper use of TypeScript for type safety
  2. Good: Comprehensive error handling with invariants
  3. Good: Clear separation of concerns between mapping and validation

Documentation:

  1. Consider: Adding JSDoc comments for complex enum mapping logic in name-mapper.ts
  2. Consider: Document the PostgreSQL text casting workaround

Testing:

  1. Excellent: Test coverage is comprehensive and well-structured
  2. Suggestion: Consider adding performance tests for large enum sets

🎯 Verdict

✅ Approve - This is a well-implemented feature with:

  • Solid architecture and clean code
  • Comprehensive test coverage
  • No security vulnerabilities
  • Good performance characteristics
  • Proper handling of edge cases

The enum mapping functionality is correctly implemented and the validation improvements enhance the robustness of the schema handling. The extensive test suite provides confidence in the implementation's correctness.


Copilot finished reviewing on behalf of ymc9 November 12, 2025 06:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the enum schema structure to be stricter and properly align with Prisma schema requirements. The key change is that enums now have a nested values property (and optionally fields and attributes properties for metadata), instead of directly containing the enum values at the top level.

  • Restructured enum definitions to use a nested values object instead of flat structure
  • Added support for enum field attributes and enum-level attributes (@Map)
  • Updated enum value mapping logic to handle database-level name transformations

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/schema/src/schema.ts Updated EnumDef type definition to include values, fields, and attributes properties
packages/sdk/src/ts-schema-generator.ts Modified enum code generation to create nested structure with values, fields (when attributes exist), and enum attributes
packages/orm/src/client/crud/validator/index.ts Updated validator to access enumDef.values instead of enumDef directly and added enum validation support
packages/orm/src/client/executor/name-mapper.ts Added comprehensive enum value mapping for database transformations, handling @Map on enum values
packages/orm/src/client/helpers/schema-db-pusher.ts Updated to process enum fields with @Map attributes when creating PostgreSQL enums
packages/testtools/src/client.ts Refactored API to use schemaFile as an option property instead of a separate parameter
packages/language/res/stdlib.zmodel Fixed @@Schema attribute parameter name from name to map
Test files Updated all test schemas and models to use new enum structure (enums.Role.values)

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

@ymc9 ymc9 changed the title fix: stricter default schema validation and proper transformation to Prisma schema feat: enum name mapping support and several fixes about postgres default schema Nov 12, 2025
@claude
Copy link

claude bot commented Nov 12, 2025

Claude finished @ymc9's task —— View job


Code Review for PR #391

Reviewing PR #391: feat: enum name mapping support and several fixes about postgres default schema

Review Tasks

  • Read and analyze core implementation files
  • Review enum mapping logic in name-mapper.ts
  • Check validation changes in validator/index.ts
  • Assess schema generation updates
  • Review test coverage and implementation
  • Check for potential bugs, security issues, and performance concerns
  • Provide comprehensive feedback and recommendations

📋 Summary

This PR implements enum name mapping functionality and improves schema validation. The main changes include:

  1. Enum value mapping support - Allows @map attributes on enum values to customize database storage
  2. Enhanced name mapping - Comprehensive handling of field/table name mapping in queries
  3. Improved validation - Stricter schema validation with better error messages
  4. Comprehensive test coverage - Extensive tests for the new enum mapping features

✅ Code Quality

Strengths:

  • Well-structured implementation: Clear separation between name mapping logic and enum handling
  • Comprehensive test coverage: Extensive test suite covering CRUD operations, aggregations, and edge cases
  • Good error handling: Proper invariants and validation throughout the codebase
  • Performance conscious: Uses caching for schema validation to avoid redundant work

🔍 Detailed Analysis

Core Changes

packages/orm/src/client/executor/name-mapper.ts (+279/-42):

  • Major enhancement to QueryNameMapper class with enum value mapping support
  • Added comprehensive enum value transformation logic for both input and output
  • Proper handling of scope resolution and alias management
  • Uses CASE-WHEN expressions for enum value mapping in select queries

packages/orm/src/client/crud/validator/index.ts (+38/-21):

  • Enhanced enum validation logic in makeEnumSchema
  • Improved error handling for enum fields
  • Better integration with name mapping functionality

packages/sdk/src/ts-schema-generator.ts (+69/-7):

  • Extended schema generation to include enum field attributes
  • Proper handling of @map attributes on enum values
  • Enhanced enum object creation with field-level attributes

🔒 Security Considerations

✅ No Security Issues Found

  • No direct security vulnerabilities identified
  • Proper input validation and sanitization
  • No exposure of sensitive information in error messages
  • Uses parameterized queries through Kysely (SQL injection protection)

🚀 Performance Considerations

Optimizations:

  • Schema caching: schemaCache WeakMap prevents redundant validation schema creation
  • Lazy evaluation: Uses TypeScript lazy types for recursive schema definitions
  • Efficient enum mapping: Only applies CASE-WHEN transformations when enum mappings exist

Potential Concerns:

  • CASE-WHEN expressions for enum mapping add SQL complexity, but necessary for correct mapping
  • Large enum sets with many mapped values could result in complex SQL expressions

🧪 Test Coverage Assessment

Excellent Coverage:

  • tests/e2e/orm/client-api/name-mapping.test.ts provides comprehensive testing
  • Tests cover all CRUD operations with enum mapping
  • Includes edge cases like mixed mapped/unmapped enum values
  • Tests both ORM-level and raw Kysely query operations
  • Validates correct database storage vs. application-level representation

Test Scenarios Covered:

  • Create with enum defaults and explicit values
  • Find operations with enum filtering
  • Update with enum value changes
  • Delete operations
  • Count and aggregate operations with enums
  • GroupBy operations with enum fields

🐛 Potential Issues

Minor Concerns:

  1. name-mapper.ts:707: The explicit cast to 'text' for PostgreSQL CASE-WHEN expressions is a workaround for type inference issues. This is documented and appears to be the correct approach.

  2. Enum validation edge case: In makeEnumSchema, empty enum values result in z.never() schema, which might be too restrictive. Consider if this should allow null/undefined for optional fields.

  3. Performance with complex mappings: The CASE-WHEN approach for enum mapping could become unwieldy with very large enums (>50 values), though this is an edge case.

🔧 Recommendations

Code Quality:

  1. Good: Proper use of TypeScript for type safety
  2. Good: Comprehensive error handling with invariants
  3. Good: Clear separation of concerns between mapping and validation

Documentation:

  1. Consider: Adding JSDoc comments for complex enum mapping logic in name-mapper.ts
  2. Consider: Document the PostgreSQL text casting workaround

Testing:

  1. Excellent: Test coverage is comprehensive and well-structured
  2. Suggestion: Consider adding performance tests for large enum sets

🎯 Verdict

✅ Approve - This is a well-implemented feature with:

  • Solid architecture and clean code
  • Comprehensive test coverage
  • No security vulnerabilities
  • Good performance characteristics
  • Proper handling of edge cases

The enum mapping functionality is correctly implemented and the validation improvements enhance the robustness of the schema handling. The extensive test suite provides confidence in the implementation's correctness.


@ymc9
Copy link
Member Author

ymc9 commented Nov 12, 2025

fixes #315 #316

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants