Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Aug 24, 2025

Summary by CodeRabbit

  • New Features
    • Enum filters now support aggregations (_count, _min, _max) for more powerful queries.
  • Bug Fixes
    • GroupBy validation correctly disallows “by” when no non-relational fields are available.
  • Refactor
    • Improved name-mapping reliability across nested queries and joins; no API changes.
  • Tests
    • Added schema-driven end-to-end tests covering create, read, update, delete, and joins.
  • Chores
    • Version bump to 3.0.0-alpha.31 across packages.

ymc9 and others added 3 commits August 23, 2025 12:12
* fix: misc code review issues

* fix lint
* refactor: re-implement name mapper

* update

* update
Copilot AI review requested due to automatic review settings August 24, 2025 06:50
@coderabbitai
Copy link

coderabbitai bot commented Aug 24, 2025

Walkthrough

The PR bumps versions across many packages and samples, adds enum aggregation support via a new type parameter in CRUD types, adjusts groupBy validation for empty non-relational fields, refactors query name-mapping to a scope-based approach, and overhauls/expands tests with a new schema-driven harness and generated test schema files.

Changes

Cohort / File(s) Summary
Version bumps
package.json, packages/*/package.json, samples/blog/package.json, tests/e2e/package.json
Increment version fields from 3.0.0-alpha.30 to 3.0.0-alpha.31; no other changes.
Runtime types: enum aggregation
packages/runtime/src/client/crud-types.ts
Adds WithAggregations generic to WhereInput and EnumFilter; enables enum field aggregations (_count, _min, _max); updates internal references and not-propagation.
Runtime validator: groupBy "by"
packages/runtime/src/client/crud/validator.ts
Introduces bySchema to disallow “by” when no non-relational fields exist; otherwise preserves existing enum-or-array behavior.
Name mapper refactor (scoped)
packages/runtime/src/client/executor/name-mapper.ts
Replaces stack with scope-based resolution; adds helpers for FROM/JOIN processing, reference/column mapping, select-all expansion, and nested query wrapping; adjusts transform flows for select/insert/update/delete/returning.
Client API test overhaul
packages/runtime/test/client-api/name-mapping.test.ts
Migrates to schema-driven test client; exercises create/find/update/delete with joins, returning, aliasing, and includes; replaces inline schema setup.
Generated test schema artifacts
packages/runtime/test/schemas/name-mapping/schema.zmodel, .../schema.ts, .../models.ts, .../input.ts
Adds ZModel source and generated TS schema/types for User/Post models used by tests.
Test utilities adjustment
packages/runtime/test/utils.ts
Asserts workDir presence via invariant; applies non-null assertions for schema paths; logs workDir unconditionally.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

A bunny maps the names with care,
Hops through scopes, refactors air.
Enums count, find min and max,
GroupBy checks its “by” with facts.
Tests now bloom from schema’s seed—
Version bumps? Just what we need.
Thump-thump, shipped at rabbit speed! 🐇✨

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@claude
Copy link

claude bot commented Aug 24, 2025

Claude finished @ymc9's task —— View job


Code Review for PR #205: merge dev to main

Reviewing changes for:

  • Code quality and best practices
  • Potential bugs or issues
  • Performance considerations
  • Security concerns
  • Test coverage

Key Changes Identified:

  • Version bump to 3.0.0-alpha.31 across all packages
  • Major refactoring of name-mapper.ts implementation
  • New comprehensive test suite for name mapping
  • 26 files changed (+748/-288 lines)

Review Summary

This PR contains a substantial refactoring of the name mapping functionality, which appears to be a critical component for handling database schema to model name translations. The changes look well-implemented with comprehensive test coverage.

✅ Strengths

Code Quality & Architecture:

  • Clean, well-structured class design with clear separation of concerns
  • Proper use of TypeScript generics and type safety
  • Good use of composition patterns and helper methods
  • Comprehensive error handling with invariant assertions
  • Clear method naming and logical organization into regions

Name Mapping Implementation (packages/runtime/src/client/executor/name-mapper.ts):

  • Robust scope management system for handling nested queries and aliases
  • Intelligent handling of SQL AST transformations via Kysely's OperationNodeTransformer
  • Proper support for complex query scenarios (joins, subqueries, aliases)
  • Smart optimization: only creates nested queries when name mapping is actually needed (hasMappedColumns)

Test Coverage (packages/runtime/test/client-api/name-mapping.test.ts):

  • Comprehensive test suite covering CRUD operations
  • Tests both high-level ORM API and low-level Kysely query builder
  • Good coverage of edge cases like aliases, joins, and nested queries
  • Real-world schema with @map attributes for both tables and columns

🔧 Technical Analysis

Performance Considerations:

  • ✅ Efficient mapping with pre-computed Map structures for O(1) lookups
  • ✅ Smart nested query generation only when needed
  • ✅ Proper scope management prevents unnecessary transformations
  • ✅ Use of toReversed() for scope resolution follows correct precedence

Security:

  • ✅ No SQL injection risks - uses Kysely's type-safe query building
  • ✅ Proper input validation with schema definition constraints
  • ✅ No dynamic string concatenation for SQL generation

Robustness:

  • ✅ Handles complex scenarios: aliases, joins, subqueries, SELECT * expansion
  • ✅ Graceful fallbacks when mapping information isn't available
  • ✅ Proper handling of both table-level (@@map) and field-level (@map) mapping

📝 Minor Observations

  1. Code Organization: The class is well-structured with clear regions, making it maintainable despite its complexity.

  2. Memory Management: The scope stack management is proper with try/finally blocks ensuring cleanup.

  3. Type Safety: Strong TypeScript usage throughout with proper type guards and assertions.

  4. Kysely Integration: Excellent integration with Kysely's AST transformation patterns.

🧪 Test Quality

The test suite is comprehensive and covers:

  • Basic CRUD operations with name mapping
  • Complex queries with joins and subqueries
  • Both ORM-style and direct Kysely query builder usage
  • Edge cases like aliases and SELECT * expansion
  • Proper schema setup with realistic @map attributes

🚀 Recommendation

APPROVE - This is a high-quality refactoring that significantly improves the name mapping functionality. The code is well-architected, thoroughly tested, and handles complex SQL transformation scenarios properly. The implementation follows best practices and maintains type safety throughout.

The refactoring appears to address limitations of the previous implementation while maintaining backward compatibility and adding robust support for complex query scenarios.


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: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/runtime/src/client/crud/validator.ts (1)

1306-1318: Bug: orderBy refinement assumes object but schema allows array; valid inputs may be rejected.

orderBy is defined as union of object | object[]. The refinement treats it as object and calls Object.keys directly. If an array is provided, keys like "0"/"1" are checked against bys, causing false negatives.

Apply this refactor to normalize orderBy to an array before validation:

-        schema = schema.refine((value) => {
-            const bys = typeof value.by === 'string' ? [value.by] : value.by;
-            if (
-                value.orderBy &&
-                Object.keys(value.orderBy)
-                    .filter((f) => !AGGREGATE_OPERATORS.includes(f as AGGREGATE_OPERATORS))
-                    .some((key) => !bys.includes(key))
-            ) {
-                return false;
-            } else {
-                return true;
-            }
-        }, 'fields in "orderBy" must be in "by"');
+        schema = schema.refine((value) => {
+            const bys = typeof value.by === 'string' ? [value.by] : value.by;
+            const orderByEntries = value.orderBy
+                ? (Array.isArray(value.orderBy) ? value.orderBy : [value.orderBy])
+                : [];
+            const violates = orderByEntries.some((o: Record<string, unknown>) =>
+                Object.keys(o)
+                    .filter((f) => !AGGREGATE_OPERATORS.includes(f as AGGREGATE_OPERATORS))
+                    .some((key) => !bys.includes(key)),
+            );
+            return !violates;
+        }, 'fields in "orderBy" must be in "by"');
packages/runtime/src/client/crud-types.ts (1)

205-243: Propagate WithAggregations through boolean combinators (AND/OR/NOT).

WhereInput gained WithAggregations, but AND, OR, and NOT still instantiate WhereInput without forwarding it. This disables aggregated predicates under combinators (e.g., in having).

Apply:

 } & {
     $expr?: (eb: ExpressionBuilder<ToKyselySchema<Schema>, Model>) => OperandExpression<SqlBool>;
 } & {
-    AND?: OrArray<WhereInput<Schema, Model, ScalarOnly>>;
-    OR?: WhereInput<Schema, Model, ScalarOnly>[];
-    NOT?: OrArray<WhereInput<Schema, Model, ScalarOnly>>;
+    AND?: OrArray<WhereInput<Schema, Model, ScalarOnly, WithAggregations>>;
+    OR?: WhereInput<Schema, Model, ScalarOnly, WithAggregations>[];
+    NOT?: OrArray<WhereInput<Schema, Model, ScalarOnly, WithAggregations>>;
 };
🧹 Nitpick comments (16)
packages/dialects/sql.js/package.json (1)

3-3: Version bump acknowledged; consider explicit publishConfig to avoid accidental restricted publish.

Scoped packages default to "restricted" access on npm. If this package is intended to be public, add publishConfig to avoid publish-time surprises.

You can add:

   "license": "MIT",
+  "publishConfig": { "access": "public" },
   "files": [
packages/zod/package.json (1)

31-38: Align zod devDependencies and peerDependencies across the monorepo

Quick check confirms that both packages/zod and packages/runtime pin zod to ~3.25.0 under devDependencies but use catalog: under peerDependencies, which can lead to dual installs if the catalog alias shifts versions.

Please align these in both packages—either:

  • Use catalog: for both dev and peer, so you always reference the workspace zod package
  • Or pin both to the same semver range (~3.25.0) to guarantee only one version is installed

Locations to update:

  • packages/zod/package.json (lines 34–38)
  • packages/runtime/package.json (lines 82–103)
packages/runtime/src/client/crud/validator.ts (1)

1259-1263: Good hardening: avoid constructing z.enum([]) for models without non-relational fields.

Conditionally computing bySchema prevents runtime errors when there are no scalar fields to group by. One small UX improvement: z.never() yields a generic error. Consider emitting a clearer message so users know why groupBy is invalid for the model.

Here’s a minimally invasive tweak that keeps the same behavior but improves the error:

-        const bySchema =
-            nonRelationFields.length > 0
-                ? this.orArray(z.enum(nonRelationFields as [string, ...string[]]), true)
-                : z.never();
+        const bySchema =
+            nonRelationFields.length > 0
+                ? this.orArray(z.enum(nonRelationFields as [string, ...string[]]), true)
+                : z.any().refine(() => false, {
+                      message: `Model "${model}" has no non-relational scalar fields to group by`,
+                  });

Also applies to: 1267-1267

packages/runtime/test/utils.ts (1)

136-137: Invariant on workDir is a solid guard; consider gating the console log behind a debug flag.

The assert eliminates downstream non-null assertions. The unconditional console.log can be noisy in CI.

-    console.log(`Work directory: ${workDir}`);
+    if (process.env.ZENSTACK_TEST_DEBUG) {
+        console.log(`Work directory: ${workDir}`);
+    }
packages/runtime/src/client/executor/name-mapper.ts (6)

88-94: Future-proof ReturningNode handling: preserve all properties from super.

Current override rebuilds ReturningNode with only kind and selections. If Kysely adds fields later, they’d be dropped. Safer to spread the transformed base node and only replace selections.

Apply:

-    protected override transformReturning(node: ReturningNode) {
-        return {
-            kind: node.kind,
-            // map column names in returning selections (include returningAll)
-            selections: this.processSelections(node.selections),
-        };
-    }
+    protected override transformReturning(node: ReturningNode) {
+        const base = super.transformReturning(node) as ReturningNode;
+        return {
+            ...base,
+            // map column names in returning selections (include returningAll)
+            selections: this.processSelections(node.selections),
+        };
+    }

77-86: Be explicit about scope flags for insert.

Minor: the scope created for transformInsertQuery leaves namesMapped undefined. Make it explicit (false) for readability/consistency with other call sites.

-        return this.withScope(
-            { model: node.into.table.identifier.name },
+        return this.withScope(
+            { model: node.into.table.identifier.name, namesMapped: false },
             () =>
                 ({
                     ...super.transformInsertQuery(node),
                     // map table name
                     into: this.processTableRef(node.into!),
                 }) satisfies InsertQueryNode,
         );

53-69: Ensure from is transformed exactly once.

You run super.transformSelectQuery(node) and then overwrite .from with the result of processFrom(node.from!), which itself calls super.transformFrom. This is fine, but it means the from subtree is not the one returned by super.transformSelectQuery. Consider capturing the base result and only replacing its from to keep a single transformation pass over children.

-        return this.withScopes(scopes, () => {
-            return {
-                ...super.transformSelectQuery(node),
-                // convert "from" to nested query as needed
-                from: this.processFrom(node.from!),
-            };
-        });
+        return this.withScopes(scopes, () => {
+            const base = super.transformSelectQuery(node) as SelectQueryNode;
+            return {
+                ...base,
+                from: this.processFrom(node.from!),
+            };
+        });

323-345: Consistency: prefer super.transformNode or super.transformFrom, not this.transformNode.

In processFrom, the else branches mix super.transformNode(from) and this.transformNode(from). Using super avoids re-entering your overrides unexpectedly and keeps the intent clear.

-                    return super.transformNode(from);
+                    return super.transformNode(from);
                 }
-                return this.transformNode(from);
+                return super.transformNode(from);

Note: the first occurrence is already super; the second should match it.


303-306: Performance nit: hasMappedColumns scans the entire map each time.

For hot paths, consider building a Set<string> of models with mapped fields during construction to make this O(1). Not critical, just a micro-optimization.


400-416: Correlation: good handling, but add a test for nested scopes with aliases.

processSelectAll takes the top scope; this works under your withScope(s) usage. To guard against regressions, add a test where a correlated subquery uses an inner alias different from the outer model name (e.g., selectFrom('User as u') inside a selectFrom('Post')) and verifies whereRef('u.id','=','Post.authorId').

I can draft an additional vitest case targeting this scenario if you want it included in this PR.

packages/runtime/test/client-api/name-mapping.test.ts (3)

10-16: Test setup is clean; consider shaving runtime if CI is tight.

Using { usePrismaPush: true } rebuilds the DB per test case via beforeEach, which is thorough but slower. If acceptable, moving to beforeAll/afterAll or relying on $pushSchema() for SQLite can make the suite faster while keeping isolation.


146-154: Great correlated subquery assertion; add an aliasing variant.

You already test a correlated subquery. Add a variant with selectFrom('User as u') and whereRef('u.id','=','Post.authorId') to lock in alias behavior.


217-223: Potentially brittle assumption on matching IDs.

deleteFrom('Post') ... returning(['id', 'title']) expects { id: user.id }. This holds because both are 1 in a fresh DB, but it ties two independent sequences together. Consider asserting only title or capturing the created post ID instead.

Apply:

-        ).resolves.toMatchObject({
-            id: user.id,
-            title: 'Post1',
-        });
+        ).resolves.toMatchObject({
+            title: 'Post1',
+        });
packages/runtime/test/schemas/name-mapping/input.ts (3)

8-10: Minor: consolidate type-only imports from the same module

You can reduce duplication by merging the two type-only imports from "@zenstackhq/runtime" into one. Not critical since this is generated, but it slightly improves readability.

Apply this diff locally if you plan to tweak the generator:

-import type { FindManyArgs as $FindManyArgs, FindUniqueArgs as $FindUniqueArgs, FindFirstArgs as $FindFirstArgs, CreateArgs as $CreateArgs, CreateManyArgs as $CreateManyArgs, CreateManyAndReturnArgs as $CreateManyAndReturnArgs, UpdateArgs as $UpdateArgs, UpdateManyArgs as $UpdateManyArgs, UpdateManyAndReturnArgs as $UpdateManyAndReturnArgs, UpsertArgs as $UpsertArgs, DeleteArgs as $DeleteArgs, DeleteManyArgs as $DeleteManyArgs, CountArgs as $CountArgs, AggregateArgs as $AggregateArgs, GroupByArgs as $GroupByArgs, WhereInput as $WhereInput, SelectInput as $SelectInput, IncludeInput as $IncludeInput, OmitInput as $OmitInput } from "@zenstackhq/runtime";
-import type { SimplifiedModelResult as $SimplifiedModelResult, SelectIncludeOmit as $SelectIncludeOmit } from "@zenstackhq/runtime";
+import type {
+  FindManyArgs as $FindManyArgs,
+  FindUniqueArgs as $FindUniqueArgs,
+  FindFirstArgs as $FindFirstArgs,
+  CreateArgs as $CreateArgs,
+  CreateManyArgs as $CreateManyArgs,
+  CreateManyAndReturnArgs as $CreateManyAndReturnArgs,
+  UpdateArgs as $UpdateArgs,
+  UpdateManyArgs as $UpdateManyArgs,
+  UpdateManyAndReturnArgs as $UpdateManyAndReturnArgs,
+  UpsertArgs as $UpsertArgs,
+  DeleteArgs as $DeleteArgs,
+  DeleteManyArgs as $DeleteManyArgs,
+  CountArgs as $CountArgs,
+  AggregateArgs as $AggregateArgs,
+  GroupByArgs as $GroupByArgs,
+  WhereInput as $WhereInput,
+  SelectInput as $SelectInput,
+  IncludeInput as $IncludeInput,
+  OmitInput as $OmitInput,
+  SimplifiedModelResult as $SimplifiedModelResult,
+  SelectIncludeOmit as $SelectIncludeOmit,
+} from "@zenstackhq/runtime";

30-30: Optional: consider providing a convenience default for GetPayload

If consumers frequently use “default payload” without select/include/omit, adding a helper alias (e.g., UserGetDefaultPayload) could reduce boilerplate in tests. Only do this if it matches runtime semantics for “no SIO args”.

For example (placed after current exports):

// Helper for default payload (no select/include/omit)
// Ensure "{}" matches your SimplifiedModelResult semantics before adopting.
export type UserGetDefaultPayload = $SimplifiedModelResult<$Schema, "User", {}>;

50-50: Same optional helper consideration for Post GetPayload

Mirroring any User default payload helper for Post keeps the surface consistent.

Example:

export type PostGetDefaultPayload = $SimplifiedModelResult<$Schema, "Post", {}>;
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ccf968e and 43e4f16.

📒 Files selected for processing (26)
  • package.json (1 hunks)
  • packages/cli/package.json (1 hunks)
  • packages/common-helpers/package.json (1 hunks)
  • packages/create-zenstack/package.json (1 hunks)
  • packages/dialects/sql.js/package.json (1 hunks)
  • packages/eslint-config/package.json (1 hunks)
  • packages/ide/vscode/package.json (1 hunks)
  • packages/language/package.json (1 hunks)
  • packages/runtime/package.json (1 hunks)
  • packages/runtime/src/client/crud-types.ts (2 hunks)
  • packages/runtime/src/client/crud/validator.ts (1 hunks)
  • packages/runtime/src/client/executor/name-mapper.ts (4 hunks)
  • packages/runtime/test/client-api/name-mapping.test.ts (1 hunks)
  • packages/runtime/test/schemas/name-mapping/input.ts (1 hunks)
  • packages/runtime/test/schemas/name-mapping/models.ts (1 hunks)
  • packages/runtime/test/schemas/name-mapping/schema.ts (1 hunks)
  • packages/runtime/test/schemas/name-mapping/schema.zmodel (1 hunks)
  • packages/runtime/test/utils.ts (2 hunks)
  • packages/sdk/package.json (1 hunks)
  • packages/tanstack-query/package.json (1 hunks)
  • packages/testtools/package.json (1 hunks)
  • packages/typescript-config/package.json (1 hunks)
  • packages/vitest-config/package.json (1 hunks)
  • packages/zod/package.json (1 hunks)
  • samples/blog/package.json (1 hunks)
  • tests/e2e/package.json (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{packages,samples,tests}/**

📄 CodeRabbit inference engine (CLAUDE.md)

Packages are located in packages/, samples/, and tests/

Files:

  • packages/common-helpers/package.json
  • tests/e2e/package.json
  • packages/cli/package.json
  • packages/testtools/package.json
  • samples/blog/package.json
  • packages/vitest-config/package.json
  • packages/dialects/sql.js/package.json
  • packages/zod/package.json
  • packages/runtime/package.json
  • packages/typescript-config/package.json
  • packages/tanstack-query/package.json
  • packages/sdk/package.json
  • packages/create-zenstack/package.json
  • packages/runtime/test/schemas/name-mapping/models.ts
  • packages/eslint-config/package.json
  • packages/language/package.json
  • packages/ide/vscode/package.json
  • packages/runtime/src/client/crud/validator.ts
  • packages/runtime/test/schemas/name-mapping/schema.ts
  • packages/runtime/test/utils.ts
  • packages/runtime/test/schemas/name-mapping/schema.zmodel
  • packages/runtime/test/schemas/name-mapping/input.ts
  • packages/runtime/test/client-api/name-mapping.test.ts
  • packages/runtime/src/client/crud-types.ts
  • packages/runtime/src/client/executor/name-mapper.ts
tests/e2e/**

📄 CodeRabbit inference engine (CLAUDE.md)

E2E tests are in tests/e2e/ directory

Files:

  • tests/e2e/package.json
**/schema.zmodel

