Preserve grouping and typing in TA reset length lowering

pineforge_codegen/analyzer/diagnostics.py

@@ -89,8 +89,22 @@ def _warn_if_unknown_source_id(
             loc,
         )
 
+    # Non-atomic node kinds whose serialized infix form must be parenthesized
+    # when used as an operand, so the string re-parses to the SAME tree. A TA
+    # ctor-arg string produced here is later re-parsed and lowered by the
+    # codegen (``_runtime_ctor_arg_for_reset``); a flattened ``(a - b) / c``
+    # would otherwise silently reassociate under C++ precedence.
+    _NONATOMIC_EXPR_NODES = (BinOp, UnaryOp, Ternary)
+
+    def _operand_to_str(self, node: ASTNode) -> str:
+        s = self._expr_to_str(node)
+        if isinstance(node, self._NONATOMIC_EXPR_NODES):
+            return f"({s})"
+        return s
+
     def _expr_to_str(self, node: ASTNode) -> str:
-        """Convert an expression node to a rough string representation."""
+        """Convert an expression node to a string that re-parses to the same
+        tree (grouping preserved for non-atomic operands)."""
         if isinstance(node, NumberLiteral):
             return str(node.value)
         if isinstance(node, StringLiteral):
@@ -104,15 +118,15 @@ def _expr_to_str(self, node: ASTNode) -> str:
         if isinstance(node, MemberAccess):
             return f"{self._expr_to_str(node.object)}.{node.member}"
         if isinstance(node, BinOp):
-            return f"{self._expr_to_str(node.left)} {node.op} {self._expr_to_str(node.right)}"
+            return f"{self._operand_to_str(node.left)} {node.op} {self._operand_to_str(node.right)}"
         if isinstance(node, UnaryOp):
-            return f"{node.op}{self._expr_to_str(node.operand)}"
+            return f"{node.op}{self._operand_to_str(node.operand)}"
         if isinstance(node, FuncCall):
             args = ", ".join(self._expr_to_str(a) for a in node.args)
             callee_str = self._expr_to_str(node.callee)
             return f"{callee_str}({args})"
         if isinstance(node, Subscript):
             return f"{self._expr_to_str(node.object)}[{self._expr_to_str(node.index)}]"
         if isinstance(node, Ternary):
-            return f"{self._expr_to_str(node.condition)} ? {self._expr_to_str(node.true_val)} : {self._expr_to_str(node.false_val)}"
+            return f"{self._operand_to_str(node.condition)} ? {self._operand_to_str(node.true_val)} : {self._operand_to_str(node.false_val)}"
         return "<?>"

pineforge_codegen/codegen/base.py

@@ -191,6 +191,13 @@ def __init__(self, ctx: AnalyzerContext) -> None:
         self._func_cs_var_remap: dict[tuple[str, int], dict[str, str]] = {}
         # Active var name remap (set during per-call-site function emission)
         self._active_var_remap: dict[str, str] = {}
+        # When True (only while lowering a TA runtime-reset length expression
+        # through the expression visitor), an input-backed variable identifier
+        # renders as an override-aware ``get_input_*()`` read instead of its
+        # class member name. The reset can run in ``evaluate_security`` BEFORE
+        # the input members are initialised, so it must not depend on their
+        # init order. See ``_lower_reset_expr_via_visitor``.
+        self._reset_input_getter_mode: bool = False
         # Set of var/series member names that belong to user functions (need cloning)
         self._func_var_members_set: set[str] = set()
         # BUG C: function-local names emitted as ``UDT*`` pointer aliases (a UDT
@@ -1434,11 +1441,31 @@ def _expr_is_stable(self, node) -> bool:
             return False
         return False
 
