Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Sep 9, 2025

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 9, 2025

📝 Walkthrough

Walkthrough

Replaces external Zod error formatter with a local cross-version helper, tightens several z.record key schemas, refactors Zod schema generation (DecimalSchema, centralized base-schema, addDefaults), hardens Prisma inputObjectTypes access, adds VS Code features (auth provider, docs cache, preview, release notes), bumps JetBrains plugin and updates LICENSE/tests.

Changes

Cohort / File(s) Summary of changes
Licensing
LICENSE, packages/LICENSE
Updated copyright line to (c) 2022-2025 ZenStack.
Build / IDE
packages/ide/jetbrains/build.gradle.kts
Bumped JetBrains IDE Gradle plugin version 2.18.1 → 2.19.0.
Zod error helper & local-helpers exports
packages/runtime/src/local-helpers/zod-utils.ts, packages/runtime/src/local-helpers/index.ts
Added getZodErrorMessage(err) to unify Zod v3/v4 formatting and re-exported it; reordered some exports.
Replace fromZodError usages
packages/runtime/src/validation.ts, packages/runtime/src/enhancements/node/policy/handler.ts, packages/runtime/src/enhancements/node/policy/policy-utils.ts, packages/server/src/api/rest/index.ts, packages/server/src/api/rpc/index.ts, packages/plugins/openapi/src/generator-base.ts
Replaced fromZodError(...)getZodErrorMessage(...), adjusted imports/typing; behavior unchanged aside from error text.
Zod lazy & utils
packages/runtime/src/zod-utils.ts
Made unwrapLazy robust: prefer unwrap(), fallback to .schema, otherwise throw.
Zod schema generator & helpers
packages/schema/src/plugins/zod/generator.ts, packages/schema/src/plugins/zod/utils/schema-gen.ts, packages/schema/src/plugins/zod/transformer.ts
Centralized base-schema creation (createModelBaseSchema), added addDefaults param to makeFieldSchema, refined DecimalSchema using z.any().refine(...), updated JSON record key schemas to explicit string keys, and improved delegate input-type name resolution.
Schema enhancer & generated client fixes
packages/schema/src/plugins/enhancer/enhance/index.ts
Null-safe iteration over inputObjectTypes.prisma, removed import $Types alias from generated statements, replaced $Types.Skip with runtime.Types.Skip, minor formatting.
OpenAPI generator & schema
packages/plugins/openapi/src/rpc-generator.ts, packages/plugins/openapi/src/schema.ts
Defensive access to inputObjectTypes.prisma ?? []; include AST enums in enum component generation; change record key schemas to z.record(z.string(), ...) and update oauth2 scopes to z.record(z.string(), z.string()).
Server REST/RPC payloads & validation
packages/server/src/api/rest/index.ts, packages/server/src/api/rpc/index.ts
Tightened createUpdatePayloadSchema.relationships and other maps to z.record(z.string(), ...); unified Zod error formatting via getZodErrorMessage; minor typing tweaks.
CLI config validation
packages/schema/src/cli/config.ts
Removed fromZodError usage; throw CLI errors using raw/converted error message.
Runtime policy checks
packages/runtime/src/enhancements/node/policy/handler.ts, packages/runtime/src/enhancements/node/policy/policy-utils.ts
Swapped to getZodErrorMessage(...) for Zod validation failures; small typing/import updates.
Local helpers re-exports
packages/runtime/src/local-helpers/index.ts
Reordered exports and added export * from './zod-utils'.
Schema extension: documentation & UI
packages/schema/src/documentation-cache.ts, packages/schema/src/zmodel-preview.ts, packages/schema/src/release-notes-manager.ts, packages/schema/src/res/zmodel-preview-release-notes.html, packages/schema/src/extension.ts
Added DocumentationCache, ZModelPreview, ReleaseNotesManager, static release-notes HTML; wired authentication and language-client startup, added exported requireAuth() and deactivate().
VS Code auth provider
packages/schema/src/zenstack-auth-provider.ts
New ZenStackAuthenticationProvider implementing VS Code AuthenticationProvider with browser sign-in/callback handling and secret persistence.
Zod generator/transformer regression & SDK fix
packages/sdk/src/utils.ts, tests/regression/tests/issue-2226.test.ts
Added sameField effective-owner comparison for backlink detection; added regression test for delegated subtype update-input (issue-2226).
Tests & deps / expectations
tests/integration/tests/cli/plugins.test.ts, tests/integration/tests/enhancements/with-password/with-password.test.ts, tests/integration/tests/enhancements/with-policy/field-validation.test.ts
Bumped Prisma versions in test scaffold (6.14.x → 6.16.x); simplified expected error-message tokens in multiple tests.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client
    participant Server
    participant Zod as ZodValidator
    participant Helper as getZodErrorMessage

    Client->>Server: send create/update payload
    Server->>Zod: validate payload with Zod schema
    alt valid
        Zod-->>Server: parsed result
        Server->>Client: success response
    else invalid
        Zod-->>Server: ZodError
        Server->>Helper: getZodErrorMessage(ZodError)
        Helper-->>Server: formatted message
        Server->>Client: 400 with formatted error message
    end