📄 CodeRabbit inference engine (CLAUDE.md)

Always run zenstack generate after modifying ZModel schemas

Files:

  • packages/runtime/test/schemas/name-mapping/schema.zmodel
🧠 Learnings (3)
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: `zenstack generate` compiles ZModel to TypeScript schema (`schema.ts`)

Applied to files:

  • packages/runtime/test/schemas/name-mapping/models.ts
  • packages/language/package.json
  • packages/runtime/test/schemas/name-mapping/schema.ts
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: ZModel schema (`schema.zmodel`) defines database structure and policies

Applied to files:

  • packages/runtime/test/schemas/name-mapping/schema.zmodel
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: Schema-first approach with ZModel DSL extension of Prisma schema language

Applied to files:

  • packages/runtime/test/schemas/name-mapping/schema.zmodel
🧬 Code graph analysis (6)
packages/runtime/src/client/crud/validator.ts (2)
samples/blog/zenstack/schema.ts (1)
  • schema (9-230)
packages/runtime/test/schemas/basic/schema.ts (1)
  • schema (9-284)
packages/runtime/test/schemas/name-mapping/schema.ts (2)
packages/runtime/src/schema/expression.ts (1)
  • ExpressionUtils (16-112)
packages/sdk/src/schema/schema.ts (1)
  • SchemaDef (10-18)
