diff --git a/packages/compiler/src/expressions/compile-expression.ts b/packages/compiler/src/expressions/compile-expression.ts index 50674314..a2f9008a 100644 --- a/packages/compiler/src/expressions/compile-expression.ts +++ b/packages/compiler/src/expressions/compile-expression.ts @@ -7,6 +7,7 @@ import { type Expression, + type BinaryOperator, type TemplatePart, MemberExpression, AtIdentifier, @@ -297,17 +298,21 @@ function compileMemberExpression( } default: { - const obj = compileExprNode(expr.object, ctx, opts); + const obj = compileAccessTarget(expr.object, ctx, opts); return `${obj}.${property}`; } } } - const obj = compileExprNode(expr.object, ctx, opts); // Convert .length to len() function call for runtime compatibility if (expr.property === 'length') { - return `len(${obj})`; + return `len(${compileExprNode(expr.object, ctx, opts)})`; } + // A number before `.` would lex as a float literal (`1.real`), so wrap it. + const obj = + expr.object instanceof NumberLiteral + ? `(${compileExprNode(expr.object, ctx, opts)})` + : compileAccessTarget(expr.object, ctx, opts); return `${obj}.${expr.property}`; } @@ -437,7 +442,7 @@ function compileSubscriptExpression( return `system.uploaded_files[${compileIndex()}]`; } - const obj = compileExprNode(expr.object, ctx, opts); + const obj = compileAccessTarget(expr.object, ctx, opts); return `${obj}[${compileIndex()}]`; } @@ -458,33 +463,64 @@ function compileSlice( return `${start}:${stop}`; } -function getParenthesizedCst(expr: Expression): SyntaxNode | undefined { - let node = expr.__cst?.node; - let parenthesized: SyntaxNode | undefined; - // The dialect unwraps grouping and expression/atom nodes from the AST. - // Retain the outermost grouping node for comparison source-text boundaries. - while (node) { - if (node.type === 'parenthesized_expression') parenthesized = node; - const parent = node.parent; - if ( - parent?.type !== 'parenthesized_expression' && - parent?.type !== 'expression' && - parent?.type !== 'atom' - ) { - break; - } - node = parent; +/** + * Operator binding strength, following Python's precedence because compiled + * expressions are evaluated as Python. Higher binds tighter; everything that + * is not an operator expression (literals, references, calls, subscripts, + * member accesses) is primary. + */ +const PRECEDENCE = { + ternary: 1, + or: 2, + and: 3, + not: 4, + comparison: 5, + additive: 6, + multiplicative: 7, + unary: 8, + primary: 9, +} as const; + +function binaryPrecedence(operator: BinaryOperator): number { + switch (operator) { + case 'or': + return PRECEDENCE.or; + case 'and': + return PRECEDENCE.and; + case '+': + case '-': + return PRECEDENCE.additive; + case '*': + case '/': + return PRECEDENCE.multiplicative; + } +} + +function precedenceOf(expr: Expression): number { + if (expr instanceof TernaryExpression) return PRECEDENCE.ternary; + if (expr instanceof ComparisonExpression) return PRECEDENCE.comparison; + if (expr instanceof UnaryExpression) { + return expr.operator === 'not' ? PRECEDENCE.not : PRECEDENCE.unary; } - return parenthesized; + if (expr instanceof BinaryExpression) return binaryPrecedence(expr.operator); + // A negative literal (only constructible in code) prints with a leading `-`. + if (expr instanceof NumberLiteral && expr.value < 0) return PRECEDENCE.unary; + return PRECEDENCE.primary; } +/** + * Compile an operand, adding parentheses when it binds more loosely than + * `minPrecedence`. Grouping comes from the shape of the tree, not from the + * source text, so the output is correct for hand-built ASTs too. + */ function compileOperand( expr: Expression, + minPrecedence: number, ctx: CompilerContext, opts: CompileExpressionOptions ): string { const compiled = compileExprNode(expr, ctx, opts); - return getParenthesizedCst(expr) ? `(${compiled})` : compiled; + return precedenceOf(expr) < minPrecedence ? `(${compiled})` : compiled; } function compileBinaryExpression( @@ -492,8 +528,10 @@ function compileBinaryExpression( ctx: CompilerContext, opts: CompileExpressionOptions ): string { - const left = compileOperand(expr.left, ctx, opts); - const right = compileOperand(expr.right, ctx, opts); + // Left-associative: an equal-precedence right operand needs parentheses. + const precedence = binaryPrecedence(expr.operator); + const left = compileOperand(expr.left, precedence, ctx, opts); + const right = compileOperand(expr.right, precedence + 1, ctx, opts); return `${left} ${expr.operator} ${right}`; } @@ -502,10 +540,11 @@ function compileUnaryExpression( ctx: CompilerContext, opts: CompileExpressionOptions ): string { - const operand = compileOperand(expr.operand, ctx, opts); if (expr.operator === 'not') { - return `not ${operand}`; + return `not ${compileOperand(expr.operand, PRECEDENCE.not, ctx, opts)}`; } + // Wrap a nested sign too, so `-(-1)` never prints as `--1`. + const operand = compileOperand(expr.operand, PRECEDENCE.primary, ctx, opts); return `${expr.operator}${operand}`; } @@ -514,29 +553,47 @@ function compileComparisonExpression( ctx: CompilerContext, opts: CompileExpressionOptions ): string { - const left = compileOperand(expr.left, ctx, opts); - const right = compileOperand(expr.right, ctx, opts); - - // Preserve original whitespace around the operator from the source text - const cstText = expr.__cst?.node?.text; - if (cstText) { - const leftCst = (getParenthesizedCst(expr.left) ?? expr.left.__cst?.node) - ?.text; - const rightCst = (getParenthesizedCst(expr.right) ?? expr.right.__cst?.node) - ?.text; - if (leftCst && rightCst) { - // Extract the operator with surrounding whitespace from the CST - const leftStart = cstText.indexOf(leftCst); - const leftEnd = leftStart + leftCst.length; - const rightStart = cstText.lastIndexOf(rightCst); - if (leftStart >= 0 && rightStart > leftEnd) { - const operatorWithSpace = cstText.slice(leftEnd, rightStart); - return `${left}${operatorWithSpace}${right}`; - } - } + // Python chains comparisons (`a < b < c` is `a < b and b < c`). A chain + // written in the source stays a chain; any other comparison operand of a + // comparison, such as `(a < b) < c` or one built in code, is parenthesized. + const compileSide = (side: Expression) => + isSourceChainLink(side, expr) + ? compileExprNode(side, ctx, opts) + : compileOperand(side, PRECEDENCE.comparison + 1, ctx, opts); + return `${compileSide(expr.left)} ${expr.operator} ${compileSide(expr.right)}`; +} + +/** + * Whether `operand` is a link of a comparison chain written without + * parentheses, like `a < b` in `a < b < c`. The AST nests a chain the same + * way as `(a < b) < c`, so only the source CST can tell them apart. + */ +function isSourceChainLink( + operand: Expression, + comparison: ComparisonExpression +): boolean { + const outer = comparison.__cst?.node; + let node = operand.__cst?.node; + if (!(operand instanceof ComparisonExpression) || !outer || !node) { + return false; } + // Climb past `expression` wrappers. A `parenthesized_expression` stops the + // climb, so an explicitly grouped operand never matches. + do { + node = node.parent ?? undefined; + } while (node?.type === 'expression'); + return node !== undefined && isSameNode(node, outer); +} - return `${left} ${expr.operator} ${right}`; +/** Compare by position; `SyntaxNode` does not promise stable object identity. */ +function isSameNode(a: SyntaxNode, b: SyntaxNode): boolean { + return ( + a.type === b.type && + a.startRow === b.startRow && + a.startCol === b.startCol && + a.endRow === b.endRow && + a.endCol === b.endCol + ); } function compileTernaryExpression( @@ -544,18 +601,34 @@ function compileTernaryExpression( ctx: CompilerContext, opts: CompileExpressionOptions ): string { - const consequence = compileExprNode(expr.consequence, ctx, opts); - const condition = compileExprNode(expr.condition, ctx, opts); - const alternative = compileExprNode(expr.alternative, ctx, opts); + // Right-associative: only the alternative may be an unparenthesized ternary. + const operandMin = PRECEDENCE.ternary + 1; + const consequence = compileOperand(expr.consequence, operandMin, ctx, opts); + const condition = compileOperand(expr.condition, operandMin, ctx, opts); + const alternative = compileOperand( + expr.alternative, + PRECEDENCE.ternary, + ctx, + opts + ); return `${consequence} if ${condition} else ${alternative}`; } +/** The target of `.x`, `[i]` or `(...)` must be primary. */ +function compileAccessTarget( + expr: Expression, + ctx: CompilerContext, + opts: CompileExpressionOptions +): string { + return compileOperand(expr, PRECEDENCE.primary, ctx, opts); +} + function compileCallExpression( expr: CallExpression, ctx: CompilerContext, opts: CompileExpressionOptions ): string { - const func = compileExprNode(expr.func, ctx, opts); + const func = compileAccessTarget(expr.func, ctx, opts); const args = expr.args .map((a: Expression) => compileExprNode(a, ctx, opts)) .join(', '); diff --git a/packages/compiler/src/nodes/compile-collect.ts b/packages/compiler/src/nodes/compile-collect.ts index bea16736..1c696375 100644 --- a/packages/compiler/src/nodes/compile-collect.ts +++ b/packages/compiler/src/nodes/compile-collect.ts @@ -260,8 +260,8 @@ export function buildPriorGuardByTarget( /** * Right-associate a list of operands under a binary operator, parenthesizing * the tail so that 3+ operand expressions read `A OP (B OP (C OP D))`. The - * runtime evaluator requires explicit parens for 3+ operand expressions (the - * compiler's expression emitter never auto-parenthesizes), and this matches the + * runtime evaluator requires explicit parens for 3+ operand expressions (these + * strings are built here, not by the expression emitter), and this matches the * only existing 3-operand `enabled` condition in the golden corpus * (`pronto_customer_support`: `A and (B and C)`). For 1 operand the operand is * returned as-is; for 2 it is `A OP B` (no parens needed). diff --git a/packages/compiler/test/expressions.test.ts b/packages/compiler/test/expressions.test.ts index 19ac8213..6a348df3 100644 --- a/packages/compiler/test/expressions.test.ts +++ b/packages/compiler/test/expressions.test.ts @@ -20,6 +20,7 @@ import { AtIdentifier, BinaryExpression, UnaryExpression, + TernaryExpression, ComparisonExpression, SubscriptExpression, SliceExpression, @@ -370,8 +371,23 @@ describe('compileExpression', () => { }); }); - describe('binary expressions', () => { + describe('operator grouping', () => { + function compileSource(source: string): string { + for (const name of ['a', 'b', 'c']) ctx.mutableVariableNames.add(name); + const { rootNode } = parse(`value: ${source}\n`); + const mapping = + rootNode.namedChildren.find(node => node.type === 'mapping') ?? + rootNode; + const result = new Dialect().parse(mapping, { value: ExpressionValue }); + + expect(result.diagnostics).toEqual([]); + const compiled = compileExpression(result.value.value, ctx); + expect(ctx.diagnostics).toEqual([]); + return compiled; + } + it.each([ + // Binary operands keep the grouping they need. ['(1 + 2) * (3 + 4)', '(1 + 2) * (3 + 4)'], ['(1 + 2) * 3', '(1 + 2) * 3'], ['1 * (2 + 3)', '1 * (2 + 3)'], @@ -380,18 +396,165 @@ describe('compileExpression', () => { '(True or False) and (False or False)', '(True or False) and (False or False)', ], - ])('preserves operand grouping in %s', (source, expected) => { - const { rootNode } = parse(`value: ${source}\n`); - const mapping = - rootNode.namedChildren.find(node => node.type === 'mapping') ?? - rootNode; - const result = new Dialect().parse(mapping, { value: ExpressionValue }); + // Redundant parentheses are dropped. + ['(1 + 2)', '1 + 2'], + ['((1 + 2)) * 3', '(1 + 2) * 3'], + ['(1 * 2) + 3', '1 * 2 + 3'], + ['(1 * 2 + 3)', '1 * 2 + 3'], + ['(True and False) or True', 'True and False or True'], + // Associativity. + ['1 - (2 - 3)', '1 - (2 - 3)'], + ['(1 - 2) - 3', '1 - 2 - 3'], + ['1 - 2 - 3', '1 - 2 - 3'], + ['1 / (2 * 3)', '1 / (2 * 3)'], + ['1 - (2 + 3)', '1 - (2 + 3)'], + // Unary operators. + ['not (True and False)', 'not (True and False)'], + [ + '@variables.a and not (@variables.b or @variables.c)', + 'state.a and not (state.b or state.c)', + ], + ['-(1 + 2)', '-(1 + 2)'], + ['-(1 + 2) * 3', '-(1 + 2) * 3'], + ['not (@variables.a == 1)', 'not state.a == 1'], + ['not (not True)', 'not not True'], + ['-(-1)', '-(-1)'], + ['-(-@variables.a)', '-(-state.a)'], + ['(-@variables.a).x', '(-state.a).x'], + ['not (1 if True else 2)', 'not (1 if True else 2)'], + ['-(1 if True else 2)', '-(1 if True else 2)'], + ['not True', 'not True'], + ['-1', '-1'], + // Comparisons chain in Python, so grouped comparisons stay grouped. + ['(True or False) == False', '(True or False) == False'], + ['True == (False or True)', 'True == (False or True)'], + ['(1 + 2) < (3 + 4)', '1 + 2 < 3 + 4'], + ['(@variables.a) == (@variables.a)', 'state.a == state.a'], + ['@variables.a == @variables.a', 'state.a == state.a'], + ['((1)) == ((2))', '1 == 2'], + ['(1 == 2) == False', '(1 == 2) == False'], + ['1 == (2 == 3)', '1 == (2 == 3)'], + ['(1 < 2) < 3', '(1 < 2) < 3'], + ['(1 + 2) == (3)', '1 + 2 == 3'], + ['(1 if True else 2) == 1', '(1 if True else 2) == 1'], + ['1 == 1', '1 == 1'], + ['1==1', '1 == 1'], + ['@variables.a is not None', 'state.a is not None'], + // An unparenthesized chain stays a Python chain. + ['3 > 2 > 1', '3 > 2 > 1'], + ['0 < @variables.a < 10', '0 < state.a < 10'], + ['"a" < "b" < "c"', '"a" < "b" < "c"'], + ['1 < 2 <= 3 == 3', '1 < 2 <= 3 == 3'], + ['0 < @variables.a + 1 < 10', '0 < state.a + 1 < 10'], + ['(0 < @variables.a < 10)', '0 < state.a < 10'], + ['not 0 < @variables.a < 10', 'not 0 < state.a < 10'], + ['((1 < 2) < 3) < 4', '((1 < 2) < 3) < 4'], + ['(1 < 2 < 3) < 4', '(1 < 2 < 3) < 4'], + ['(1 < 2) < 3 < 4', '(1 < 2) < 3 < 4'], + ['1 < (2 < 3 < 4)', '1 < (2 < 3 < 4)'], + // Ternaries. + [ + '(1 if True else 2) if False else 3', + '(1 if True else 2) if False else 3', + ], + ['1 if (2 if True else 3) else 4', '1 if (2 if True else 3) else 4'], + [ + '1 if True else (2 if False else 3)', + '1 if True else 2 if False else 3', + ], + ['1 if (True or False) else 2', '1 if True or False else 2'], + ['(1 + 2) if True else (3 + 4)', '1 + 2 if True else 3 + 4'], + ['1 if True else 2 if False else 3', '1 if True else 2 if False else 3'], + ['1 + (2 if True else 3)', '1 + (2 if True else 3)'], + // Access targets. + ['(1 + 2)[0]', '(1 + 2)[0]'], + ['(@variables.a + @variables.b)[0]', '(state.a + state.b)[0]'], + ['(@variables.a or @variables.b)[0]', '(state.a or state.b)[0]'], + ['(@variables.a or @variables.b).x', '(state.a or state.b).x'], + ['(a + b)()', '(a + b)()'], + ['(1 if True else 2).x', '(1 if True else 2).x'], + ['(@variables.a).x', 'state.a.x'], + ['(1).real', '(1).real'], + ['@variables.a[0]', 'state.a[0]'], + ['(@variables.a + @variables.b).length', 'len(state.a + state.b)'], + ['f((1 + 2) * 3)', 'f((1 + 2) * 3)'], + ])('compiles %s as %s', (source, expected) => { + expect(compileSource(source)).toBe(expected); + }); - expect(result.diagnostics).toEqual([]); - expect(result.value.value).toBeInstanceOf(BinaryExpression); - expect(compileExpression(result.value.value, ctx)).toBe(expected); + it.each([ + [ + 'a nested binary on the left', + new BinaryExpression( + new BinaryExpression(new NumberLiteral(1), '+', new NumberLiteral(2)), + '*', + new NumberLiteral(3) + ), + '(1 + 2) * 3', + ], + [ + 'a nested binary on the right of a left-associative operator', + new BinaryExpression( + new NumberLiteral(1), + '-', + new BinaryExpression(new NumberLiteral(2), '-', new NumberLiteral(3)) + ), + '1 - (2 - 3)', + ], + [ + 'a ternary used as a condition', + new TernaryExpression( + new NumberLiteral(1), + new TernaryExpression( + new NumberLiteral(2), + new BooleanLiteral(true), + new NumberLiteral(3) + ), + new NumberLiteral(4) + ), + '1 if (2 if True else 3) else 4', + ], + [ + 'not applied to or', + new UnaryExpression( + 'not', + new BinaryExpression( + new BooleanLiteral(true), + 'or', + new BooleanLiteral(false) + ) + ), + 'not (True or False)', + ], + [ + 'a comparison on the left of a comparison', + new ComparisonExpression( + new ComparisonExpression( + new NumberLiteral(3), + '>', + new NumberLiteral(2) + ), + '>', + new NumberLiteral(1) + ), + '(3 > 2) > 1', + ], + [ + 'a negative literal as a member target', + new MemberExpression(new NumberLiteral(-1), 'real'), + '(-1).real', + ], + [ + 'a negative literal under unary minus', + new UnaryExpression('-', new NumberLiteral(-1)), + '-(-1)', + ], + ])('groups %s without any CST', (_name, expr, expected) => { + expect(compileExpression(expr, ctx)).toBe(expected); }); + }); + describe('binary expressions', () => { it('should compile binary addition', () => { const expr = new BinaryExpression( new NumberLiteral(1), @@ -412,30 +575,6 @@ describe('compileExpression', () => { }); describe('unary expressions', () => { - it.each([ - ['not (True and False)', 'not (True and False)'], - [ - '@variables.a and not (@variables.b or @variables.c)', - 'state.a and not (state.b or state.c)', - ], - ['-(1 + 2)', '-(1 + 2)'], - ['not True', 'not True'], - ['-1', '-1'], - ])('preserves operand grouping in %s', (source, expected) => { - ctx.mutableVariableNames.add('a'); - ctx.mutableVariableNames.add('b'); - ctx.mutableVariableNames.add('c'); - const { rootNode } = parse(`value: ${source}\n`); - const mapping = - rootNode.namedChildren.find(node => node.type === 'mapping') ?? - rootNode; - const result = new Dialect().parse(mapping, { value: ExpressionValue }); - - expect(result.diagnostics).toEqual([]); - expect(compileExpression(result.value.value, ctx)).toBe(expected); - expect(ctx.diagnostics).toEqual([]); - }); - it('should compile not operator', () => { const expr = new UnaryExpression('not', new BooleanLiteral(true)); expect(compileExpression(expr, ctx)).toBe('not True'); @@ -448,27 +587,6 @@ describe('compileExpression', () => { }); describe('comparison expressions', () => { - it.each([ - ['(True or False) == False', '(True or False) == False'], - ['True == (False or True)', 'True == (False or True)'], - ['(1 + 2) < (3 + 4)', '(1 + 2) < (3 + 4)'], - ['((1 + 2)) == ((3 + 4))', '(1 + 2) == (3 + 4)'], - ['(1 + 2)==(1 + 2)', '(1 + 2)==(1 + 2)'], - ['1 == 1', '1 == 1'], - ['1==1', '1==1'], - ])('preserves operand grouping and spacing in %s', (source, expected) => { - const { rootNode } = parse(`value: ${source}\n`); - const mapping = - rootNode.namedChildren.find(node => node.type === 'mapping') ?? - rootNode; - const result = new Dialect().parse(mapping, { value: ExpressionValue }); - - expect(result.diagnostics).toEqual([]); - expect(result.value.value).toBeInstanceOf(ComparisonExpression); - expect(compileExpression(result.value.value, ctx)).toBe(expected); - expect(ctx.diagnostics).toEqual([]); - }); - it('should compile == comparison', () => { ctx.mutableVariableNames.add('x'); const expr = new ComparisonExpression( diff --git a/packages/compiler/test/fixtures/expected/connection_with_unified_email.camel.json b/packages/compiler/test/fixtures/expected/connection_with_unified_email.camel.json index 85d3569b..c71dd2d8 100644 --- a/packages/compiler/test/fixtures/expected/connection_with_unified_email.camel.json +++ b/packages/compiler/test/fixtures/expected/connection_with_unified_email.camel.json @@ -1255,7 +1255,7 @@ "enabled": "True", "stateUpdates": [ { - "AgentScriptInternal_condition_1": "state.QualificationEnabled == True and state.HasSalesInterest is not None or (state.QualificationEnabled == False and state.MeetingSchedulingEnabled == True and state.WantsMeeting is not None) or (state.QualificationEnabled == False and state.MeetingSchedulingEnabled == False)" + "AgentScriptInternal_condition_1": "state.QualificationEnabled == True and state.HasSalesInterest is not None or state.QualificationEnabled == False and state.MeetingSchedulingEnabled == True and state.WantsMeeting is not None or state.QualificationEnabled == False and state.MeetingSchedulingEnabled == False" } ] }, @@ -2656,7 +2656,7 @@ "enabled": "True", "stateUpdates": [ { - "AgentScriptInternal_condition_1": "state.ProspectId is not None and state.RecordCreationIntent == \"qualify\" and (state.VerificationEnabled == True or (state.VerificationEnabled == False and state.IsNewRecord != \"false\"))" + "AgentScriptInternal_condition_1": "state.ProspectId is not None and state.RecordCreationIntent == \"qualify\" and (state.VerificationEnabled == True or state.VerificationEnabled == False and state.IsNewRecord != \"false\")" } ] }, @@ -2814,7 +2814,7 @@ "enabled": "state.AgentScriptInternal_next_topic==\"__EMPTY__\"", "stateUpdates": [ { - "AgentScriptInternal_condition_1": "state.ProspectId is not None and state.RecordCreationIntent == \"qualify\" and (state.VerificationEnabled == True or (state.VerificationEnabled == False and state.IsNewRecord != \"false\"))" + "AgentScriptInternal_condition_1": "state.ProspectId is not None and state.RecordCreationIntent == \"qualify\" and (state.VerificationEnabled == True or state.VerificationEnabled == False and state.IsNewRecord != \"false\")" } ] }, diff --git a/packages/compiler/test/fixtures/expected/connection_with_unified_email.snake.json b/packages/compiler/test/fixtures/expected/connection_with_unified_email.snake.json index 7078edfd..8ceff801 100644 --- a/packages/compiler/test/fixtures/expected/connection_with_unified_email.snake.json +++ b/packages/compiler/test/fixtures/expected/connection_with_unified_email.snake.json @@ -1255,7 +1255,7 @@ "enabled": "True", "state_updates": [ { - "AgentScriptInternal_condition_1": "state.QualificationEnabled == True and state.HasSalesInterest is not None or (state.QualificationEnabled == False and state.MeetingSchedulingEnabled == True and state.WantsMeeting is not None) or (state.QualificationEnabled == False and state.MeetingSchedulingEnabled == False)" + "AgentScriptInternal_condition_1": "state.QualificationEnabled == True and state.HasSalesInterest is not None or state.QualificationEnabled == False and state.MeetingSchedulingEnabled == True and state.WantsMeeting is not None or state.QualificationEnabled == False and state.MeetingSchedulingEnabled == False" } ] }, @@ -2656,7 +2656,7 @@ "enabled": "True", "state_updates": [ { - "AgentScriptInternal_condition_1": "state.ProspectId is not None and state.RecordCreationIntent == \"qualify\" and (state.VerificationEnabled == True or (state.VerificationEnabled == False and state.IsNewRecord != \"false\"))" + "AgentScriptInternal_condition_1": "state.ProspectId is not None and state.RecordCreationIntent == \"qualify\" and (state.VerificationEnabled == True or state.VerificationEnabled == False and state.IsNewRecord != \"false\")" } ] }, @@ -2814,7 +2814,7 @@ "enabled": "state.AgentScriptInternal_next_topic==\"__EMPTY__\"", "state_updates": [ { - "AgentScriptInternal_condition_1": "state.ProspectId is not None and state.RecordCreationIntent == \"qualify\" and (state.VerificationEnabled == True or (state.VerificationEnabled == False and state.IsNewRecord != \"false\"))" + "AgentScriptInternal_condition_1": "state.ProspectId is not None and state.RecordCreationIntent == \"qualify\" and (state.VerificationEnabled == True or state.VerificationEnabled == False and state.IsNewRecord != \"false\")" } ] }, diff --git a/packages/compiler/test/fixtures/expected/post_action_conditionals.camel.json b/packages/compiler/test/fixtures/expected/post_action_conditionals.camel.json index 79179ed9..937dcd9a 100644 --- a/packages/compiler/test/fixtures/expected/post_action_conditionals.camel.json +++ b/packages/compiler/test/fixtures/expected/post_action_conditionals.camel.json @@ -123,7 +123,7 @@ } ], "name": "retrieve_order_details", - "enabled": "state.email is not None and state.order_number is not None" + "enabled": "state.email is not None and state.order_number is not None" }, { "type": "action", diff --git a/packages/compiler/test/fixtures/expected/post_action_conditionals.snake.json b/packages/compiler/test/fixtures/expected/post_action_conditionals.snake.json index 95a4f3a3..635397d3 100644 --- a/packages/compiler/test/fixtures/expected/post_action_conditionals.snake.json +++ b/packages/compiler/test/fixtures/expected/post_action_conditionals.snake.json @@ -123,7 +123,7 @@ } ], "name": "retrieve_order_details", - "enabled": "state.email is not None and state.order_number is not None" + "enabled": "state.email is not None and state.order_number is not None" }, { "type": "action", diff --git a/packages/compiler/test/operator-grouping.test.ts b/packages/compiler/test/operator-grouping.test.ts new file mode 100644 index 00000000..58be8873 --- /dev/null +++ b/packages/compiler/test/operator-grouping.test.ts @@ -0,0 +1,193 @@ +/* + * Copyright (c) 2026, Salesforce, Inc. + * All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * For full license text, see the LICENSE file in the repo root or https://www.apache.org/licenses/LICENSE-2.0 + */ + +/** + * End-to-end checks that operator grouping survives the full compile: + * parse -> compile -> inspect the emitted expression strings. + */ +import { execFileSync, spawnSync } from 'node:child_process'; +import { describe, it, expect } from 'vitest'; +import { compile } from '../src/compile.js'; +import { parseSource } from './test-utils.js'; +import { + RUNTIME_CONDITION_VARIABLE, + STATE_UPDATE_ACTION, +} from '../src/constants.js'; + +const HEADER = ` +config: + agent_name: "TestBot" + agent_type: "AgentforceServiceAgent" + default_agent_user: "test@example.com" + +variables: + a: mutable boolean = False + b: mutable boolean = False + c: mutable boolean = False + total: mutable number = 0 + amount: mutable number = 0 +`; + +function compileOutput(body: string) { + const { output, diagnostics } = compile(parseSource(HEADER + body)); + expect(diagnostics.filter(d => d.severity === 1)).toEqual([]); + return output; +} + +describe('operator grouping end to end', () => { + it('keeps grouping on arithmetic assigned in a set statement', () => { + const output = compileOutput(` +start_agent main: + description: "Test" + reasoning: + instructions: -> + set @variables.total = (1 + 2) * (3 + 4) + | Do something +`); + const node = output.agent_version.nodes.find( + n => n.developer_name === 'main' + )!; + const updates = (node.before_reasoning_iteration ?? []).flatMap( + a => + (a as { state_updates?: Record[] }).state_updates ?? [] + ); + expect(updates).toContainEqual({ total: '(1 + 2) * (3 + 4)' }); + }); + + it('keeps grouping under not in an if condition', () => { + const output = compileOutput(` +start_agent main: + description: "Test" + reasoning: + instructions: -> + if not (@variables.a and @variables.b): + set @variables.c = True + | Do something +`); + const node = output.agent_version.nodes.find( + n => n.developer_name === 'main' + )!; + const updates = (node.before_reasoning_iteration ?? []).flatMap( + a => + (a as { state_updates?: Record[] }).state_updates ?? [] + ); + expect( + updates.some(u => Object.values(u).includes('not (state.a and state.b)')) + ).toBe(true); + }); + + it('keeps grouping in an available when clause', () => { + const output = compileOutput(` +start_agent main: + description: "Test" + reasoning: + instructions: -> + | Test + actions: + go: @utils.transition to @topic.destination + available when (@variables.a or @variables.b) and @variables.c + +topic destination: + description: "destination" +`); + const node = output.agent_version.nodes.find( + n => n.developer_name === 'main' + )!; + const tool = node.tools.find(t => t.target === STATE_UPDATE_ACTION)!; + expect(tool.enabled).toBe('(state.a or state.b) and state.c'); + }); + + it('keeps grouping inside a template interpolation', () => { + const output = compileOutput(` +start_agent main: + description: "Test" + reasoning: + instructions: -> + | Total {!(@variables.total + 1) * 2} +`); + expect(JSON.stringify(output)).toContain('{{(state.total + 1) * 2}}'); + }); +}); + +describe('comparison chains end to end', () => { + /** Compile `if :` and return the emitted condition expression. */ + function compileCondition(condition: string): string { + const output = compileOutput(` +start_agent main: + description: "Test" + reasoning: + instructions: -> + if ${condition}: + set @variables.c = True + | Test +`); + const node = output.agent_version.nodes.find( + n => n.developer_name === 'main' + )!; + const updates = (node.before_reasoning_iteration ?? []).flatMap( + a => + (a as { state_updates?: Record[] }).state_updates ?? [] + ); + const slot = `${RUNTIME_CONDITION_VARIABLE}_1`; + const update = updates.find(u => slot in u); + expect(update).toBeDefined(); + return update![slot]; + } + + /** Evaluate a compiled expression with Python, the language it runs as. */ + function evaluate(expression: string, state: Record) { + const script = [ + 'import json, sys, types', + 'state = types.SimpleNamespace(**json.loads(sys.argv[2]))', + 'print(json.dumps(eval(sys.argv[1], {"state": state})))', + ].join('\n'); + const stdout = execFileSync( + 'python3', + ['-c', script, expression, JSON.stringify(state)], + { encoding: 'utf8' } + ); + return JSON.parse(stdout) as unknown; + } + + const hasPython = spawnSync('python3', ['--version']).status === 0; + + it('keeps a numeric range guard chained', () => { + expect(compileCondition('0 < @variables.amount < 10')).toBe( + '0 < state.amount < 10' + ); + }); + + it('keeps a string chain chained', () => { + expect(compileCondition('"a" < "b" < "c"')).toBe('"a" < "b" < "c"'); + }); + + it('keeps an explicitly grouped comparison grouped', () => { + expect(compileCondition('(3 > 2) > 1')).toBe('(3 > 2) > 1'); + }); + + describe.skipIf(!hasPython)('evaluated as Python', () => { + it.each([ + [100, false], + [-5, false], + [0, false], + [10, false], + [5, true], + ])('a range guard with amount %s is %s', (amount, expected) => { + const condition = compileCondition('0 < @variables.amount < 10'); + expect(evaluate(condition, { amount })).toBe(expected); + }); + + it('a string chain is True', () => { + expect(evaluate(compileCondition('"a" < "b" < "c"'), {})).toBe(true); + }); + + it('a grouped comparison compares the inner result', () => { + expect(evaluate(compileCondition('3 > 2 > 1'), {})).toBe(true); + expect(evaluate(compileCondition('(3 > 2) > 1'), {})).toBe(false); + }); + }); +});