Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Nov 3, 2025

Summary by CodeRabbit

  • New Features
    • Added --lite and --lite-only CLI options to produce lighter schema variants; generator can emit both full and lite schemas.
  • Tests
    • Added tests verifying lite and lite-only generation and that lite schemas omit attribute metadata.
  • Chores
    • Updated build/test scripts and TypeScript configs to run generation and type-checks for the improved test workflow.

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

coderabbitai bot commented Nov 3, 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

Adds a "lite" generation mode and CLI flags (--lite, --lite-only), changes TsSchemaGenerator.generate to accept an options object ({ outDir, lite, liteOnly }), threads lite/liteOnly through generation internals to emit schema.ts and/or schema-lite.ts, and updates callers, tests, and samples to use the new behavior.

Changes

Cohort / File(s) Summary
CLI flags & plugin wiring
packages/cli/src/index.ts, packages/cli/src/actions/generate.ts
Added --lite and --lite-only options; extended Options type; refactored plugin binding to pass pluginOptions objects (lite/liteOnly) and updated default plugin handling.
TypeScript plugin integration
packages/cli/src/plugins/typescript.ts
Passes TsSchemaGenerator.generate an options object { outDir, lite, liteOnly } derived from pluginOptions instead of a simple outDir string.
Core generator (API + implementation)
packages/sdk/src/ts-schema-generator.ts
Changed generate signature to accept TsSchemaGeneratorOptions ({ outDir, lite?, liteOnly?}); threaded lite flag through many internal methods; emit both schema.ts and/or schema-lite.ts depending on flags; adjusted import/emit logic.
Clients: tanstack-query changes
packages/clients/tanstack-query/package.json, packages/clients/tanstack-query/scripts/generate.ts, packages/clients/tanstack-query/tsconfig.json, packages/clients/tanstack-query/tsconfig.test.json, packages/clients/tanstack-query/test/*
Updated build script to run generation and typecheck; added tsconfig.test.json; updated generate script to call TsSchemaGenerator.generate with options object (outDir, liteOnly true); updated tests and schema imports to use schema-lite.
Testtools & test call sites
packages/testtools/src/schema.ts, tests/e2e/orm/scripts/generate.ts, tests/regression/generate.ts, packages/cli/test/*, packages/cli/test/ts-schema-gen.test.ts
generateTsSchema and other callers now pass options object to TsSchemaGenerator.generate; tests added/updated to assert lite and lite-only behaviors and lite schema attribute omissions.
Samples & sample schema files
samples/next.js/package.json, samples/next.js/app/page.tsx, samples/next.js/zenstack/*, samples/next.js/zenstack/schema-lite.ts
Sample generate script now uses --lite; imports updated to schema-lite; added generated schema-lite.ts for sample app.
Schema content changes (lite variant)
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts
Stripped inline attribute metadata from schema fields while preserving functional properties (id/default/unique/relation semantics) for the lite schema representation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay extra attention to:
    • packages/sdk/src/ts-schema-generator.ts — correct propagation of lite/liteOnly and conditional emission paths.
    • packages/cli/src/actions/generate.ts — default plugin construction and option binding semantics.
    • Call sites: ensure all TsSchemaGenerator.generate invocations uniformly use the new options shape.
    • Tests that assert attribute omission — verify assertions align with emitted lite schema shape.

Possibly related PRs

Poem

🐇 I hopped through code at break of light,

Trimmed attributes, made schemas light.
Flags now whisper: "lite" or "lite-only" — neat,
Two files produced, both tidy and sweet.
Hooray for smaller hops and faster flights! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'feat(schema): add option for generating lite schema' accurately and concisely describes the main change in the changeset. It clearly communicates that a new feature is being added for generating a lite version of schemas, which aligns with the primary objectives evident throughout the changes: adding --lite and --lite-only CLI options, implementing lite schema generation in TsSchemaGenerator, and updating multiple files to use the new schema-lite variant.
✨ 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/lite-schema

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 3, 2025

Claude finished @ymc9's task —— View job


PR Review: feat(schema): add option for generating lite schema

I'll review this PR and provide feedback on code quality, potential issues, performance, security, and test coverage.

Review Tasks:

  • Analyze core schema generator changes
  • Review CLI integration and options
  • Examine test coverage and implementation
  • Check for potential bugs and security issues
  • Evaluate performance implications
  • Provide comprehensive feedback

Detailed Analysis

✅ Core Schema Generator Implementation (packages/sdk/src/ts-schema-generator.ts)

Excellent Design Decisions:

  • Clean separation between full and lite schema generation using the lite boolean parameter
  • Appropriate skipping of attributes at both model-level (lines 265-273) and field-level (lines 516-526)
  • Consistent file naming convention: schema.ts for full, schema-lite.ts for lite
  • Proper TypeScript factory usage for generating AST nodes
  • The liteOnly option correctly controls whether to generate only lite schema or both versions

⚠️ Issue Found: Missing liteOnly in TypeScript Plugin

Critical Bug (packages/cli/src/plugins/typescript.ts:22):

// Current - missing liteOnly parameter
await new TsSchemaGenerator().generate(model, { outDir, lite });

// Should be:
await new TsSchemaGenerator().generate(model, { outDir, lite, liteOnly: pluginOptions['liteOnly'] === true });

This means the --lite-only CLI flag is parsed but never passed to the actual generator, so both schema files are always generated even when only lite is requested.

✅ CLI Integration

Excellent CLI Design:

  • Clear option names --lite and --lite-only with descriptive help text
  • Proper defaults (both false)
  • Options correctly parsed and passed to plugins (except the bug above)

✅ Test Coverage Analysis

Strong Test Updates:

  • TanStack Query tests properly updated to use lite schema (packages/clients/tanstack-query/test/react-query.test.tsx:12)
  • Generated lite schema files in test fixtures show correct attribute removal
  • Sample applications updated with lite schema generation

Missing Test Coverage:

  • No direct unit tests for the --lite-only flag behavior
  • No tests verifying that --lite-only generates only schema-lite.ts and not schema.ts

🔒 Security Assessment

No Security Concerns Found:

  • Feature only affects generated TypeScript files, not runtime behavior
  • No new attack vectors introduced
  • Generated schemas still maintain type safety

⚡ Performance Implications

Positive Performance Impact:

  • Lite schemas will be smaller due to removed attributes
  • Faster parsing/loading for applications that don't need full attribute information
  • Build time impact is minimal - just additional file generation

🐛 Additional Minor Issues

  1. Type Inconsistency: The plugin interface could be more explicit about which options are supported
  2. Documentation: No inline documentation explaining when to use lite vs full schemas

🎯 Recommendations

  1. Fix the Critical Bug: Add liteOnly parameter to TypeScript plugin
  2. Add Unit Tests: Test --lite-only flag specifically
  3. Add Validation: Prevent both --lite and --lite-only being used together (could be confusing)
  4. Documentation: Add JSDoc comments explaining the lite schema concept

Overall Assessment

This is a well-designed feature with clean implementation and good separation of concerns. The core logic is solid, but there's one critical bug preventing the --lite-only option from working correctly. Once fixed, this will be a valuable addition for applications needing lightweight schema definitions.

Verdict: Approve with required fixes


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 pull request adds support for generating a "lite" version of TypeScript schemas that excludes attribute metadata, reducing the generated code size for runtime usage. The lite schema contains only the essential runtime information needed for query execution.

  • Introduced --lite and --lite-only CLI flags to control schema generation modes
  • Modified TsSchemaGenerator to conditionally generate schema files with or without attributes
  • Updated generated files to import from schema-lite.ts instead of schema.ts where appropriate

Reviewed Changes

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

Show a summary per file
File Description
packages/sdk/src/ts-schema-generator.ts Core implementation of lite schema generation logic, including conditional attribute generation
packages/cli/src/index.ts Added --lite and --lite-only command-line options
packages/cli/src/actions/generate.ts Passes lite mode options to the TypeScript plugin
packages/cli/src/plugins/typescript.ts Handles lite mode option from plugin configuration
tests/regression/generate.ts Updated to use new API signature with options object
tests/e2e/orm/scripts/generate.ts Updated to use new API signature with options object
packages/testtools/src/schema.ts Updated to use new API signature with options object
packages/clients/tanstack-query/scripts/generate.ts Updated to use new API signature with liteOnly: true
packages/clients/tanstack-query/test/schemas/basic/schema-lite.ts Generated lite schema with attributes removed
samples/next.js/zenstack/schema-lite.ts Generated lite schema for sample project
samples/next.js/package.json Updated script to use --lite flag
packages/clients/tanstack-query/package.json Updated build script to include test generation and type checking
packages/clients/tanstack-query/tsconfig.json Separated test files to dedicated test config
packages/clients/tanstack-query/tsconfig.test.json New TypeScript config for test files with JSX support
packages/clients/tanstack-query/test/react-query.test.tsx Multiple test improvements and import updates

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

@claude
Copy link

claude bot commented Nov 3, 2025

Claude finished @ymc9's task —— View job


Claude Code is reviewing this PR…

PR Review: feat(schema): add option for generating lite schema

Review Tasks:

  • Analyze core schema generator changes
  • Review CLI integration and options
  • Examine test coverage and implementation
  • Check for potential bugs and security issues
  • Evaluate performance implications
  • Provide comprehensive feedback

Detailed Analysis

✅ Core Schema Generator Implementation (packages/sdk/src/ts-schema-generator.ts)

Excellent Design Decisions:

  • Clean separation between full and lite schema generation using the lite boolean parameter
  • Appropriate skipping of attributes at both model-level (lines 265-273) and field-level (lines 516-526)
  • Consistent file naming convention: schema.ts for full, schema-lite.ts for lite
  • Proper TypeScript factory usage for generating AST nodes
  • The multi-target generation logic (lines 79-109) correctly handles all combinations of lite and liteOnly flags
  • Smart conditional import generation based on useLite flag (lines 1271-1302)

✅ CLI Integration

Well-designed CLI options:

  • Clear option names --lite and --lite-only with descriptive help text
  • Proper defaults (both false)
  • Options correctly parsed and passed to plugins
  • Good separation between generating both versions vs lite-only

✅ Test Coverage Analysis

Strong Test Implementation:

  • Tests in packages/cli/test/generate.test.ts verify both --lite and --lite-only flags work correctly (lines 48-60)
  • TanStack Query tests properly updated to use lite schema
  • Generated lite schema files show correct attribute removal while preserving essential metadata (e.g., unique: true)
  • Sample applications updated consistently

🔒 Security Assessment

No Security Concerns Found:

  • Feature only affects generated TypeScript files, not runtime behavior
  • No new attack vectors introduced
  • Generated schemas maintain type safety

⚡ Performance Implications

Positive Performance Impact:

  • Lite schemas will be significantly smaller due to removed attributes
  • Faster parsing/loading for applications that don't need full attribute information
  • Essential runtime information (relations, field types, constraints) preserved
  • Build time impact is minimal - just additional file generation

🐛 Code Quality Issues Found

1. Minor: Inconsistent Error Handling
In packages/cli/src/actions/generate.ts:125-128, plugin errors are logged but don't stop execution. This is probably intentional but could be documented.

2. Suggestion: Validation
Consider preventing both --lite and --lite-only being used together (could be confusing for users), though current behavior is well-defined.

📋 Code Quality Assessment

Strengths:

  • Clean abstraction with TsSchemaGeneratorOptions interface
  • Consistent code patterns throughout
  • Good separation of concerns between CLI parsing and schema generation
  • Proper handling of edge cases (no typeDefs, no enums, etc.)
  • Well-structured conditional logic for lite mode

Areas for Enhancement:

  • Could benefit from JSDoc comments explaining when to use lite vs full schemas
  • Plugin interface could be more explicit about supported options

🎯 Key Technical Insights

  1. Smart Attribute Filtering: The implementation correctly distinguishes between essential runtime metadata (like unique: true) and verbose attribute decorators that are only needed for tooling.

  2. Proper Import Resolution: The useLite flag correctly switches import paths between './schema' and './schema-lite' ensuring generated types reference the appropriate schema variant.

  3. Multi-file Generation: The target-based generation approach cleanly handles the complexity of generating multiple schema files based on user options.

Overall Assessment

This is a well-architected feature with solid implementation. The core logic is sound, the testing is comprehensive, and the CLI integration follows established patterns. The lite schema approach provides real value for applications needing lightweight runtime schemas while maintaining full compatibility.

Code Quality: ⭐⭐⭐⭐⭐ (5/5)
Test Coverage: ⭐⭐⭐⭐⭐ (5/5)
Performance Impact: ⭐⭐⭐⭐⭐ (5/5) - Positive
Security: ⭐⭐⭐⭐⭐ (5/5) - No concerns

Verdict: Approve


Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/cli/test/ts-schema-gen.test.ts (1)

424-444: Add explicit assertion for schemaLite before using non-null assertion.

The test uses schemaLite! without explicitly checking that schemaLite is defined. If the lite schema generation fails silently, the error message will be unclear.

Add an explicit assertion before the property checks:

         );
 
+        expect(schemaLite).toBeDefined();
         expect(schemaLite!.models.User.attributes).toBeUndefined();
         expect(schemaLite!.models.User.fields.id.attributes).toBeUndefined();
         expect(schemaLite!.models.User.fields.email.attributes).toBeUndefined();
📜 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 dd60d5c and 8d9778b.

📒 Files selected for processing (4)
  • packages/cli/src/plugins/typescript.ts (1 hunks)
  • packages/cli/test/generate.test.ts (1 hunks)
  • packages/cli/test/ts-schema-gen.test.ts (1 hunks)
  • packages/testtools/src/schema.ts (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/cli/test/generate.test.ts (1)
packages/cli/test/utils.ts (2)
  • createProject (12-18)
  • runCli (20-23)
packages/cli/test/ts-schema-gen.test.ts (1)
packages/testtools/src/schema.ts (1)
  • generateTsSchema (35-66)
packages/testtools/src/schema.ts (1)
packages/schema/src/schema.ts (1)
  • SchemaDef (10-18)
packages/cli/src/plugins/typescript.ts (1)
packages/sdk/src/ts-schema-generator.ts (1)
  • TsSchemaGenerator (60-1450)
⏰ 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: build-test (20.x, postgresql)
  • GitHub Check: build-test (20.x, sqlite)
  • GitHub Check: claude-review
🔇 Additional comments (5)
packages/cli/test/generate.test.ts (2)

48-53: LGTM!

The test correctly validates that the --lite flag generates both schema.ts and schema-lite.ts files.


55-60: LGTM!

The test correctly validates that the --lite-only flag generates only schema-lite.ts while skipping the regular schema.ts file.

packages/cli/src/plugins/typescript.ts (1)

10-25: LGTM!

The implementation correctly extracts both lite and liteOnly options from pluginOptions and passes them to the TsSchemaGenerator. This addresses the previous review comment about missing liteOnly support.

packages/testtools/src/schema.ts (2)

40-40: LGTM!

The withLiteSchema parameter is correctly added as an optional boolean parameter.


54-54: LGTM!

The changes correctly adapt to the new TsSchemaGenerator API:

  • Line 54: Passes options object with lite flag to the generator
  • Lines 77-83: Gracefully handles optional schema-lite.js loading with try-catch
  • Line 98: Updates generateTsSchemaInPlace to use the options object pattern

The return type will be correctly inferred to include the optional schemaLite property.

Also applies to: 77-83, 98-98

@ymc9 ymc9 merged commit 170535a into dev Nov 3, 2025
6 checks passed
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