-
-
Couldn't load subscription status.
- Fork 126
merge dev to main (v2.19.0) #2237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughReplaces 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
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
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)
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)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Comment |
There was a problem hiding this 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 underinputObjectTypes.model. Limiting toprismamay miss fixes for[Model]CreateInputetc.- 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:
descriptionandrefreshUrlare optional, andflowsmembers (authorizationCode/implicit/password/clientCredentials) are optional with at least one required. Current schema requires all flows andrefreshUrl, 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 errmay 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 withSecuritySchemesSchema(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 branchInside 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 equalityMinor consistency nit.
- security: create === true && update == true ? [] : undefined, + security: create === true && update === true ? [] : undefined,
127-129: Typo in warning messageExtra 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 responsesConsider 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
⛔ Files ignored due to path filters (23)
-
package.jsonis excluded by!**/*.json -
packages/ide/jetbrains/package.jsonis excluded by!**/*.json -
packages/language/package.jsonis excluded by!**/*.json -
packages/misc/redwood/package.jsonis excluded by!**/*.json -
packages/plugins/openapi/package.jsonis excluded by!**/*.json -
packages/plugins/swr/package.jsonis excluded by!**/*.json -
packages/plugins/tanstack-query/package.jsonis excluded by!**/*.json -
packages/plugins/trpc/package.jsonis excluded by!**/*.json -
packages/plugins/trpc/tests/projects/nuxt-trpc-v10/package.jsonis excluded by!**/*.json -
packages/plugins/trpc/tests/projects/nuxt-trpc-v11/package.jsonis excluded by!**/*.json -
packages/plugins/trpc/tests/projects/t3-trpc-v10/package.jsonis excluded by!**/*.json -
packages/plugins/trpc/tests/projects/t3-trpc-v11/package-lock.jsonis excluded by!**/package-lock.json,!**/*.json -
packages/plugins/trpc/tests/projects/t3-trpc-v11/package.jsonis excluded by!**/*.json -
packages/runtime/package.jsonis excluded by!**/*.json -
packages/schema/package.jsonis excluded by!**/*.json -
packages/sdk/package.jsonis excluded by!**/*.json -
packages/server/package.jsonis excluded by!**/*.json -
packages/testtools/package.jsonis excluded by!**/*.json -
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml -
pnpm-workspace.yamlis excluded by!**/*.yaml -
tests/integration/test-run/package.jsonis excluded by!**/*.json -
tests/integration/tests/frameworks/nextjs/test-project/package.jsonis excluded by!**/*.json -
tests/integration/tests/frameworks/trpc/test-project/package.jsonis 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.
Handlesunwrap()and.schemawith 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-levelnullfor Prisma InputJsonValue.Top-level
nullis valid JSON/Prisma input but is currently rejected. Includez.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
getZodErrorMessagecentralizes formatting; looseningZodError<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
anyforvalidatedDatais 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.addDefaultsflag 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 goodSwitching to getZodErrorMessage via local-helpers keeps error handling consistent across the codebase.
1446-1449: LGTM: informative validation logUsing getZodErrorMessage here is appropriate and consistent.
packages/runtime/src/local-helpers/index.ts (1)
4-8: Good re-export of zod-utilsExporting 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 typesNull-safe default avoids runtime crashes when Prisma omits inputObjectTypes.prisma.
662-670: Cover enums declared only in ASTAdding AST enums fills gaps not present in DMMF. Nice.
777-807: TypeDef-aware JSON field refs — nice improvementDetecting 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 getZodErrorMessageAligns REST errors with the new formatter.
203-211: JSON:API relationships as string-keyed records — goodz.record(z.string(), ...) better reflects relationship maps.
824-829: 422 payload validation detailReturning DATA_VALIDATION_VIOLATION with the raw ZodError preserves useful context; message text via getZodErrorMessage is user-friendly.
1023-1030: Relationship payload validation paths updatedGood use of getZodErrorMessage for detailed 422s.
Also applies to: 1054-1061
1977-1978: Type param on ZodError is fineUsing ZodError avoids TS friction across zod v3/v4.
packages/schema/src/plugins/zod/generator.ts (6)
102-107: Null-safe Prisma input types filterUsing (…prisma ?? []) prevents crashes with older/newer Prisma DMMF shapes.
439-446: Base schema helper reduces duplicationcreateModelBaseSchema with addDefaults=true/false is a clean refactor.
Also applies to: 682-707
543-546: PrismaCreateSchema from base without defaultsUsing baseSchemaWithoutDefaults + partial + passthrough avoids accidental defaults during create validation. Good.
563-566: Atomic ops support for numeric fields in updateUnion 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 refinedDecimalSchemacorrectly accepts actual Decimal.js instances (in addition to strings and numbers) and rejects invalid shapes.
There was a problem hiding this 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 inheritanceUsing
$inheritedFrom ?? $containercan 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 fieldsEnsure 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 perfThe candidate probing fixes Prisma’s inconsistent Update/Nested naming. To trim repeated O(n) scans over
inputObjectTypes, cache names in aSet.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
📒 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 clonesReplacing
otherField === fieldwith 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 correctSwitching to
z.record(z.string(), jsonSchema.nullable())aligns with Zod’s explicit string-key records and Prisma’sInputJsonValueshape.
There was a problem hiding this 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
tscstays 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.xcan pick up future patch releases and cause flaky CI if Prisma ships type changes. Consider pinning exact patches or ensuring the generatedpackage-lock.jsonis committed and installs vianpm ciin the test harness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
packages/plugins/trpc/tests/projects/nuxt-trpc-v10/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/nuxt-trpc-v11/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/t3-trpc-v11/package-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackages/plugins/trpc/tests/projects/t3-trpc-v11/package.jsonis excluded by!**/*.jsonpackages/runtime/package.jsonis excluded by!**/*.jsonpackages/schema/package.jsonis excluded by!**/*.jsonpackages/sdk/package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yamltests/integration/test-run/package.jsonis excluded by!**/*.jsontests/integration/tests/frameworks/nextjs/test-project/package.jsonis excluded by!**/*.jsontests/integration/tests/frameworks/trpc/test-project/package.jsonis 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.xandprisma@6.16.xare in lockstep, which avoids generator/runtime skew.
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
There was a problem hiding this 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
between key combinations while the Windows shortcut doesn't. Consider applying consistent formatting.- <code>Cmd + Shift + V</code> (Mac) or - <code>Ctrl + Shift + V</code> (Windows) + <code>Cmd + Shift + V</code> (Mac) or + <code>Ctrl + Shift + 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 unusedgenerateState()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
1for 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
⛔ Files ignored due to path filters (2)
packages/schema/package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis 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.
No description provided.