refactor: split engine into umbrella + engine-core packages#21
Open
albertgwo wants to merge 2 commits intofeat/engine-pr8-gorules-zenfrom
Open
refactor: split engine into umbrella + engine-core packages#21albertgwo wants to merge 2 commits intofeat/engine-pr8-gorules-zenfrom
albertgwo wants to merge 2 commits intofeat/engine-pr8-gorules-zenfrom
Conversation
Rename packages/engine/ to packages/engine-core/ and update the package name to @ruminaider/flowprint-engine-core. Update all references in engine-gorules (dependency, imports, tsconfig) and root tsconfig.json to point to the new location.
Create packages/engine/ as a thin umbrella that re-exports from @ruminaider/flowprint-engine-core and @ruminaider/flowprint-engine-gorules. Existing consumers (CLI, app) keep importing from @ruminaider/flowprint-engine and get both core and gorules functionality. The ./browser subpath re-exports only the browser-safe core. GoRules evaluateExpression/evaluateExpressions are re-exported as zenEvaluateExpression/zenEvaluateExpressions to avoid collision with the core runner's evaluateExpression.
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
Comment on lines
1
to
9
| import { defineConfig } from 'tsup' | ||
|
|
||
| export default defineConfig({ | ||
| entry: ['src/index.ts', 'src/rules/evaluator-browser.ts', 'src/rules/core.ts'], | ||
| entry: ['src/index.ts', 'src/browser.ts'], | ||
| format: ['esm', 'cjs'], | ||
| dts: { | ||
| compilerOptions: { | ||
| composite: false, | ||
| }, | ||
| }, | ||
| clean: true, | ||
| dts: true, | ||
| sourcemap: true, | ||
| clean: true, | ||
| }) |
There was a problem hiding this comment.
tsup no longer emits dist/rules/evaluator-browser.*, so the deep import @ruminaider/flowprint-engine/rules/evaluator-browser fails — should we re-add that entry and re-export it or add an alias?
Finding type: Breaking Changes | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In packages/engine/tsup.config.ts around lines 1-9, the tsup entry array removed
src/rules/evaluator-browser.ts (now ['src/index.ts','src/browser.ts']), which prevents
emitting dist/rules/evaluator-browser.* and breaks callers importing the deep path
@ruminaider/flowprint-engine/rules/evaluator-browser. Restore backward compatibility by
re-adding 'src/rules/evaluator-browser.ts' to the entry array so tsup emits that file,
and/or add a compatibility re-export: either add an explicit package export/subpath in
packages/engine package.json that maps './rules/evaluator-browser' to
'./dist/rules/evaluator-browser.js' (and .d.ts), or create a small source re-export file
(e.g., src/rules/evaluator-browser.ts) that re-exports from src/browser.ts and ensure
it's included in entries. Make this change so previous deep imports continue to resolve
without runtime module-not-found errors.
Heads up!
Your free trial ends tomorrow.
To keep getting your PRs reviewed by Baz, update your team's subscription
Comment on lines
+1
to
+4
| { | ||
| "name": "@ruminaider/flowprint-engine-core", | ||
| "version": "0.0.0", | ||
| "description": "Expression parser, dev runner, and code generator for Flowprint execution engine", |
There was a problem hiding this comment.
The package name "@ruminaider/flowprint-engine-core" isn't in CLAUDE.md's canonical package list — should we rename it to an allowed name or update the guidelines to permit it?
Finding type: AI Coding Guidelines | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In packages/engine-core/package.json around lines 1-4, the "name" field is set to
"@ruminaider/flowprint-engine-core", which violates the repository's canonical package
naming policy. Either (A) rename the package to one of the allowed canonical names (for
example replace the value with "@ruminaider/flowprint-engine"), update any references in
build/publish configs or workspace entries, and ensure package files
(main/module/exports) still point to the correct output; OR (B) update the team
guideline file (CLAUDE.md) to explicitly permit "@ruminaider/flowprint-engine-core" and
add a short justification and versioning/publishing rules. Make the chosen change
consistently across the repo (package.json, workspace references, and documentation) and
return the modified files and a brief commit message explaining the decision.
Heads up!
Your free trial ends tomorrow.
To keep getting your PRs reviewed by Baz, update your team's subscription
Comment on lines
+1
to
+4
| { | ||
| "name": "@ruminaider/flowprint-engine-core", | ||
| "version": "0.0.0", | ||
| "description": "Expression parser, dev runner, and code generator for Flowprint execution engine", |
There was a problem hiding this comment.
New engine-core package and engine umbrella refactor lack a .changeset entry — should we add one announcing these package changes?
Finding type: AI Coding Guidelines | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In packages/engine-core/package.json around lines 1-4, a new package
@ruminaider/flowprint-engine-core was added but there is no .changeset entry for this
PR. Add a new changeset file under .changeset (e.g.
.changeset/<short-descriptive-name>.md) that declares the new package should be released
(use a minor or appropriate bump for a new package) and also documents the change to the
umbrella package @ruminaider/flowprint-engine (use patch/minor as appropriate). In the
changeset file include a brief human-readable summary: announce the new engine-core
package and the umbrella refactor so the release tooling will pick up both package-level
changes.
Heads up!
Your free trial ends tomorrow.
To keep getting your PRs reviewed by Baz, update your team's subscription
Deploying flowprint with
|
| Latest commit: |
2b75325
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a82df751.flowprint.pages.dev |
| Branch Preview URL: | https://feat-engine-pr9-package-spli.flowprint.pages.dev |
Contributor
Author
Code Review FindingsImportant (80-89 confidence)1. CLAUDE.md not updated to reflect new monorepo structure (88)
2. Version manually bumped to 0.0.1 without changeset (82)
3. Umbrella tsup.config.ts DTS inconsistency (81)
Positive notes
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
Dependency
Base: `feat/engine-pr8-gorules-zen`
PR 13 of 16 in the execution engine implementation.
Test plan
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Split the Flowprint engine into
@ruminaider/flowprint-engine-coreso the parser, runner, expressions, rules, walker, security, adapters, and codegen helpers are all surfaced from a dedicated package with a browser-safe entrypoint. Provide the new@ruminaider/flowprint-engineumbrella that re-exports the core surface plus Gorules helpers while refreshing workspace metadata so downstream tools resolve the split cleanly.engine-coreworkspace with its own package metadata, tsconfig references, and tsup build so that all parser/runner/rules/walker/security/adapter/codegen exports plus the browser entrypoint live behind@ruminaider/flowprint-engine-core.Modified files (4)
Latest Contributors(0)
engine-core, update the Gorules tsconfig/imports/tests, and capture all new workspace lockfile artifacts so theengine-goruleshelpers still wrap the Zen engine over the renamed core package.Modified files (7)
Latest Contributors(0)
@ruminaider/flowprint-enginepackage into a thin umbrella that re-exports the core package, surfaces a browser-safe entry, and points its build/tsconfig at the new workspaces so consumers keep a familiar entrypoint.Modified files (6)
Latest Contributors(1)
Modified files (75)
Latest Contributors(0)