packages/runtime/test/utils.ts (3)
packages/common-helpers/src/tiny-invariant.ts (1)
  • invariant (4-14)
packages/language/src/index.ts (1)
  • loadDocument (21-132)
packages/sdk/src/prisma/prisma-schema-generator.ts (1)
  • PrismaSchemaGenerator (64-476)
packages/runtime/test/client-api/name-mapping.test.ts (3)
packages/runtime/src/client/contract.ts (1)
  • ClientContract (50-159)
packages/runtime/test/schemas/name-mapping/schema.ts (2)
  • SchemaType (88-88)
  • schema (9-87)
packages/runtime/test/utils.ts (1)
  • createTestClient (78-194)
packages/runtime/src/client/crud-types.ts (2)
packages/sdk/src/schema/schema.ts (5)
  • GetModelFieldType (151-155)
  • ModelFieldIsOptional (217-221)
  • SchemaDef (10-18)
  • GetEnums (123-123)
  • GetEnum (125-125)
packages/runtime/src/utils/type-utils.ts (1)
  • NullableIf (5-5)
packages/runtime/src/client/executor/name-mapper.ts (3)
packages/runtime/src/client/query-utils.ts (2)
  • getModel (16-18)
  • requireModel (20-26)
packages/common-helpers/src/tiny-invariant.ts (1)
  • invariant (4-14)
