feat(engine-gorules): GoRules ZEN wrapper + rules DRY fix#20
feat(engine-gorules): GoRules ZEN wrapper + rules DRY fix#20albertgwo wants to merge 3 commits intofeat/engine-pr5-scoped-compensationfrom
Conversation
Move pure evaluation functions (resolveDotPath, ruleMatches, evaluateCondition, normalizeCondition, evaluateOperator, applyHitPolicy) from evaluator-browser.ts into a new core.ts module. Both Node.js and browser evaluators now import from core.ts, eliminating duplication.
New @ruminaider/flowprint-engine-gorules package wrapping @gorules/zen-engine: - evaluateExpression/evaluateExpressions: sync expression evaluation via ZEN - translateToJDM: converts flowprint .rules.yaml to GoRules JDM format - evaluateRulesViaZen: async decision table evaluation with caching - conditionToUnary: operator-to-ZEN unary expression mapping Supports all flowprint operators (eq, gt, gte, lt, lte, not_eq, in, not_in, between) and all hit policies (first, collect, all, priority).
38 tests covering: - Expression evaluation: arithmetic, boolean logic, ZEN syntax, errors - Multiple expressions with chained references - Rules translator: all operator mappings, JDM structure, hit policies - Rules evaluator: first/collect hit policies, wildcard rules, no-match
Deploying flowprint with
|
| Latest commit: |
e48fef4
|
| Status: | 🚫 Build failed. |
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| const response = await decision.evaluate(input) | ||
|
|
||
| const isCollect = doc.hit_policy === 'collect' || doc.hit_policy === 'all' | ||
|
|
||
| return { | ||
| hit_policy: doc.hit_policy, | ||
| matched_count: isCollect | ||
| ? (Array.isArray(response.result) ? response.result.length : 0) | ||
| : (isNonEmptyObject(response.result) ? 1 : 0), | ||
| output: response.result, |
There was a problem hiding this comment.
For hit_policy: 'all' the GoRules wrapper returns matched_count:0/output:[] instead of throwing like the core engine — should we throw when doc.hit_policy === 'all' and the translated result array is empty?
Finding type: Logical Bugs | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In packages/engine-gorules/src/rules-evaluator.ts around lines 41 to 50, the
evaluateRulesViaZen function treats hit_policy 'all' as a collect array and returns
matched_count 0/output [] instead of throwing when no rules matched. Change the logic so
that after awaiting decision.evaluate(input) and before returning, if doc.hit_policy ===
'all' and response.result is an array with length === 0, throw an Error (matching the
core engine semantics) rather than returning a zero count. Keep the existing
matched_count/output computation for other cases.
Heads up!
Your free trial ends tomorrow.
To keep getting your PRs reviewed by Baz, update your team's subscription
| function normalizeCondition(condition: Condition): OperatorCondition { | ||
| if ( | ||
| condition === null || | ||
| typeof condition === 'string' || | ||
| typeof condition === 'number' || | ||
| typeof condition === 'boolean' | ||
| ) { | ||
| return { eq: condition } | ||
| } | ||
| return condition |
There was a problem hiding this comment.
normalizeCondition duplicates the exact scalar-to-{eq} normalization that already exists in packages/engine/src/rules/core.ts (lines 77‑86). Any future change to shorthand handling (e.g., new operators, null/boolean treatment) would need to be applied in both translator and engine logic, which is easy to miss and causes inconsistent condition normalization between the GoRules translator and the Flowprint runtime. Can we import/re‑export the shared normalizeCondition helper from the Flowprint rules core (or add it to a shared utils module) so the translator and runtime use the same logic rather than duplicating the implementation?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Heads up!
Your free trial ends tomorrow.
To keep getting your PRs reviewed by Baz, update your team's subscription
| return `> ${String(operand)}` | ||
| case 'gte': | ||
| return `>= ${String(operand)}` | ||
| case 'lt': | ||
| return `< ${String(operand)}` |
There was a problem hiding this comment.
operatorToUnary stringifies Condition values and emits unparseable literals for non-primitives like objects/arrays/dates — should we validate/reject non-primitive operands or serialize arrays per GoRules before emitting the cell?
Finding type: Logical Bugs | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In packages/engine-gorules/src/rules-translator.ts around lines 284 to 288, the
operatorToUnary function currently interpolates String(operand) for gt/gte/lt/lte which
will produce invalid JDM when operand is a non-primitive (object/array/date). Change
operatorToUnary to validate operands: for gt/gte/lt/lte require a number or
numeric-string and throw a clear Error if the operand is not a primitive number; for
eq/not_eq accept string/number/boolean/null only and throw for objects; for
in/not_in/between allow arrays but validate each element is a primitive
(string/number/boolean/null) and serialize via formatLiteral (or throw if any element is
non-primitive); for default cases throw an error for unsupported operand types instead
of String(operand). Also update conditionToUnary to catch and rethrow errors with the
field name and operator context so invalid rules fail fast with helpful messages.
Heads up!
Your free trial ends tomorrow.
To keep getting your PRs reviewed by Baz, update your team's subscription
| function valueToExpression(value: unknown): string { | ||
| if (typeof value === 'string') { | ||
| return `"${value}"` | ||
| } | ||
| if (value === null) { |
There was a problem hiding this comment.
Should we guard/throw in valueToExpression when an output isn't a primitive or array literal, or serialize it to a supported form, since non-primitives become [object Object] or functions that GoRules can't parse?
Finding type: Logical Bugs | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In packages/engine-gorules/src/rules-translator.ts around lines 324 to 328, the
valueToExpression function currently only quotes strings and falls back to
String(value), which produces '[object Object]' for objects/arrays/dates. Change
valueToExpression to: accept primitives (string -> quoted, null -> 'null',
number/boolean -> String), handle arrays by recursively serializing each element (only
allow primitives inside arrays) and emit a bracketed list or comma-separated literals,
and explicitly detect and throw a descriptive error for unsupported types (plain
objects, Dates, functions, symbols) instead of returning String(value). Ensure
tests/usage expect a thrown error for unsupported outputs or that arrays are serialized
deterministically.
Heads up!
Your free trial ends tomorrow.
To keep getting your PRs reviewed by Baz, update your team's subscription
| { | ||
| "name": "@ruminaider/flowprint-engine-gorules", | ||
| "version": "0.0.1", | ||
| "private": true, | ||
| "type": "module", | ||
| "main": "dist/index.js", | ||
| "types": "dist/index.d.ts", | ||
| "exports": { | ||
| ".": { | ||
| "import": { | ||
| "types": "./dist/index.d.ts", | ||
| "default": "./dist/index.js" |
There was a problem hiding this comment.
@ruminaider/flowprint-engine-gorules added without a .changeset/*.md file — should we add a changeset?
Finding type: AI Coding Guidelines | Severity: 🟠 Medium
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In packages/engine-gorules/package.json around lines 1-34, a new package
"@ruminaider/flowprint-engine-gorules" was added but no .changeset/*.md file was
created. Add a new changeset file under .changeset (for example
.changeset/add-flowprint-engine-gorules.md) that documents this package addition:
include a YAML-style header or changeset YAML mapping listing
"@ruminaider/flowprint-engine-gorules: minor" (or patch if you prefer), and a short
description like "Add @ruminaider/flowprint-engine-gorules package" plus an optional
reference to the PR/author. Ensure the new file follows the repo's changeset format so
the CI will recognize the package change.
Heads up!
Your free trial ends tomorrow.
To keep getting your PRs reviewed by Baz, update your team's subscription
Code Review FindingsCritical (90-100 confidence)1.
2.
3.
Important (80-89 confidence)4. Unbounded
5.
6.
7. Missing changeset file (82)
8.
|
1 participant
User description
Summary
Dependency
Base: `feat/engine-pr5-scoped-compensation`
PR 12 of 16 in the execution engine implementation.
Test plan
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Add a
@ruminaider/flowprint-engine-gorulespackage that wraps GoRules ZEN to evaluate flowprint expressions and rules via JDM with caching, expression helpers, translator, and tests. Extract the shared operator/condition/hit policy helpers into a core module so both the browser and Node evaluators reuse the same logic while keeping bundler configs aware of the new entry point.Modified files (11)
Latest Contributors(0)
core.tsand rewire browser/Node evaluators plus bundler config to consume it.Modified files (5)
Latest Contributors(1)