fix(compiler): preserve parentheses on both binary operands

[BichengWang]

What

Preserve operand grouping when compiling binary, unary, and comparison expressions. (1 + 2) * (3 + 4) previously became 1 + 2 * (3 + 4), changing the result from 21 to 15; it now retains the original grouping.

Why

Fixes #94. The same loss of grouping affects not (True and False) and -(1 + 2), and grouped comparison operands could produce unbalanced parentheses. These failures produced no compiler diagnostic.

How

Use a shared CST grouping helper for binary operands, unary operands, and comparison operands. Extract comparison operators from the original operands' full grouped source boundaries to preserve spacing without including stray parentheses. Parser-backed tests cover arithmetic and boolean grouping, left-only/right-only/both-grouped operands, ungrouped controls, repeated operands, and nested parentheses.

Test Plan

• The original binary regression cases failed before the initial fix (2 failed, 89 passed).

• The added unary/comparison regression cases fail at the previous PR head (8 failed, 95 passed) and pass with this update (103 passed).

• Compiler suite: 1,113 passed, 3 existing skips.

• All 17 grouping/spacing cases also pass using the native tree-sitter parser and CST adapter. The normal test suite uses the default JavaScript parser.

• pnpm test --force: all 24 Turbo tasks passed, no cached tasks; includes all 27 compile-server tests.

• pnpm lint --force: passed with 12 existing warnings in unchanged UI files; changed-file ESLint passed without warnings.

• pnpm typecheck --force: all 24 Turbo tasks passed, no cached tasks.

• pnpm build: all 17 Turbo tasks passed (14 cached); reports documentation broken-link and UI chunk-size warnings.

• pnpm format:check, standalone typecheck of expressions.test.ts, and the path-traversal regression: passed.

• Existing skips: AgentScript dialect 973, LSP 7, compiler 3.

• Existing tests pass (pnpm test)

• New/updated tests cover the change

• Linting and type checks pass (pnpm lint && pnpm typecheck)

Checklist

• My code follows the project's coding style
• I have reviewed my own diff
• I have added/updated documentation as needed (no documentation change needed)
• This change does not introduce new warnings

[awli]

Hi Kenneth!

Thanks for flagging this - it's a major bug, and I'm really glad that you found it. I did a brief review and there's an extension flagged by AI (reproduced below). I've dispatched an agent to attempt to fix it, but you can also review yourself if you wish.

---

Verdict

Approve with changes. The fix is correct and tested. But the same grouping bug survives in unary and comparison expressions, so in not (a and b) the parens are still silently dropped. Fix those in this PR or file a follow-up before closing #94.

Summary

This PR changes compileBinaryExpression (packages/compiler/src/expressions/compile-expression.ts:461-486) to check each operand's grouping separately. Before, it returned as soon as the right operand had parens and never checked the left. The change matches the issue's root-cause analysis, and the description's claims hold up:

• With the fix reverted, the new tests fail (2 failed / 89 passed).
• At the PR head the compiler suite gives 1,101 passed, 3 skipped, and tsc --noEmit is clean.
• All CI checks are green.

It's a small, clean change. The problem is that it fixes one symptom of a wider bug.

Findings

[Major] The same bug remains in unary and comparison expressions

packages/compiler/src/expressions/compile-expression.ts:488-522

The dialect throws away parenthesized_expression when it builds the AST (packages/language/src/core/dialect.ts:1222-1226). The only place that recovers grouping is the CST-parent check inside compileBinaryExpression. compileUnaryExpression and compileComparisonExpression have no such check. Running these through this PR's own parser-backed test harness at the PR head:

Source	Compiled at PR head	Effect
not (True and False)	not True and False	True becomes False
@variables.a and not (@variables.b or @variables.c)	state.a and not state.b or state.c	condition changes meaning
-(1 + 2)	-1 + 2	-3 becomes 1
(True or False) == False	True or False) == False	malformed output, unbalanced paren

None of these produce a diagnostic. The comparison case breaks because the whitespace-preserving slice at lines 509-517 takes the ) along with the operator text. These bugs exist on main too, so this PR doesn't cause them. But #94 explicitly calls out boolean conditions changing silently, and not (x or y) is probably more common in agent conditions than (a) op (b). Closing #94 with this PR would leave the worst version of the bug open.

Suggested fix: Pull the check into an isParenthesized(expr) helper. Use it for the unary operand and for both comparison operands, and make the comparison path work from the parens-inclusive text. Add parser-backed cases for the four rows above. The better long-term fix is to keep grouping in the AST, either as a ParenthesizedExpression node or a flag, so the CST parent/grandparent sniffing can go.

[Minor] The language emitter has the same root cause

packages/language/src/core/expressions.ts:391-393, 431-436

BinaryExpression.__emit and UnaryExpression.__emit ignore grouping completely. (1 + 2) * (3 + 4) emits as 1 + 2 * 3 + 4. This is out of scope for this PR. I'm raising it because it argues for fixing grouping in the AST, which repairs both emitters, rather than adding more per-function checks in the compiler.

Questions

• (Unverified) Is the parent/grandparent check also correct on the tree-sitter backend? All the new tests use the default JS parser. The check was already there before this PR, but the PR now relies on it for the left operand as well.
• Once this is merged, should #94 stay open, or be re-scoped, for the unary and comparison cases?

What's solid

• It fixes the bug at the right spot in the code, and the three-line version is simpler than the early returns it replaces.
• The tests go through the real parser and dialect instead of hand-built ASTs, which is what makes the CST-based check testable. They include left-only, right-only and ungrouped controls, and they really do fail without the fix.

[BichengWang]

Thanks for the review. I extended the grouping helper to unary and comparison operands and fixed the comparison operator slice to include the original grouped boundaries. The four reported cases now pass, with controls for ungrouped operands, repeated operands, spacing, and nested parentheses.

The compiler suite passes 1,113 tests, and all 17 grouping/spacing cases also pass with the native tree-sitter parser and CST adapter. The language emitter remains a separate issue; this update changes compiler output only.

[awli]

Thanks! This looks good to merge. There's one followup for ternaries which I will handle myself. Thanks again for submitting.