Loading
sequenceDiagram
    autonumber
    participant Editor
    participant LangServer as LanguageClient
    participant Preview as ZModelPreview
    participant Cache as DocumentationCache
    participant Auth as ZenStackAuth
    participant API

    Editor->>Preview: previewZModelFile()
    Preview->>LangServer: zenstack/getAllImportedZModelURIs (document URI)
    LangServer-->>Preview: importedURIs
    Preview->>Auth: requireAuth()
    Auth-->>Preview: auth session
    Preview->>Cache: getCachedResponse(models)
    alt cache hit
        Cache-->>Preview: cached HTML
    else cache miss
        Preview->>API: POST /api/doc with models + auth
        API-->>Preview: generated HTML
        Preview->>Cache: setCachedResponse(models, HTML)
    end
    Preview->>Editor: open Markdown preview (side)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Possibly related PRs

Pre-merge checks (2 passed, 1 inconclusive)

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description Check ❓ Inconclusive No pull request description was provided in the input, so I cannot determine whether the PR description is related to the changeset; the raw_summary exists but was not supplied as the PR description to evaluate. Please provide the PR description (or confirm that the raw_summary should be used as the description); if missing, add a brief description summarizing the merge intent and major changes (for example: version bump to v2.19.0, Zod error helper consolidation, schema and VS Code extension additions) so this check can be re-evaluated.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "merge dev to main (v2.19.0)" correctly and concisely describes the PR's purpose (merging dev into main and a version/release bump) and is directly related to the provided changeset; although it omits specific feature details, that brevity is acceptable for a release-style merge title.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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

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

Copy link
Contributor

@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: 1

Caution

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

⚠️ Outside diff range comments (2)
packages/schema/src/plugins/enhancer/enhance/index.ts (1)

482-498: Also process model-scoped inputObjectTypes, not just prisma-scoped.
Create/Update input types for models typically live under inputObjectTypes.model. Limiting to prisma may miss fixes for [Model]CreateInput etc.