packages/sdk/src/schema/schema.ts (1)
  • ModelDef (20-36)
⏰ 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). (2)
  • GitHub Check: claude-review
  • GitHub Check: build-test (20.x)
🔇 Additional comments (31)
tests/e2e/package.json (1)

3-3: LGTM: e2e package version bumped consistently

Version updated to 3.0.0-alpha.31; matches the monorepo bump intent. No functional impact here.

packages/cli/package.json (1)

6-6: LGTM: CLI package version bump

Looks good. As a quick follow-up, ensure the published CLI reports the same version via zen --version in CI.

You can validate across the repo with the script in the runtime package comment below.

packages/common-helpers/package.json (1)

3-3: LGTM: common-helpers version bump

No other changes. This keeps internal workspace consumers aligned.

packages/runtime/package.json (1)

3-3: All package versions verified at 3.0.0-alpha.31

Script confirms that every package.json version field matches 3.0.0-alpha.31, there are no lingering “alpha.30” references, and all internal dependencies use the workspace protocol. LGTM.

packages/sdk/package.json (1)

3-3: LGTM: SDK package version bump

Aligned with the repo-wide release. Nothing else to call out.

packages/create-zenstack/package.json (1)

3-3: Version bump to 3.0.0-alpha.31 looks good

No functional or publish-surface changes in this file; aligns with the repo-wide release bump.