+    # AST node kinds whose serialized form is self-delimiting (a literal, a
+    # name, a member read, or a ``name(...)`` call). Non-atomic kinds (BinOp /
+    # UnaryOp / Ternary) MUST be parenthesized when they appear as an operand,
+    # otherwise re-parsing the flattened infix string silently reassociates the
+    # tree: Pine grouping ``(a - b) / (c - d)`` degrades to ``a - b / c - d``
+    # under C++ precedence. See ``_runtime_ctor_arg_for_reset`` (the string is
+    # re-parsed and lowered through the expression visitor).
+    _ATOMIC_ARITH_NODES = (NumberLiteral, Identifier, MemberAccess, FuncCall)
+
+    def _arith_operand_to_str(self, node) -> str | None:
+        """Serialize ``node`` for use as an operand: parenthesize it unless its
+        serialized form is already self-delimiting, so grouping survives a
+        round-trip through the parser."""
+        s = self._arith_expr_to_str(node)
+        if s is None:
+            return None
+        if isinstance(node, self._ATOMIC_ARITH_NODES):
+            return s
+        return f"({s})"
+
     def _arith_expr_to_str(self, node) -> str | None:
         """Render a numeric arithmetic-over-identifiers expression to a string
-        whose token spelling matches ``_resolve_known`` / ``_runtime_ctor_arg_for_reset``
-        (e.g. ``rsiLen * 2 - 1``). Returns None for any node shape we don't fold
-        (series subscripts, ternaries, etc.) so the caller leaves the var untracked.
+        that re-parses to the SAME tree (grouping preserved via
+        ``_arith_operand_to_str``). Returns None for any node shape we don't
+        fold (series subscripts, etc.) so the caller leaves the var untracked.
         """
         if isinstance(node, NumberLiteral):
             v = node.value
@@ -1450,20 +1477,20 @@ def _arith_expr_to_str(self, node) -> str | None:
         if isinstance(node, MemberAccess) and isinstance(node.object, Identifier):
             return f"{node.object.name}.{node.member}"
         if isinstance(node, BinOp):
-            l = self._arith_expr_to_str(node.left)
-            r = self._arith_expr_to_str(node.right)
+            l = self._arith_operand_to_str(node.left)
+            r = self._arith_operand_to_str(node.right)
             if l is None or r is None:
                 return None
             return f"{l} {node.op} {r}"
         if isinstance(node, UnaryOp):
-            o = self._arith_expr_to_str(node.operand)
+            o = self._arith_operand_to_str(node.operand)
             if o is None:
                 return None
             return f"{node.op}{o}"
         if isinstance(node, Ternary):
-            c = self._arith_expr_to_str(node.condition)
-            t = self._arith_expr_to_str(node.true_val)
-            f = self._arith_expr_to_str(node.false_val)
+            c = self._arith_operand_to_str(node.condition)
+            t = self._arith_operand_to_str(node.true_val)
+            f = self._arith_operand_to_str(node.false_val)
             if c is None or t is None or f is None:
                 return None
             return f"{c} ? {t} : {f}"
@@ -2479,6 +2506,18 @@ def _rep(p: re.Match) -> str:
         if not (has_input or has_timeframe or has_inline_input):
             return None
 
+        # Preferred path: re-parse the (gate-approved, grouping-faithful)
+        # expression and lower it through the SAME expression visitor the
+        # statement path uses. Operator grouping and Pine numeric typing
+        # (``/`` always yields float; ``(double)`` coercion) are then correct
+        # by construction — reused from ``_visit_binop``, not re-derived here.
+        # Falls back to the legacy token-substitution renderer below only if
+        # re-parse / lowering unexpectedly fails, so a working site is never
+        # worse off than before this change.
+        lowered = self._lower_reset_expr_via_visitor(expanded)
+        if lowered is not None:
+            return lowered
+
         # Render inline input() calls to override-aware get_input_*() reads
         # now that the gate has accepted the expression. Done after the gate
         # so the rendered getter tokens (get_input_int, ...) do not have to
@@ -2546,6 +2585,50 @@ def _sub(p: re.Match) -> str:
             return f"(int)({rewritten})"
         return rewritten
 
