merge dev to main (v3.0.0-beta.7) #272
Conversation
…269) * fix(language): ref resolution failure causes exception in validator * fix lint * update * fix tests
Co-authored-by: ymc9 <[email protected]>
* fix(runtime): policy plugin exports * update
WalkthroughVersion updates across multiple packages and samples. Language package adds ZModelDocumentBuilder, adjusts module wiring, relaxes linking assumptions in utils, and runs validators unconditionally. Runtime package adjusts exports paths for policy plugin. Minor test name/content tweaks. VS Code extension version bumped. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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 |
|
Claude finished @ymc9's task —— View job PR Review - merge dev to mainI've completed a comprehensive review of this PR. Here's my analysis: ✅ Summary: APPROVED - High Quality ChangesThis PR represents a solid merge from dev to main with important bug fixes and proper version management. 📋 Detailed ReviewVersion Management ✅ Excellent
Critical Bug Fixes ⭐ High Impact1. Language Module Robustness (commit
|
There was a problem hiding this comment.
Pull Request Overview
This PR merges changes from the dev branch to main, primarily consisting of version bumps across all packages from 3.0.0-beta.6 to 3.0.0-beta.7, along with several code improvements to the language processing system.
Key changes include:
- Version updates across all packages and samples
- Enhanced validation system with custom document builder
- Improved error handling by removing dependency on invariant assertions
- Package export path corrections for the policy plugin
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Root package version bump to 3.0.0-beta.7 |
| packages/*/package.json | Version updates across all packages to 3.0.0-beta.7 |
| packages/runtime/package.json | Export path fixes for policy plugin and version bump |
| packages/language/src/zmodel-document-builder.ts | New custom document builder with validation options |
| packages/language/src/validator.ts | Simplified validator by removing shouldCheck method |
| packages/language/src/utils.ts | Improved error handling without invariant assertions |
| packages/language/src/module.ts | Integration of new document builder |
| packages/language/test/expression-validation.test.ts | Test improvements with better naming and missing field addition |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/language/test/expression-validation.test.ts (2)
5-5: Rename test to be descriptive instead of suffixed numerals.
E.g., “should reject model-to-model comparison (relation field)” for clarity.
26-26: Same here: prefer a descriptive test name.
Avoid numeric suffixes in test titles.packages/language/src/validator.ts (1)
51-85: Unconditional validations: reuse validator instances and confirm gating by DocumentBuilder.
- To reduce allocations and allow potential state reuse, instantiate validators once and reuse in methods.
- Verify that callers always go through the new DocumentBuilder so validation doesn’t run after lexing/parsing/linking errors.
Proposed changes within this file:
export class ZModelValidator { - constructor(protected readonly services: ZModelServices) {} + constructor(protected readonly services: ZModelServices) {} + private readonly schemaValidator = new SchemaValidator(this.services.shared.workspace.LangiumDocuments); + private readonly dataSourceValidator = new DataSourceValidator(); + private readonly dataModelValidator = new DataModelValidator(); + private readonly typeDefValidator = new TypeDefValidator(); + private readonly enumValidator = new EnumValidator(); + private readonly attributeValidator = new AttributeValidator(); + private readonly expressionValidator = new ExpressionValidator(); + private readonly functionInvocationValidator = new FunctionInvocationValidator(); + private readonly functionDeclValidator = new FunctionDeclValidator(); checkModel(node: Model, accept: ValidationAcceptor): void { - new SchemaValidator(this.services.shared.workspace.LangiumDocuments).validate(node, accept); + this.schemaValidator.validate(node, accept); } checkDataSource(node: DataSource, accept: ValidationAcceptor): void { - new DataSourceValidator().validate(node, accept); + this.dataSourceValidator.validate(node, accept); } checkDataModel(node: DataModel, accept: ValidationAcceptor): void { - new DataModelValidator().validate(node, accept); + this.dataModelValidator.validate(node, accept); } checkTypeDef(node: TypeDef, accept: ValidationAcceptor): void { - new TypeDefValidator().validate(node, accept); + this.typeDefValidator.validate(node, accept); } checkEnum(node: Enum, accept: ValidationAcceptor): void { - new EnumValidator().validate(node, accept); + this.enumValidator.validate(node, accept); } checkAttribute(node: Attribute, accept: ValidationAcceptor): void { - new AttributeValidator().validate(node, accept); + this.attributeValidator.validate(node, accept); } checkExpression(node: Expression, accept: ValidationAcceptor): void { - new ExpressionValidator().validate(node, accept); + this.expressionValidator.validate(node, accept); } checkFunctionInvocation(node: InvocationExpr, accept: ValidationAcceptor): void { - new FunctionInvocationValidator().validate(node, accept); + this.functionInvocationValidator.validate(node, accept); } checkFunctionDecl(node: FunctionDecl, accept: ValidationAcceptor): void { - new FunctionDeclValidator().validate(node, accept); + this.functionDeclValidator.validate(node, accept); } }Additionally, please confirm all schema validations are triggered via the new ZModelDocumentBuilder. If there are code paths manually invoking the registry without the builder, they may need guards.
packages/language/src/zmodel-document-builder.ts (2)
9-17: Don’t drop other validation options; merge when validation is an object.
If options.validation is an object, your override replaces any existing flags (e.g., custom checks). Merge instead; for boolean true, synthesize the stopAfter* flags.Apply:
- validation: - // force overriding validation options - options.validation === false || options.validation === undefined - ? options.validation - : { - stopAfterLexingErrors: true, - stopAfterParsingErrors: true, - stopAfterLinkingErrors: true, - }, + validation: (() => { + const v = options.validation; + if (v === false) return v; + const base = + v && typeof v === 'object' + ? v + : { /* boolean true or undefined => start from empty */ }; + return { + ...base, + stopAfterLexingErrors: true, + stopAfterParsingErrors: true, + stopAfterLinkingErrors: true, + }; + })(),
4-4: Type the cancelToken parameter.
Use the proper CancellationToken type instead of any for better type safety.Example:
import type { CancellationToken } from 'vscode-jsonrpc'; override buildDocuments(documents: LangiumDocument[], options: BuildOptions, cancelToken?: CancellationToken): Promise<void>packages/language/src/utils.ts (1)
174-178: Fallback to resolved ref when available to include imported mixins.
Using only $refText + local container misses mixins imported from other modules. Prefer mixin.ref when resolved; fall back to container lookup pre-link.Apply:
- decl.mixins.forEach((mixin) => { - // avoid using mixin.ref since this function can be called before linking - const baseDecl = decl.$container.declarations.find( - (d): d is TypeDef => isTypeDef(d) && d.name === mixin.$refText, - ); + decl.mixins.forEach((mixin) => { + // prefer resolved ref; fallback to same-container lookup pre-link + const baseDecl = + mixin.ref ?? + decl.$container.declarations.find( + (d): d is TypeDef => isTypeDef(d) && d.name === mixin.$refText, + );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
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/language/src/module.ts(2 hunks)packages/language/src/utils.ts(3 hunks)packages/language/src/validator.ts(2 hunks)packages/language/src/zmodel-document-builder.ts(1 hunks)packages/language/test/expression-validation.test.ts(2 hunks)packages/runtime/package.json(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)tests/regression/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place packages only under
packages/,samples/, ortests/
Files:
packages/sdk/package.jsontests/e2e/package.jsonpackages/testtools/package.jsonpackages/language/package.jsonpackages/typescript-config/package.jsonpackages/language/src/zmodel-document-builder.tssamples/blog/package.jsonpackages/runtime/package.jsonpackages/zod/package.jsonpackages/language/test/expression-validation.test.tspackages/vitest-config/package.jsonpackages/eslint-config/package.jsonpackages/ide/vscode/package.jsontests/regression/package.jsonpackages/common-helpers/package.jsonpackages/cli/package.jsonpackages/language/src/module.tspackages/dialects/sql.js/package.jsonpackages/language/src/utils.tspackages/create-zenstack/package.jsonpackages/tanstack-query/package.jsonpackages/language/src/validator.ts
tests/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
End-to-end tests must live under
tests/e2e/
Files:
tests/e2e/package.json
packages/ide/vscode/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
The
ide/vscodepackage is intentionally versioned differently from other packages
Files:
packages/ide/vscode/package.json
package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Pin the repository package manager to
[email protected]via thepackageManagerfield
Files:
package.json
🧠 Learnings (1)
📚 Learning: 2025-09-04T12:38:14.150Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-04T12:38:14.150Z
Learning: Applies to packages/ide/vscode/package.json : The `ide/vscode` package is intentionally versioned differently from other packages
Applied to files:
packages/ide/vscode/package.json
🧬 Code graph analysis (3)
packages/language/src/module.ts (1)
packages/language/src/zmodel-document-builder.ts (1)
ZModelDocumentBuilder(3-22)
packages/language/src/utils.ts (1)
packages/language/src/generated/ast.ts (4)
TypeDef(725-733)TypeDef(735-735)isTypeDef(737-739)isDataModel(391-393)
packages/language/src/validator.ts (1)
packages/language/src/validators/schema-validator.ts (1)
SchemaValidator(10-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: claude-review
🔇 Additional comments (26)
packages/language/package.json (1)
4-4: Version bump looks good.No issues spotted; aligns with repo-wide beta.7.
packages/testtools/package.json (1)
3-3: Version bump LGTM.Consistent with workspace-wide update.
packages/sdk/package.json (1)
3-3: Version bump LGTM.No other changes; consistent with the release.
packages/zod/package.json (1)
3-3: Version bump LGTM.No functional changes in this file.
packages/tanstack-query/package.json (1)
3-3: Version bump LGTM.No additional changes detected.
packages/ide/vscode/package.json (1)
4-4: VS Code extension version bump is appropriate.Per prior guidance, this package is intentionally versioned independently. packageManager is correctly pinned.
tests/regression/package.json (1)
3-3: Version bump LGTM.Aligned with beta.7 release.
package.json (1)
3-3: Root packageManager pinned; package-version scan incompleteRoot packageManager: [email protected]. The verification script failed while scanning package.json files (shell error: "/bin/bash: line 14: !: command not found"), so I cannot confirm all packages use version 3.0.0-beta.7 — re-run the check with a corrected script or confirm versions manually.
tests/e2e/package.json (1)
3-3: Version bump to 3.0.0-beta.7 looks good; path complies with e2e guideline.
No further action needed.packages/common-helpers/package.json (1)
3-3: Version bump to 3.0.0-beta.7 looks good.
No other changes detected.packages/dialects/sql.js/package.json (1)
3-3: Version bump to 3.0.0-beta.7 looks good.
Exports and peers unchanged; OK.packages/typescript-config/package.json (1)
3-3: Version bump to 3.0.0-beta.7 looks good.
Private config package—no further checks needed here.samples/blog/package.json (1)
3-3: Version bump to 3.0.0-beta.7 looks good.
Sample remains consistent with workspace dependencies.packages/create-zenstack/package.json (1)
3-3: Version bump to 3.0.0-beta.7 looks good.
Bin and exports unchanged; OK.packages/cli/package.json (1)
6-6: Version bump to 3.0.0-beta.7 looks good.
CLI deps are workspace-pinned; alignment looks fine.packages/vitest-config/package.json (1)
4-4: Version bump to 3.0.0-beta.7 looks good.
No functional changes.packages/language/test/expression-validation.test.ts (1)
18-18: Good: relation scalar added to back the relation.
Adding authorId Int aligns with the relation definition and prevents invalid schema state.packages/language/src/zmodel-document-builder.ts (1)
11-13: Confirm intended behavior when validation is undefined.
If Langium defaults validation to on when undefined, current logic may skip the stopAfter* protections. Verify Langium’s default and adjust accordingly (the merge snippet above handles it).packages/language/src/module.ts (1)
53-55: DocumentBuilder wiring looks correct.
Custom builder is injected under workspace; aligns with validator changes.packages/runtime/package.json (2)
3-3: Version bump OK.
No concerns.
56-62: Verified — build is configured to emit dist/plugins/policy/index.{js,cjs,d.ts,d.cts}
packages/runtime/tsup.config.ts has an entry for 'plugins/policy/index' with dts:true and formats ['cjs','esm']; src/plugins/policy/index.ts exists and packages/runtime/package.json exports point to the matching dist/plugins/policy/index.* artifacts.packages/language/src/utils.ts (4)
522-525: Pre-link safety improvement looks good.
Guarding on mixin.ref avoids throwing before linking.
528-531: Pre-link safety for baseModel is good.
Conditional recursion prevents access to unresolved refs.
548-551: Same approval for attributes on mixins.
No issues.
554-557: Same approval for attributes on baseModel.
No issues.packages/eslint-config/package.json (1)
3-3: Approve — repo-wide version alignment verified.
All @zenstackhq/* packages are at 3.0.0-beta.7; no action needed.
Summary by CodeRabbit
Improvements
Bug Fixes
Chores