packages/typescript-config/package.json (1)

3-3: Typescript config package version bumped correctly

Pure metadata update; no API surface. All good.

packages/vitest-config/package.json (1)

4-4: Vitest config version bump approved

Matches the monorepo version. Nothing else changed.

package.json (1)

3-3: Version bump verified — no mismatches or stale references found

A full scan of all package.json files under packages/, samples/, and tests/ confirms every version is now set to 3.0.0-alpha.31. A repo-wide search also shows no lingering 3.0.0-alpha.30 references (excluding CHANGELOGs).

You can safely merge.

packages/eslint-config/package.json (1)

3-3: ESLint config package version bump approved

Metadata-only change; consistent with the release train.

packages/testtools/package.json (1)

3-3: I’ve added a script to list all CI workflow files and search for any “testtools” references. This will help confirm whether our CI matrix explicitly includes the correct peer dependency versions (better-sqlite3 ^12.2.0 and pg ^8.13.1). Please review the output to ensure the workflows install and test against those versions.

packages/tanstack-query/package.json (1)

3-3: Version bump approved – peerDependency for @tanstack/react-query remains ^5.0.0

  • Verified in packages/tanstack-query/package.json (line 35) that the peerDependency "@tanstack/react-query": "^5.0.0" is unchanged.
  • Confirmed that packages/tanstack-query/src/react.ts simply re-exports from @tanstack/react-query.
  • No local samples or tests import React Query here — please ensure any consuming examples/tests against v5 still run clean after this bump.