-        for (const inputType of dmmf.schema.inputObjectTypes.prisma ?? []) {
+        const inputTypes =
+            [...(dmmf.schema.inputObjectTypes.model ?? []), ...(dmmf.schema.inputObjectTypes.prisma ?? [])];
+        for (const inputType of inputTypes) {
packages/plugins/openapi/src/schema.ts (1)

17-41: OAuth2 flows are over-restricted; make optional fields optional and require at least one flow.

Per OAS 3.0/3.1: description and refreshUrl are optional, and flows members (authorizationCode/implicit/password/clientCredentials) are optional with at least one required. Current schema requires all flows and refreshUrl, which can reject valid specs.

Apply optionally as a targeted update (outside the changed lines) to relax constraints:

// inside the oauth2 object
description: z.string().optional(),
flows: z
  .object({
    authorizationCode: z
      .object({
        authorizationUrl: z.string(),
        tokenUrl: z.string(),
        refreshUrl: z.string().optional(),
        scopes: z.record(z.string(), z.string()),
      })
      .optional(),
    implicit: z
      .object({
        authorizationUrl: z.string(),
        refreshUrl: z.string().optional(),
        scopes: z.record(z.string(), z.string()),
      })
      .optional(),
    password: z
      .object({
        tokenUrl: z.string(),
        refreshUrl: z.string().optional(),
        scopes: z.record(z.string(), z.string()),
      })
      .optional(),
    clientCredentials: z
      .object({
        tokenUrl: z.string(),
        refreshUrl: z.string().optional(),
        scopes: z.record(z.string(), z.string()),
      })
      .optional(),
  })
  .refine(
    (v) => !!(v.authorizationCode || v.implicit || v.password || v.clientCredentials),
    { message: 'At least one OAuth2 flow must be specified' }
  )
🧹 Nitpick comments (9)
packages/schema/src/cli/config.ts (1)

30-30: Prefer error.message over interpolating the whole error.
Interpolating the Error object can include noisy class names/stack. Using message is cleaner and avoids leaking internals.

-            throw new CliError(`Config file ${filename} is not valid: ${err}`);
+            throw new CliError(`Config file ${filename} is not valid: ${(err as ZodError).message}`);
packages/schema/src/plugins/enhancer/enhance/index.ts (1)

573-575: Use path.join for portability.
Hardcoded slashes can misbehave on Windows. path.join keeps it OS-safe.

-        const internalFilename = `${prismaClientDir}/internal/prismaNamespace.ts`;
-        const internalFilenameFixed = `${prismaClientDir}/internal/prismaNamespace-fixed.ts`;
+        const internalFilename = path.join(prismaClientDir, 'internal', 'prismaNamespace.ts');
+        const internalFilenameFixed = path.join(prismaClientDir, 'internal', 'prismaNamespace-fixed.ts');
packages/runtime/src/local-helpers/zod-utils.ts (1)

1-22: Make Zod v3/v4 detection more robust and avoid relying on private props.
'_zod' in err may be brittle; try v4 then fall back to v3, and early-return non-Zod errors.

 export function getZodErrorMessage(err: unknown): string {
     if (!(err instanceof Error)) {
         return 'Unknown error';
     }
 
     try {
-        if ('_zod' in err) {
-            return fromZodErrorV4(err as any).message;
-        } else {
-            return fromZodErrorV3(err as any).message;
-        }
+        const anyErr = err as any;
+        // Not a ZodError: return the original message
+        if (anyErr?.name !== 'ZodError' && !('issues' in anyErr) && !('errors' in anyErr)) {
+            return err.message;
+        }
+        // Prefer v4 formatter; fall back to v3
+        try {
+            return fromZodErrorV4(anyErr).message;
+        } catch {
+            return fromZodErrorV3(anyErr).message;
+        }
     } catch {
         return err.message;
     }
 }
packages/plugins/openapi/src/generator-base.ts (1)

92-99: Type the securitySchemes option to match the Zod schema.

getOption<Record<string, string>[]>('securitySchemes') suggests an array of string records, which doesn’t align with SecuritySchemesSchema (a record of security scheme objects). Safer to type as unknown (or the precise OpenAPI type) to avoid misleading consumers.

-        const securitySchemes = this.getOption<Record<string, string>[]>('securitySchemes');
+        const securitySchemes = this.getOption<unknown>('securitySchemes');

Optionally, narrow the return type:

protected generateSecuritySchemes(): OAPI.ComponentsObject['securitySchemes'] | undefined { /* ... */ }
packages/runtime/src/enhancements/node/policy/policy-utils.ts (1)

943-947: Normalize error message formatting (drop square brackets)

This path wraps the Zod message in brackets while other paths don’t. Suggest aligning for consistency.

- `entity ${formatObject(uniqueFilter, false)} failed validation: [${getZodErrorMessage(err)}]`,
+ `entity ${formatObject(uniqueFilter, false)} failed validation: ${getZodErrorMessage(err)}`,
packages/plugins/openapi/src/rpc-generator.ts (3)

928-931: Unreachable TypeDef check in JSON branch

Inside prismaTypeToOpenAPIType, type is 'JSON'/'Json', so comparing against TypeDef names is ineffective. Simplify to return {} here.

-            .with(P.union('JSON', 'Json'), () => {
-                // For Json fields, check if there's a specific TypeDef reference
-                // Otherwise, return empty schema for arbitrary JSON
-                const isTypeDefType = this.model.declarations.some((d) => isTypeDef(d) && d.name === type);
-                return isTypeDefType ? this.ref(type, false) : {};
-            })
+            .with(P.union('JSON', 'Json'), () => ({} as OAPI.SchemaObject))

374-377: Use strict equality

Minor consistency nit.

-                security: create === true && update == true ? [] : undefined,
+                security: create === true && update === true ? [] : undefined,

127-129: Typo in warning message

Extra closing brace.

-                this.warnings.push(`Unable to load ZModel definition for: ${model.name}}`);
+                this.warnings.push(`Unable to load ZModel definition for: ${model.name}`);
packages/server/src/api/rest/index.ts (1)

1967-1969: Avoid leaking stack traces in API responses

Consider omitting stack in production or behind a debug flag.

-            return this.makeError('unknownError', `${_err.message}\n${_err.stack}`);
+            const detail = process.env.NODE_ENV === 'production' ? _err.message : `${_err.message}\n${_err.stack}`;
+            return this.makeError('unknownError', detail);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 789e772 and 25da0c7.

⛔ Files ignored due to path filters (23)
  • package.json is excluded by !**/*.json
  • packages/ide/jetbrains/package.json is excluded by !**/*.json
  • packages/language/package.json is excluded by !**/*.json
  • packages/misc/redwood/package.json is excluded by !**/*.json
  • packages/plugins/openapi/package.json is excluded by !**/*.json
  • packages/plugins/swr/package.json is excluded by !**/*.json
  • packages/plugins/tanstack-query/package.json is excluded by !**/*.json
  • packages/plugins/trpc/package.json is excluded by !**/*.json
  • packages/plugins/trpc/tests/projects/nuxt-trpc-v10/package.json is excluded by !**/*.json
  • packages/plugins/trpc/tests/projects/nuxt-trpc-v11/package.json is excluded by !**/*.json
  • packages/plugins/trpc/tests/projects/t3-trpc-v10/package.json is excluded by !**/*.json
  • packages/plugins/trpc/tests/projects/t3-trpc-v11/package-lock.json is excluded by !**/package-lock.json, !**/*.json
  • packages/plugins/trpc/tests/projects/t3-trpc-v11/package.json is excluded by !**/*.json
  • packages/runtime/package.json is excluded by !**/*.json
  • packages/schema/package.json is excluded by !**/*.json
  • packages/sdk/package.json is excluded by !**/*.json
  • packages/server/package.json is excluded by !**/*.json
  • packages/testtools/package.json is excluded by !**/*.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/*.yaml
  • pnpm-workspace.yaml is excluded by !**/*.yaml
  • tests/integration/test-run/package.json is excluded by !**/*.json
  • tests/integration/tests/frameworks/nextjs/test-project/package.json is excluded by !**/*.json
  • tests/integration/tests/frameworks/trpc/test-project/package.json is excluded by !**/*.json
📒 Files selected for processing (22)
  • LICENSE (1 hunks)
  • packages/LICENSE (1 hunks)
  • packages/ide/jetbrains/build.gradle.kts (1 hunks)
  • packages/plugins/openapi/src/generator-base.ts (2 hunks)
  • packages/plugins/openapi/src/rpc-generator.ts (6 hunks)
  • packages/plugins/openapi/src/schema.ts (2 hunks)
  • packages/runtime/src/enhancements/node/policy/handler.ts (3 hunks)
  • packages/runtime/src/enhancements/node/policy/policy-utils.ts (3 hunks)
  • packages/runtime/src/local-helpers/index.ts (1 hunks)
  • packages/runtime/src/local-helpers/zod-utils.ts (1 hunks)
  • packages/runtime/src/validation.ts (2 hunks)
  • packages/runtime/src/zod-utils.ts (1 hunks)
  • packages/schema/src/cli/config.ts (1 hunks)
  • packages/schema/src/plugins/enhancer/enhance/index.ts (2 hunks)
  • packages/schema/src/plugins/zod/generator.ts (10 hunks)
  • packages/schema/src/plugins/zod/transformer.ts (1 hunks)
  • packages/schema/src/plugins/zod/utils/schema-gen.ts (2 hunks)
  • packages/server/src/api/rest/index.ts (6 hunks)
  • packages/server/src/api/rpc/index.ts (3 hunks)
  • tests/integration/tests/cli/plugins.test.ts (2 hunks)
  • tests/integration/tests/enhancements/with-password/with-password.test.ts (2 hunks)
  • tests/integration/tests/enhancements/with-policy/field-validation.test.ts (16 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
packages/plugins/openapi/src/generator-base.ts (3)
packages/sdk/src/types.ts (1)
  • PluginError (115-119)
packages/plugins/openapi/src/index.ts (1)
  • name (5-5)
packages/runtime/src/local-helpers/zod-utils.ts (1)
  • getZodErrorMessage (8-22)
packages/runtime/src/validation.ts (1)
packages/runtime/src/local-helpers/zod-utils.ts (1)
  • getZodErrorMessage (8-22)
packages/schema/src/cli/config.ts (1)
packages/schema/src/cli/cli-error.ts (1)
  • CliError (4-4)
packages/server/src/api/rpc/index.ts (1)
packages/runtime/src/local-helpers/zod-utils.ts (1)
  • getZodErrorMessage (8-22)
packages/runtime/src/enhancements/node/policy/policy-utils.ts (1)
packages/runtime/src/local-helpers/zod-utils.ts (1)
  • getZodErrorMessage (8-22)
packages/runtime/src/enhancements/node/policy/handler.ts (1)
packages/runtime/src/local-helpers/zod-utils.ts (1)
  • getZodErrorMessage (8-22)
packages/server/src/api/rest/index.ts (1)
packages/runtime/src/local-helpers/zod-utils.ts (1)
  • getZodErrorMessage (8-22)
packages/plugins/openapi/src/rpc-generator.ts (2)
packages/runtime/src/local-helpers/upper-case-first.ts (1)
  • upperCaseFirst (1-3)
packages/language/src/ast.ts (1)
  • DataModel (56-66)
packages/schema/src/plugins/zod/generator.ts (1)
packages/schema/src/plugins/zod/utils/schema-gen.ts (1)
  • makeFieldSchema (17-165)
⏰ 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). (5)
  • GitHub Check: dependency-review
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
  • GitHub Check: OSSAR-Scan
🔇 Additional comments (30)
LICENSE (1)

3-3: LGTM: year range updated.

packages/ide/jetbrains/build.gradle.kts (1)

12-12: Version bump aligned with release tag.
Project version set to 2.19.0; no other changes. Looks good.

packages/LICENSE (1)

3-3: LGTM: year range updated.

packages/runtime/src/zod-utils.ts (1)

124-134: LGTM: robust lazy unwrap across zod versions.
Handles unwrap() and .schema with a clear fallback error.

packages/plugins/openapi/src/schema.ts (1)

7-7: Switch to string-keyed records is correct for OpenAPI/OAuth scopes.

Using z.record(z.string(), ...) aligns with OpenAPI’s string-keyed maps and avoids permissive key types. Looks good.

Also applies to: 24-24, 29-29, 34-34, 39-39

packages/plugins/openapi/src/generator-base.ts (1)

1-1: Standardized Zod error formatting via getZodErrorMessage.

Replacement is straightforward and reduces dependency surface. Good change.

Also applies to: 97-98

tests/integration/tests/enhancements/with-password/with-password.test.ts (1)

97-107: Test expectation updates look good.

Shorter token-based messages keep tests stable across formatter tweaks.

packages/schema/src/plugins/zod/transformer.ts (1)

489-491: Allow top-level null for Prisma InputJsonValue.

Top-level null is valid JSON/Prisma input but is currently rejected. Include z.null() in the union.

-  jsonSchemaImplementation += `  z.union([literalSchema, z.array(jsonSchema.nullable()), z.record(z.string(), jsonSchema.nullable())])\n`;
+  jsonSchemaImplementation += `  z.union([literalSchema, z.null(), z.array(jsonSchema.nullable()), z.record(z.string(), jsonSchema.nullable())])\n`;

Likely an incorrect or invalid review comment.

packages/server/src/api/rpc/index.ts (1)

10-10: Good migration to unified Zod error messages.

Importing and using getZodErrorMessage centralizes formatting; loosening ZodError<any> is acceptable here.

Also applies to: 204-206, 254-256

packages/runtime/src/validation.ts (1)

2-2: Consistent error messaging via getZodErrorMessage.

Change is correct and localized.

Also applies to: 18-19

packages/runtime/src/enhancements/node/policy/handler.ts (1)

17-17: Policy validation now uses centralized Zod error formatter.

The updates improve consistency; explicit any for validatedData is reasonable in this context.

Also applies to: 423-425, 1265-1273

packages/schema/src/plugins/zod/utils/schema-gen.ts (1)

17-26: String-keyed records and configurable defaults are sensible improvements.

  • z.record(z.string(), z.unknown()) clarifies object key types.
  • addDefaults flag gives finer control during generation.

Also applies to: 146-156

packages/runtime/src/enhancements/node/policy/policy-utils.ts (2)

16-22: Centralized Zod error formatting import looks good

Switching to getZodErrorMessage via local-helpers keeps error handling consistent across the codebase.


1446-1449: LGTM: informative validation log

Using getZodErrorMessage here is appropriate and consistent.

packages/runtime/src/local-helpers/index.ts (1)

4-8: Good re-export of zod-utils

Exporting zod-utils here makes the helper easy to consume; re-ordering is harmless.

packages/plugins/openapi/src/rpc-generator.ts (3)

60-61: Defensive read of DMMF input types

Null-safe default avoids runtime crashes when Prisma omits inputObjectTypes.prisma.


662-670: Cover enums declared only in AST

Adding AST enums fills gaps not present in DMMF. Nice.


777-807: TypeDef-aware JSON field refs — nice improvement

Detecting Json fields pointing to TypeDefs and referencing the proper component improves schema fidelity.

tests/integration/tests/enhancements/with-policy/field-validation.test.ts (1)

68-69: Verify toBeRejectedByPolicy matcher semantics
Ensure the custom matcher checks for inclusion of each expected token (i.e. “contains tokens”) rather than strict equality of the entire error message. Confirm its implementation aligns with the updated tokenized Zod output.

packages/server/src/api/rest/index.ts (5)

14-15: Consistent error messaging via getZodErrorMessage

Aligns REST errors with the new formatter.


203-211: JSON:API relationships as string-keyed records — good

z.record(z.string(), ...) better reflects relationship maps.


824-829: 422 payload validation detail

Returning DATA_VALIDATION_VIOLATION with the raw ZodError preserves useful context; message text via getZodErrorMessage is user-friendly.


1023-1030: Relationship payload validation paths updated

Good use of getZodErrorMessage for detailed 422s.

Also applies to: 1054-1061


1977-1978: Type param on ZodError is fine

Using ZodError avoids TS friction across zod v3/v4.

packages/schema/src/plugins/zod/generator.ts (6)

102-107: Null-safe Prisma input types filter

Using (…prisma ?? []) prevents crashes with older/newer Prisma DMMF shapes.


439-446: Base schema helper reduces duplication

createModelBaseSchema with addDefaults=true/false is a clean refactor.

Also applies to: 682-707


543-546: PrismaCreateSchema from base without defaults

Using baseSchemaWithoutDefaults + partial + passthrough avoids accidental defaults during create validation. Good.


563-566: Atomic ops support for numeric fields in update

Union with z.record(z.string(), z.unknown()) covers { increment: … } etc.


586-601: Create/Update Scalar schemas exported (naming matches runtime usage)

Names match ${Model}CreateScalarSchema / ${Model}UpdateScalarSchema expected by REST handler.

Also applies to: 639-641


218-235: Add tests to verify DecimalSchema accepts Decimal instances
Confirm in unit tests that the refined DecimalSchema correctly accepts actual Decimal.js instances (in addition to strings and numbers) and rejects invalid shapes.

Copy link
Contributor

@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: 1

🧹 Nitpick comments (3)
packages/sdk/src/utils.ts (1)

662-669: Prefer comparing by the ultimate delegate base to avoid false negatives across multi-level inheritance

Using $inheritedFrom ?? $container can misclassify “same field” when an intermediate non-delegate sits between a delegate base and the concrete model. Compare by the original delegate owner when available.

Apply:

-    const sameField = (f1: DataModelField, f2: DataModelField) => {
-        // for fields inherited from a delegate model, always use
-        // the base to compare
-        const parent1 = f1.$inheritedFrom ?? f1.$container;
-        const parent2 = f2.$inheritedFrom ?? f2.$container;
-        return f1.name === f2.name && parent1 === parent2;
-    };
+    const effectiveOwner = (f: DataModelField) =>
+        getInheritedFromDelegate(f) ?? f.$inheritedFrom ?? f.$container;
+    const sameField = (f1: DataModelField, f2: DataModelField) =>
+        f1.name === f2.name && effectiveOwner(f1) === effectiveOwner(f2);

Please verify with a test where: Delegate -> Abstract -> Concrete, and the counterpart model inherits the same-named field from the same delegate through a different intermediate base.

tests/regression/tests/issue-2226.test.ts (1)

26-29: Add a negative assertion to guard against accidental passthrough fields

Ensure unrelated fields are rejected, catching regressions where .passthrough() sneaks in.

         const parsed = schema.safeParse({
             replacedRegistrationId: '123',
         });
         expect(parsed.success).toBe(true);
+        expect(schema.safeParse({ foo: 'bar' }).success).toBe(false);
packages/schema/src/plugins/zod/transformer.ts (1)

345-360: Robust fallback mapping for delegated update inputs looks good; consider caching for perf

The candidate probing fixes Prisma’s inconsistent Update/Nested naming. To trim repeated O(n) scans over inputObjectTypes, cache names in a Set.

Within this hunk:

-            const finalMappedName =
-                candidates.find((name) => this.inputObjectTypes.some((it) => it.name === name)) ?? mappedInputTypeName;
+            const finalMappedName =
+                candidates.find((name) => this.inputObjectTypeNames.has(name)) ?? mappedInputTypeName;

And elsewhere (outside this hunk):

// add a field
private inputObjectTypeNames: Set<string> = new Set();

// in constructor after assigning this.inputObjectTypes
this.inputObjectTypeNames = new Set(this.inputObjectTypes.map((i) => i.name));
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25da0c7 and 8962ad9.

📒 Files selected for processing (3)
  • packages/schema/src/plugins/zod/transformer.ts (2 hunks)
  • packages/sdk/src/utils.ts (1 hunks)
  • tests/regression/tests/issue-2226.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/regression/tests/issue-2226.test.ts (1)
packages/testtools/src/schema.ts (1)
  • loadSchema (172-248)
⏰ 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). (6)
  • GitHub Check: dependency-review
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
  • GitHub Check: OSSAR-Scan
  • GitHub Check: OSSAR-Scan
🔇 Additional comments (2)
packages/sdk/src/utils.ts (1)

671-671: LGTM: self-skip now robust across inherited clones

Replacing otherField === field with a structural “same field” check fixes false backlink matches when fields are cloned via inheritance.

packages/schema/src/plugins/zod/transformer.ts (1)

503-504: Explicit record key schema for JSON is correct

Switching to z.record(z.string(), jsonSchema.nullable()) aligns with Zod’s explicit string-key records and Prisma’s InputJsonValue shape.

Copy link
Contributor

@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

♻️ Duplicate comments (1)
tests/integration/tests/cli/plugins.test.ts (1)

72-79: Prisma 6.16.x bump: re-verify enhancer overloads (DynamicClientExtensionThis) still match arity.

This update can break our enhancer type-guards if they assume the older generic-arity. Please reconfirm the enhancer and any generated types use the 4‑generic signature so tsc stays clean.

Run to check usage and arity across the repo:

#!/bin/bash
set -euo pipefail

# All references
rg -nP --type=ts -C2 '\bDynamicClientExtensionThis\b'

echo
echo '=== 3-arg occurrences (should be 0) ==='
rg -nP --type=ts '\bDynamicClientExtensionThis\s*<\s*[^,>]+,\s*[^,>]+,\s*[^,>]+\s*>'

echo
echo '=== 4-arg occurrences (expected) ==='
rg -nP --type=ts '\bDynamicClientExtensionThis\s*<\s*[^,>]+,\s*[^,>]+,\s*[^,>]+,\s*[^,>]+\s*>'

Expected: zero 3‑arg matches; at least one 4‑arg match where applicable.

🧹 Nitpick comments (1)
tests/integration/tests/cli/plugins.test.ts (1)

78-88: Optional: pin exact patch or rely strictly on lockfile for CI determinism.

Using 6.16.x can pick up future patch releases and cause flaky CI if Prisma ships type changes. Consider pinning exact patches or ensuring the generated package-lock.json is committed and installs via npm ci in the test harness.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 87083c0 and d6da6cf.

⛔ Files ignored due to path filters (11)
  • packages/plugins/trpc/tests/projects/nuxt-trpc-v10/package.json is excluded by !**/*.json
  • packages/plugins/trpc/tests/projects/nuxt-trpc-v11/package.json is excluded by !**/*.json
  • packages/plugins/trpc/tests/projects/t3-trpc-v11/package-lock.json is excluded by !**/package-lock.json, !**/*.json
  • packages/plugins/trpc/tests/projects/t3-trpc-v11/package.json is excluded by !**/*.json
  • packages/runtime/package.json is excluded by !**/*.json
  • packages/schema/package.json is excluded by !**/*.json
  • packages/sdk/package.json is excluded by !**/*.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/*.yaml
  • tests/integration/test-run/package.json is excluded by !**/*.json
  • tests/integration/tests/frameworks/nextjs/test-project/package.json is excluded by !**/*.json
  • tests/integration/tests/frameworks/trpc/test-project/package.json is excluded by !**/*.json
📒 Files selected for processing (1)
  • tests/integration/tests/cli/plugins.test.ts (2 hunks)
⏰ 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). (6)
  • GitHub Check: build-test (20.x)
  • GitHub Check: dependency-review
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
  • GitHub Check: OSSAR-Scan
  • GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
tests/integration/tests/cli/plugins.test.ts (1)

85-89: LGTM: client/CLI versions aligned.

@prisma/client@6.16.x and prisma@6.16.x are in lockstep, which avoids generator/runtime skew.

Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
Copy link
Contributor

@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: 6

🧹 Nitpick comments (10)
packages/schema/src/res/zmodel-preview-release-notes.html (1)

66-68: Minor: Inconsistent non-breaking space usage in keyboard shortcuts.

The Mac shortcut uses &nbsp; between key combinations while the Windows shortcut doesn't. Consider applying consistent formatting.

-                    <code>Cmd&nbsp;+&nbsp;Shift&nbsp;+&nbsp;V</code> (Mac) or
-                    <code>Ctrl&nbsp;+&nbsp;Shift&nbsp;+&nbsp;V</code> (Windows)
+                    <code>Cmd&nbsp;+&nbsp;Shift&nbsp;+&nbsp;V</code> (Mac) or
+                    <code>Ctrl&nbsp;+&nbsp;Shift&nbsp;+&nbsp;V</code> (Windows/Linux)
packages/schema/src/release-notes-manager.ts (1)

18-20: Consider making the initialization asynchronous.

The initialize() method calls an async function without awaiting it, which could lead to unhandled promise rejections.

-    initialize(): void {
-        this.showReleaseNotesIfFirstTime();
+    async initialize(): Promise<void> {
+        await this.showReleaseNotesIfFirstTime();

And update the constructor:

     constructor(context: vscode.ExtensionContext) {
         this.extensionContext = context;
-        this.initialize();
+        void this.initialize();
packages/schema/src/zenstack-auth-provider.ts (2)

153-155: Remove unused generateState() method.

This private method is not referenced anywhere in the code.

-    private generateState(): string {
-        return Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15);
-    }

114-114: Use strict equality for string comparison.

-                    const isCursor = vscode.env.appName == 'Cursor';
+                    const isCursor = vscode.env.appName === 'Cursor';
packages/schema/src/language-server/main.ts (1)

38-45: Consider extracting the error severity constant.

The magic number 1 for error severity should be a named constant for better readability.

+        const ERROR_SEVERITY = 1; // DiagnosticSeverity.Error
         const hasSyntaxErrors = [uri, ...importedURIs].some((uri) => {
             const doc = langiumDocuments.getOrCreateDocument(uri);
             return (
                 doc.parseResult.lexerErrors.length > 0 ||
                 doc.parseResult.parserErrors.length > 0 ||
-                doc.diagnostics?.some((e) => e.severity === 1)
+                doc.diagnostics?.some((e) => e.severity === ERROR_SEVERITY)
             );
         });
packages/schema/src/zmodel-preview.ts (4)

242-247: Avoid temp file name collisions.

Running multiple previews for the same base file can overwrite the same temp path.

Apply this diff:

-        const tempFileName = `${baseName}-preview.md`;
-        const tempFilePath = path.join(os.tmpdir(), tempFileName);
+        const tempFileName = `${baseName}-preview-${Date.now()}.md`;
+        const tempFilePath = path.join(os.tmpdir(), tempFileName);

81-85: Prefer languageId check over filename suffix.

More robust for untitled or virtual documents.

Apply this diff:

-        if (!document.fileName.endsWith('.zmodel')) {
+        if (document.languageId !== 'zmodel') {
             vscode.window.showErrorMessage('The active file is not a ZModel file.');
             return;
         }

87-92: Avoid double interactive auth prompts by threading the session.

You already require auth in previewZModelFile; pass the session into generateZModelDocumentation to prevent a second prompt.

If you want, I can provide a small refactor patch to thread the session through the call chain.

Also applies to: 184-188


220-224: Optional: handle 401 by re-auth and one retry.

If the token expires, prompt once to refresh and retry the POST.

I can add a guarded 401 → re-auth → retry flow with backoff if you’d like.

packages/schema/src/documentation-cache.ts (1)

80-83: Use an OutputChannel for logs instead of console.log.

Improves discoverability and avoids console noise.

I can wire a shared vscode.OutputChannel (e.g., “ZenStack”) and route these logs there.

Also applies to: 131-133, 150-151

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6da6cf and 80ebb55.

⛔ Files ignored due to path filters (2)
  • packages/schema/package.json is excluded by !**/*.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/*.yaml
📒 Files selected for processing (7)
  • packages/schema/src/documentation-cache.ts (1 hunks)
  • packages/schema/src/extension.ts (1 hunks)
  • packages/schema/src/language-server/main.ts (1 hunks)
  • packages/schema/src/release-notes-manager.ts (1 hunks)
  • packages/schema/src/res/zmodel-preview-release-notes.html (1 hunks)
  • packages/schema/src/zenstack-auth-provider.ts (1 hunks)
  • packages/schema/src/zmodel-preview.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/schema/src/language-server/main.ts (2)
packages/schema/src/language-server/zmodel-module.ts (1)
  • createZModelServices (129-138)
packages/schema/src/cli/cli-util.ts (1)
  • eagerLoadAllImports (146-168)
packages/schema/src/zmodel-preview.ts (3)
packages/schema/src/documentation-cache.ts (1)
  • DocumentationCache (15-152)
packages/schema/src/extension.ts (1)
  • requireAuth (14-36)
packages/schema/src/zenstack-auth-provider.ts (1)
  • API_URL (13-13)
packages/schema/src/extension.ts (4)
packages/schema/src/zenstack-auth-provider.ts (2)
  • AUTH_PROVIDER_ID (11-11)
  • ZenStackAuthenticationProvider (15-257)
packages/schema/src/documentation-cache.ts (1)
  • DocumentationCache (15-152)
packages/schema/src/zmodel-preview.ts (1)
  • ZModelPreview (14-302)
packages/schema/src/release-notes-manager.ts (1)
  • ReleaseNotesManager (6-77)
⏰ 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). (6)
  • GitHub Check: build-test (20.x)
  • GitHub Check: OSSAR-Scan
  • GitHub Check: build-test (20.x)
  • GitHub Check: dependency-review
  • GitHub Check: build-test (20.x)
  • GitHub Check: OSSAR-Scan
🔇 Additional comments (10)
packages/schema/src/res/zmodel-preview-release-notes.html (1)

1-91: LGTM! Well-structured release notes HTML.

The release notes page is clean and follows VS Code theming patterns correctly. The documentation preview feature is well-explained with clear instructions.

packages/schema/src/release-notes-manager.ts (1)

6-77: LGTM! Clean implementation of release notes management.

The class properly handles one-time display, state persistence, and cross-machine synchronization.

packages/schema/src/zenstack-auth-provider.ts (1)

15-46: LGTM! Well-structured authentication provider implementation.

The authentication provider follows VS Code's authentication API patterns correctly with proper session management and disposal.

packages/schema/src/language-server/main.ts (1)

15-58: LGTM! Well-implemented LSP request handler.

The handler correctly manages document building, import resolution, and syntax error detection across the document graph.

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

14-36: LGTM! Clean authentication utility function.

The requireAuth() function provides a good user experience with clear prompts and error handling.


60-101: LGTM! Well-structured language client initialization.

The language client setup is properly configured with debugging support and file system watching.

packages/schema/src/zmodel-preview.ts (2)

53-68: Command registration looks solid.

Good separation of preview and cache-clear commands and proper disposal via context.subscriptions.


263-294: Mermaid extension hint UX is good.

Nice optional prompt + “Don’t show again” persisted in user settings.

packages/schema/src/documentation-cache.ts (2)

56-64: Confirm whitespace-stripping intent for cache keys.

Removing all whitespace may intentionally coalesce format-only diffs, but it also merges semantically distinct content that differs only in string literals/regex whitespace. Confirm this trade-off.


1-152: Cache implementation LGTM.

Clear TTL/version invalidation, stable keying, and Settings Sync integration look good.

@ymc9 ymc9 merged commit f8847f8 into main Sep 12, 2025
14 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.

4 participants