+    def _lower_reset_expr_via_visitor(self, expanded: str) -> str | None:
+        """Re-parse a gate-approved, fully-expanded TA-length expression and
+        lower it through the SAME expression visitor the statement path uses,
+        so operator grouping and Pine numeric typing are preserved identically
+        (Pine ``/`` always yields float; ``math.*`` returns double; branches are
+        parenthesized). Reuses ``_visit_binop`` etc.; nothing is re-typed here.
+
+        Input-backed variables render as override-aware ``get_input_*()`` reads
+        (not member refs) via ``_reset_input_getter_mode`` — the reset can run
+        in ``evaluate_security`` before the input members are initialised, so it
+        must not depend on their init order. Inline ``input(...)`` calls and
+        ``math.*`` / ``timeframe.*`` members lower through the visitor's own
+        handlers (same C++ the statement path would emit).
+
+        Returns None if re-parse / lowering fails, so the caller falls back to
+        the legacy token-substitution renderer (never worse than before)."""
+        try:
+            from ..lexer import Lexer
+            from ..parser import Parser
+            tokens = Lexer(expanded).tokenize()
+            node = Parser(tokens, source=expanded)._parse_expression()
+        except Exception:
+            return None
+        prev = self._reset_input_getter_mode
+        self._reset_input_getter_mode = True
+        try:
+            rendered = self._visit_expr(node)
+        except Exception:
+            return None
+        finally:
+            self._reset_input_getter_mode = prev
+        if not rendered or "/* " in rendered:
+            # Unknown/unhandled node leaked a placeholder — defer to legacy.
+            return None
+        # A TA length must be int. Truncate when the lowered form is
+        # float-typed (a ``std::*`` call, a ``(double)`` division coercion, or a
+        # ``timeframe.*`` helper — all timeframe helpers reference script_tf_).
+        # A bare int / identifier length carries none of these and is left
+        # unwrapped, so simple sites stay byte-identical to the legacy output.
+        if ("std::" in rendered or "(double)" in rendered
+                or "script_tf_" in rendered):
+            return f"(int)({rendered})"
+        return rendered
+
 
     def _collect_ta_runtime_resets(self) -> list[str]:
         """Collect reassignment statements for every TA object whose ctor args

pineforge_codegen/codegen/visit_expr.py

@@ -275,6 +275,16 @@ def _visit_ident(self, node: Identifier) -> str:
                 return str(val)
             if isinstance(val, str):
                 return f'std::string("{val}")'
+        # TA runtime-reset lowering: an input-backed var renders as its
+        # override-aware getter (not the member name), because the reset may
+        # run before the input members are initialised (evaluate_security path).
+        if (self._reset_input_getter_mode
+                and name in self._input_backed_vars
+                and name in self._input_var_to_call):
+            call_node = self._input_var_to_call[name]
+            func_name_i, namespace_i = self._resolve_callee(call_node.callee)
+            title = self._get_input_title(call_node, var_name=name)
+            return self._render_input_value(call_node, func_name_i, namespace_i, title)
         # Pine type name `color` used as a value (no variable) → int64 color constant.
         # Params handled above; symbol table does not retain function locals after analysis.
         if name == "color":

tests/test_codegen_ta_derived_length.py

@@ -66,7 +66,9 @@ def test_class_scope_arithmetic_over_input_length():
     # And the runtime reset re-derives it from the (possibly overridden) input.
     reset = _reset_line(cpp, member)
     assert 'get_input_int("RSI Length", 14)' in reset
-    assert "* 2 - 1" in reset
+    # Reset lowers through the expression visitor, which parenthesizes operands
+    # (grouping preserved: ``(rsiLen * 2) - 1``, not a flattened infix string).
+    assert '(get_input_int("RSI Length", 14) * 2) - 1' in reset
     assert "ta::EMA(1)" not in reset  # must NOT be the silent no-op
 
 
@@ -95,7 +97,7 @@ def test_function_local_derived_length():
     for m in sized:
         reset = _reset_line(cpp, m)
         assert 'get_input_int("Smooth EMA Length", 5)' in reset
-        assert "* 2 - 1" in reset
+        assert '(get_input_int("Smooth EMA Length", 5) * 2) - 1' in reset
     assert "ta::EMA(1)" not in cpp
 
 
@@ -529,3 +531,68 @@ def test_cloned_no_ctor_ta_sites_declared_through_dead_caller():
         "dead adx_dead body should not be emitted"
     )
 
+
+# ---------------------------------------------------------------------------
+# Regression: the stable-runtime-length reset re-materializes the length by
+# re-parsing the expanded expression and lowering it through the SAME
+# expression visitor the statement path uses. Two Pine semantics MUST survive:
+#   (1) operator grouping (a parenthesized +/- subexpression under a division),
+#   (2) float division of two int operands (Pine `/` never truncates).
+# Both used to be destroyed by the old lossy AST->infix serializer +
+# token-substitution renderer (grouping flattened, int/int stayed integer
+# division), collapsing a derived TA length to 1 and trading zero times.
+# ---------------------------------------------------------------------------
+
+def test_reset_preserves_grouping_across_division():
+    # length = int(round((a + b) / (a - b))). Pine groups the sum over the
+    # difference: (10+3)/(10-3) = 13/7 ≈ 1.857 → 2. If the serializer drops the
+    # parentheses the string reassociates under C++ precedence to
+    # `a + b / a - b` = 10 + 0.3 - 3 ≈ 7.3 → 7 — a different length. The reset
+    # must emit the difference as one grouped divisor, coerced to double.
+    src = """//@version=6
+strategy("grp")
+a = input.int(10, "A")
+b = input.int(3, "B")
+lenv = int(math.round((a + b) / (a - b)))
+plot(ta.ema(close, lenv))
+"""
+    cpp = transpile(src)
+    members = re.findall(r"(_ta_ema_\d+)\(", cpp)
+    assert members, "no EMA member emitted"
+    reset = _reset_line(cpp, members[0])
+    assert reset, "no runtime reset emitted for grouped-division length"
+    # Pine `/` is float division → both operands coerced to double.
+    assert "(double)(" in reset
+    # The difference (a - b) survives as one grouped divisor unit — NOT
+    # reassociated so that `+ b` and `- b` become sibling additive terms.
+    assert '(get_input_int("A", 10) - get_input_int("B", 3))' in reset
+    assert '(get_input_int("A", 10) + get_input_int("B", 3))' in reset
+    # The pre-fix flattened form would place the sum's `+ ... /` and a trailing
+    # `- get_input_int("B", 3)` as siblings of the top-level division.
+    assert '+ get_input_int("B", 3) /' not in reset
+    assert "ta::EMA(1)" not in reset  # must NOT collapse to the silent no-op
+
+
+def test_reset_uses_float_division_for_two_int_operands():
+    # 2 / poles must be FLOAT division (Pine semantics): 2/4 = 0.5, so
+    # pow(2, 0.5) = 1.414…, *10 → 14.1 → round 14. Integer division would give
+    # 2/4 = 0 → pow(2, 0) = 1 → *10 → round 10 — the discriminant/length
+    # collapse behind the adxae zero-trades bug.
+    src = """//@version=6
+strategy("fdiv")
+poles = input.int(4, "Poles")
+lenv = int(math.round(10 * math.pow(2.0, 2 / poles)))
+plot(ta.ema(close, lenv))
+"""
+    cpp = transpile(src)
+    members = re.findall(r"(_ta_ema_\d+)\(", cpp)
+    assert members, "no EMA member emitted"
+    reset = _reset_line(cpp, members[0])
+    assert reset, "no runtime reset emitted for float-division length"
+    # The int input operand is coerced to double for the division; the bare
+    # `2 / get_input_int(...)` integer-division form (the bug) must be gone.
+    assert '(double)(get_input_int("Poles", 4))' in reset
+    assert '2 / get_input_int("Poles", 4)' not in reset
+    assert "std::pow" in reset
+    assert "ta::EMA(1)" not in reset
+