packages/ide/vscode/package.json (1)

4-4: tsup config correctly bundles all runtime dependencies; --no-dependencies publish is safe

Verified that packages/ide/vscode/tsup.config.ts uses

noExternal: [/^(?!vscode$)/],
external: ['vscode'],

which bundles every dependency except the VS Code API into dist. With vsce publish --no-dependencies, the extension is self‐contained and does not require node_modules at runtime—no further changes needed.

samples/blog/package.json (1)

3-3: Version bump consistency confirmed

All package.json files across the workspace (including samples/blog/package.json) have been updated to 3.0.0-alpha.31, and the pnpm-lock.yaml contains no lingering alpha.30 references. Lockfile and workspace are fully in sync—no further action required.

packages/zod/package.json (1)

3-3: LGTM on the package version bump.

The package version is correctly bumped to 3.0.0-alpha.31 with no other surface changes.

packages/runtime/test/utils.ts (2)

145-153: Non-null assertions on workDir are safe after invariant; flow is correct.

Using workDir! for loadDocument and file writes is justified post-invariant. Also good to enforce schema source presence when using prisma db push.


176-179: Sqlite file path uses workDir safely; handles relative dbName.

Joining workDir with dbName (default ./test.db) keeps test DBs contained per project.

packages/runtime/test/schemas/name-mapping/schema.zmodel (3)

6-11: User model mapping setup looks correct for name-mapping tests.

  • Field-level @Map on email and model-level @@Map to 'users' are consistent for exercising mapper behavior.
  • Unique on email ensures a stable lookup surface.

13-19: Post model mapping and relation look consistent; good coverage of both field and model mapping.

  • title and authorId mappings plus @@Map('posts') should adequately test scoped name mapping.
  • Relation via authorId -> User.id is correctly declared.

1-4: Generated artifacts verified – schema.ts, models.ts, and input.ts are all present alongside schema.zmodel in packages/runtime/test/schemas/name-mapping. No further action needed.

packages/runtime/test/schemas/name-mapping/schema.ts (2)

14-45: LGTM: mapping metadata mirrors the ZModel.

@map/@@map attributes and FK metadata are correctly represented for User.email → user_email, Post.title → post_title, Post.authorId → author_id, and model tables users/posts. Should interop cleanly with the new name-mapper.


8-9: Ensure dist/schema import is available in CI tests

The generated test at packages/runtime/test/schemas/name-mapping/schema.ts currently does:

import { type SchemaDef, ExpressionUtils } from "../../../dist/schema";

However, packages/runtime/dist isn’t committed and only appears after a build step. If the CI test runner invokes Jest/TS without first compiling, this import will fail.

• Confirm that your CI workflow for packages/runtime runs the build (e.g. npm run build or tsc) before executing tests—check your package.json scripts and .github/workflows/* or equivalent.
• If the build-then-test ordering isn’t in place, update the CI configuration to include the build step prior to testing.
• Alternatively, adjust the code-generator to emit import … from "../../src/schema" (or use a test-only flag) so it resolves directly against the source files.

packages/runtime/src/client/crud-types.ts (2)

245-264: Enum aggregations: shape looks correct and consistent with primitives.

Forwarding WithAggregations and adding _count/_min/_max aligns enum filters with other scalar filters. Nice.


1084-1088: Good call exposing aggregations in GroupByHaving via WhereInput<..., true, true>.

This unlocks aggregated enum predicates in having. Once the AND/OR/NOT fix lands, the entire having tree will correctly accept aggregations.

packages/runtime/test/schemas/name-mapping/models.ts (1)

8-11: LGTM: concise, generator-friendly type exports.

Types User and Post correctly alias ModelResult over the generated SchemaType. No runtime code, safe to keep generated.

packages/runtime/test/client-api/name-mapping.test.ts (2)

119-129: Nice coverage of unqualified vs qualified refs and self-refs.

The trio of queries (selectAll + unqualified where, qualified User.email, and whereRef('email','=','email')) exercises the name-mapper across common select patterns.


130-140: Join mapping through nested select-all is validated well.

innerJoin('User', 'User.id', 'Post.authorId') plus selecting from both sides ensures mapped table/columns are correctly surfaced via aliases.

packages/runtime/test/schemas/name-mapping/input.ts (4)

1-7: Generated file header and lint disable look good

Header clearly marks the file as generated and disables linting appropriately for emitted types.


11-29: User model argument/type exports look consistent and comprehensive

Aliases cover the expected CRUD, aggregation, groupBy, and shape helpers. Naming aligns with the rest of the runtime conventions.


31-49: Post model argument/type exports mirror User correctly

Symmetric coverage for Post maintains consistency and reduces surprises in the test harness.


8-50: Runtime type exports verified
All referenced types are correctly exported from packages/runtime/src/client/crud-types.ts, so no further changes are needed.

• SelectIncludeOmit (line 453)
• SimplifiedModelResult (line 181)
• FindManyArgs (line 664)
• FindUniqueArgs (line 667)
• AggregateArgs (line 1002)
• GroupByArgs (line 1089)
• WhereInput (line 207)
• SelectInput (line 459)
• IncludeInput (line 487)
• OmitInput (line 449)

@ymc9 ymc9 added this pull request to the merge queue Aug 24, 2025
Merged via the queue into main with commit c879a87 Aug 24, 2025
10 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Aug 26, 2025
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