From c22446d6a929b3a25d564a653eb76db46b1ee5f0 Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Thu, 13 Aug 2026 07:15:25 +0800 Subject: [PATCH] fix: evaluate request.security ta.* lazily under Pine conditionals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pine evaluates `ta.*` LAZILY. A call sitting in an untaken ternary branch or in a short-circuited `and`/`or` operand is not evaluated on that bar, so its series state does not advance. `_emit_security_evaluators` hoisted EVERY TA site collected from a `request.security` expression into an unconditional prologue: auto _secval_9 = slot_is_new ? _sec0__ta_ema_10.compute(bar.close) : ... auto _secval_10 = ... ... _req_sec_0 = (_secval_9 > _secval_10 && close > _secval_11) ? 1 : ... so every site advanced on every HTF bar regardless of whether Pine reached it. On the common MTF-trend shape request.security(sym, tf, (ta.ema(close,20) > ta.ema(close,50) and close > ta.ema(close,200)) ? 1 : (ta.ema(close,20) < ta.ema(close,50) and close < ta.ema(close,200)) ? -1 : 0) the four conditionally-reached EMAs desynchronised against TradingView within the first weeks and never recovered. `_security_lazy_ta_keys` classifies each `(ta_index, binding_signature)` by where Pine actually reaches it, and conditionally-reached sites are dropped from the eager prologue. `_build_security_expr` already had an inline `(security_series_slot_is_new(N) ? m.compute(a) : m.recompute(a))` fallback for sites absent from `ta_results`; emitting there puts the advance inside the expression, where C++'s own `&&` / `||` / `?:` short-circuit fires it on exactly the bars Pine does. The relational lowering's `_pna_l` / `_pna_r` temporaries keep operand evaluation left-to-right, and `recompute()` restores the pre-advance snapshot before recomputing, so a slot first reached on a later chart bar still advances exactly once. The classifier is deliberately conservative — anything not provably single-reach AND conditional keeps the eager hoist: * A site reached more than once stays hoisted. Sharing one `_secval_*` is what bounds it to a single advance per bar; two inline copies would advance it twice. (Distinct helper call sites already get distinct TA variant members, so each variant is classified on its own.) * Laziness does not propagate through a global binding. In the requested context that global is its own unconditional top-level statement and evaluates on every HTF bar however it is read — this is what keeps `t0`-shaped chart code exact. * History-offset sites (`ta.ema(close,55)[1]`) keep the committed `_secval_*` their per-bar Series push reads. * Securities that rebind mutable globals, and multi-statement helper calls, bail entirely: both lower through statement emitters that consume `ta_results` outside this expression. The chart-context path is untouched — it never hoisted, so its `?:` / `&&` already short-circuit the compute inline. Evidence: on mylivingedge-mylivingedgefx-panel the regenerated evaluator is semantically identical to a hand-patched lazy reference and produces a byte-identical tape; against the TV deep-backtest tape (1432 entries, 13 months, +8h) it goes from 1236/1432 matched with count delta +165 to 1432/1432 with count delta 0. Regenerating all 416 standard-corpus slugs old-vs-new: 413 byte-identical (including every one of the 51 other slugs with conditional CHART ta.*, and security-only slugs such as thulashimohanr-prev-day-week-levels-or-vwap-strategy); the 3 that change are the only ones with ta.* reachable only through a security-expression conditional. The two besides mylivingedge were re-run end to end and their tapes are byte-identical to the pre-fix tapes (heneralmomo25-selda-97ma 19/19, remarkablefreddy-scale-strat-trade-plan-wedge-trendlines 8396/8396). Co-Authored-By: Claude Fable 5 --- pineforge_codegen/codegen/security.py | 160 ++++++++++++ ...est_security_lazy_ta_under_conditionals.py | 246 ++++++++++++++++++ 2 files changed, 406 insertions(+) create mode 100644 tests/test_security_lazy_ta_under_conditionals.py diff --git a/pineforge_codegen/codegen/security.py b/pineforge_codegen/codegen/security.py index 06e81d8..95934ea 100644 --- a/pineforge_codegen/codegen/security.py +++ b/pineforge_codegen/codegen/security.py @@ -2844,6 +2844,156 @@ def _collect_security_ta_indices(self, expr_node, resolving: set[str] | None = N ) return out + def _security_lazy_ta_keys( + self, + sec_id: int, + expr_node, + info: dict, + ) -> set[tuple[int, tuple]]: + """``(ta_index, binding_signature)`` pairs Pine reaches only lazily. + + Pine evaluates ``and``/``or`` with left-to-right short-circuit and a + ternary evaluates its condition plus exactly one branch. A ``ta.*`` + call sitting in a skipped operand is **not evaluated on that bar**, so + its series state does not advance. The evaluator used to hoist every + collected site into an unconditional ``auto _secval_N = ...compute(...)`` + prologue, which advanced every site on every HTF bar and desynchronised + the series against TradingView. + + A site returned here is dropped from that prologue. ``_build_security_expr`` + then falls through to its inline + ``(security_series_slot_is_new(N) ? m.compute(a) : m.recompute(a))`` + form, emitted in expression position — C++'s own ``&&`` / ``||`` / + ``?:`` short-circuit then advances the state exactly when Pine would, + and the relational lowering's ``_pna_l`` / ``_pna_r`` temporaries keep + the evaluation order left-to-right. + + Deliberately conservative — anything not provably single-reach and + conditional keeps the existing eager hoist, so scripts without + conditional security TA regenerate byte-identically: + + - A site reached more than once must stay hoisted: sharing one + ``_secval_*`` is what keeps it to a single advance per bar, whereas + two inline copies would advance it twice (or zero times). + - Laziness does **not** propagate through a global binding. In the + requested context that global is its own unconditional top-level + statement, so it evaluates on every HTF bar wherever it is read + (this is what keeps chart-side ``t0``-shaped code exact). + - Sites with a history offset (``ta.ema(close, 55)[1]``) keep the + committed ``_secval_*`` their per-bar Series push reads. + - Securities that rebind mutable globals, and multi-statement helper + calls, bail entirely: both lower through statement emitters that + consume ``ta_results`` outside this expression. + """ + if info.get("mutable_globals"): + return set() + + occurrences: dict[tuple[int, tuple], list[bool]] = {} + unsafe = False + + def walk(node, lazy: bool, binding_stack, resolving: set[str]) -> None: + nonlocal unsafe + if node is None or unsafe: + return + + if isinstance(node, Identifier): + binding = None + if not self._security_identifier_is_global_binding(node): + binding = self._security_lookup_helper_binding_context( + node.name, binding_stack + ) + if binding is not None: + bound, bound_stack = binding + if isinstance(bound, str): + return + bind_key = f"bind:{id(bound)}" + if bind_key in resolving: + return + resolving.add(bind_key) + walk(bound, lazy, bound_stack, resolving) + resolving.discard(bind_key) + return + + if self._global_mutable_infos.get(node.name) is not None: + unsafe = True + return + + global_expr_map = getattr(self.ctx, "global_expr_map", {}) or {} + if ( + self._security_identifier_is_global_binding(node) + and node.name in global_expr_map + and node.name not in resolving + ): + resolving.add(node.name) + walk(global_expr_map[node.name], False, (), resolving) + resolving.remove(node.name) + return + + if isinstance(node, FuncCall) and isinstance(node.callee, Identifier): + func_name = node.callee.name + if func_name in self._func_names: + call_key = f"func:{func_name}" + if call_key in resolving: + return + resolving.add(call_key) + plan = self._security_helper_call_plan(node, binding_stack) + if plan["mode"] == "expr": + walk(plan["expr"], lazy, plan["binding_stack"], resolving) + else: + unsafe = True + resolving.discard(call_key) + return + + if isinstance(node, BinOp) and node.op in ("and", "or"): + walk(node.left, lazy, binding_stack, resolving) + walk(node.right, True, binding_stack, resolving) + return + + if isinstance(node, Ternary): + walk(node.condition, lazy, binding_stack, resolving) + walk(node.true_val, True, binding_stack, resolving) + walk(node.false_val, True, binding_stack, resolving) + return + + site = self._get_ta_site(node) + if site is not None: + idx = self._ta_index_by_site_id.get(id(site)) + if idx is not None: + key = (idx, self._security_binding_stack_signature(binding_stack)) + occurrences.setdefault(key, []).append(lazy) + + for child in vars(node).values(): + walk_value(child, lazy, binding_stack, resolving) + + def walk_value(value, lazy: bool, binding_stack, resolving: set[str]) -> None: + if value is None or unsafe: + return + if hasattr(value, "__dict__"): + walk(value, lazy, binding_stack, resolving) + return + if isinstance(value, (list, tuple)): + for item in value: + walk_value(item, lazy, binding_stack, resolving) + return + if isinstance(value, dict): + for item in value.values(): + walk_value(item, lazy, binding_stack, resolving) + + walk(expr_node, False, (), set()) + if unsafe: + return set() + + hist_indices = set(self._security_ta_hist_idx_by_sec.get(sec_id, ())) + inline_helper_ta_indices = set(info.get("inline_helper_ta_indices", [])) + return { + key + for key, reaches in occurrences.items() + if key[0] not in hist_indices + and key[0] not in inline_helper_ta_indices + and len(reaches) == 1 + and reaches[0] + } + def _emit_security_ohlc_hist_pushes(self, sec_id: int, lines: list[str]) -> None: """Emit the OHLC history-offset Series pushes for ``sec_id``, gated on ``is_complete``. @@ -2909,6 +3059,7 @@ def _emit_security_evaluators(self, lines: list[str]) -> None: ta_indices = info.get("ta_indices") or [] security_mutable_names = set(info.get("mutable_globals", [])) inline_helper_ta_indices = set(info.get("inline_helper_ta_indices", [])) + lazy_ta_keys = self._security_lazy_ta_keys(sec_id, expr_node, info) lines.append(f" void _eval_security_{sec_id}(const Bar& bar, bool is_complete) {{") @@ -2949,6 +3100,15 @@ def emit_security_ta(indices: list[int]) -> None: site = self.ctx.ta_call_sites[idx] variants = (info.get("ta_variants") or {}).get(idx, []) for variant in variants: + if (idx, variant["signature"]) in lazy_ta_keys: + # Pine only reaches this site through a + # short-circuited operand or an untaken ternary + # branch. Leave it out of the eager prologue: + # _build_security_expr emits its + # compute()/recompute() inline in expression + # position, where C++'s &&/||/?: short-circuit + # advances the series exactly on the bars Pine does. + continue helper_binding_stack = variant.get("binding_stack", ()) compute_args = self._security_ta_compute_args_for_site( sec_id, diff --git a/tests/test_security_lazy_ta_under_conditionals.py b/tests/test_security_lazy_ta_under_conditionals.py new file mode 100644 index 0000000..7186c9e --- /dev/null +++ b/tests/test_security_lazy_ta_under_conditionals.py @@ -0,0 +1,246 @@ +"""Pine evaluates ``ta.*`` LAZILY: a call in an untaken ternary branch or a +short-circuited ``and``/``or`` operand does NOT advance its series state on +that bar. + +``_emit_security_evaluators`` used to hoist *every* collected TA site of a +``request.security`` expression into an unconditional +``auto _secval_N = ...compute(bar.close);`` prologue, so each site advanced on +every HTF bar regardless of whether Pine reached it. On the +``(ema20 > ema50 and close > ema200) ? 1 : (ema20 < ema50 and close < ema200) ? -1 : 0`` +MTF-trend shape that desynchronised the HTF EMAs against TradingView. + +The fix drops conditionally-reached sites from that prologue. +``_build_security_expr`` then emits their +``(security_series_slot_is_new(N) ? m.compute(a) : m.recompute(a))`` inline in +expression position, where C++'s own ``&&`` / ``||`` / ``?:`` short-circuit +advances the series on exactly the bars Pine does. + +The classifier is deliberately conservative, and these tests pin both halves: +what becomes lazy, and what must stay eager (multi-reach sites, globals, +history-offset sites, mutable-global securities) so scripts without +conditional security TA keep regenerating byte-identically. +""" + +from __future__ import annotations + +import re + +from pineforge_codegen import transpile + +HOIST_RE = re.compile(r"auto (_secval_\w+) = security_series_slot_is_new") +INLINE_RE = re.compile( + r"security_series_slot_is_new\(\d+\) \? (_sec\d+_\w+)\.compute" +) + + +def _strategy(body: str) -> str: + return f"""//@version=6 +strategy("t", overlay=true) +{body} +if not na(v) + strategy.entry("L", strategy.long) +plot(close) +""" + + +def _eval_body(cpp: str, sec_id: int = 0) -> str: + m = re.search(rf"void _eval_security_{sec_id}\(.*?\n \}}", cpp, re.S) + assert m is not None, f"no _eval_security_{sec_id} method found" + return m.group(0) + + +def _hoisted(body: str) -> list[str]: + """TA sites evaluated eagerly in the evaluator prologue.""" + return HOIST_RE.findall(body) + + +def _inlined(body: str) -> list[str]: + """TA members evaluated inline in expression position (lazily).""" + assign = body[body.index("_req_sec_"):] + return INLINE_RE.findall(assign) + + +# ---------------------------------------------------------------------- +# Lazy: the site sits behind a Pine short-circuit / untaken branch +# ---------------------------------------------------------------------- + + +def test_ternary_branches_are_lazy(): + """Neither ``?:`` branch may be hoisted: Pine evaluates only one.""" + body = _eval_body(transpile(_strategy( + 'v = request.security(syminfo.tickerid, "60",' + " close > open ? ta.ema(close, 20) : ta.sma(close, 20)," + " lookahead=barmerge.lookahead_off)" + ))) + assert _hoisted(body) == [] + assert sorted(_inlined(body)) == ["_sec0__ta_ema_1", "_sec0__ta_sma_2"] + + +def test_and_right_operand_is_lazy_left_stays_eager(): + """``and`` runs its LHS every bar; its RHS is short-circuited.""" + body = _eval_body(transpile(_strategy( + 'v = request.security(syminfo.tickerid, "60",' + " ta.ema(close, 20) > close and ta.sma(close, 50) > close," + " lookahead=barmerge.lookahead_off)" + ))) + assert _hoisted(body) == ["_secval_0"] + assert "_sec0__ta_ema_1.compute" in body.split("_req_sec_")[0] + assert _inlined(body) == ["_sec0__ta_sma_2"] + + +def test_or_right_operand_is_lazy_left_stays_eager(): + """``or`` short-circuits its RHS exactly like ``and``.""" + body = _eval_body(transpile(_strategy( + 'v = request.security(syminfo.tickerid, "60",' + " ta.ema(close, 20) > close or ta.sma(close, 50) > close," + " lookahead=barmerge.lookahead_off)" + ))) + assert _hoisted(body) == ["_secval_0"] + assert _inlined(body) == ["_sec0__ta_sma_2"] + + +def test_nested_ternary_and_composition_matches_pine_reach(): + """The MTF-trend shape that motivated the fix. + + ``(e20 > e50 and close > e200) ? 1 : (e20b < e50b and close < e200b) ? -1 : 0`` + + Only the two operands of the outer condition's ``and`` LHS are reached on + every bar. ``e200`` sits behind that ``and``; the whole inner ternary sits + in the outer's untaken branch; ``e200b`` is behind a second ``and``. + """ + body = _eval_body(transpile(_strategy( + 'v = request.security(syminfo.tickerid, "60",' + " (ta.ema(close,20) > ta.ema(close,50) and close > ta.ema(close,200)) ? 1" + " : (ta.ema(close,20) < ta.ema(close,50) and close < ta.ema(close,200)) ? -1 : 0," + " lookahead=barmerge.lookahead_off)" + ))) + assert _hoisted(body) == ["_secval_0", "_secval_1"] + assert _inlined(body) == [ + "_sec0__ta_ema_3", + "_sec0__ta_ema_4", + "_sec0__ta_ema_5", + "_sec0__ta_ema_6", + ] + # The lazy sites must land inside the short-circuiting structure, i.e. + # after the ternary's `?` / the `&&`, never before the assignment. + assert "_sec0__ta_ema_3" not in body.split("_req_sec_")[0] + + +def test_laziness_propagates_through_single_expression_helper(): + """``f(x) => ta.ema(x, 20)`` called in a branch is reached only there. + + Each call site gets its own TA variant member, so both branches of + ``c ? f(close) : f(close) * 2`` are independently lazy. + """ + body = _eval_body(transpile(_strategy( + "f(x) => ta.ema(x, 20)\n" + 'v = request.security(syminfo.tickerid, "60",' + " close > open ? f(close) : f(close) * 2.0," + " lookahead=barmerge.lookahead_off)" + ))) + assert _hoisted(body) == [] + assert sorted(_inlined(body)) == [ + "_sec0__ta_ema_1_v0", + "_sec0__ta_ema_1_v1", + ] + + +# ---------------------------------------------------------------------- +# Eager: unconditional Pine statements still advance every bar +# ---------------------------------------------------------------------- + + +def test_unconditional_sites_stay_eager(): + """No conditional in sight -> byte-for-byte the pre-fix hoisted form.""" + body = _eval_body(transpile(_strategy( + 'v = request.security(syminfo.tickerid, "60",' + " ta.ema(close, 20) + ta.sma(close, 50)," + " lookahead=barmerge.lookahead_off)" + ))) + assert _hoisted(body) == ["_secval_0", "_secval_1"] + assert _inlined(body) == [] + assert "_req_sec_0 = (_secval_0 + _secval_1);" in body + + +def test_ternary_condition_operands_stay_eager(): + """A ternary's *condition* is evaluated on every bar.""" + body = _eval_body(transpile(_strategy( + 'v = request.security(syminfo.tickerid, "60",' + " ta.ema(close, 20) > ta.sma(close, 50) ? 1.0 : 0.0," + " lookahead=barmerge.lookahead_off)" + ))) + assert _hoisted(body) == ["_secval_0", "_secval_1"] + assert _inlined(body) == [] + + +def test_global_binding_read_under_a_conditional_stays_eager(): + """``e = ta.ema(close, 20)`` is its own unconditional top-level statement. + + In the requested context it evaluates on every HTF bar however it is read, + so laziness must not propagate through the global binding. This is what + keeps chart-side ``t0``-shaped code exact. + """ + body = _eval_body(transpile(_strategy( + "e = ta.ema(close, 20)\n" + 'v = request.security(syminfo.tickerid, "60",' + " close > open ? e : 0.0, lookahead=barmerge.lookahead_off)" + ))) + assert _hoisted(body) == ["_secval_0"] + assert _inlined(body) == [] + + +def test_history_offset_site_under_a_conditional_stays_eager(): + """``ta.ema(close, 20)[1]`` needs its committed ``_secval_*`` to push.""" + body = _eval_body(transpile(_strategy( + 'v = request.security(syminfo.tickerid, "60",' + " close > open ? ta.ema(close, 20)[1] : 0.0," + " lookahead=barmerge.lookahead_off)" + ))) + assert _hoisted(body) == ["_secval_0"] + assert _inlined(body) == [] + assert "_sec0__ta_ema_1_hist.push(_secval_0);" in body + + +def test_mutable_global_security_stays_eager(): + """Securities that rebind mutable globals bail out of the analysis. + + Their rebind statements are lowered by a separate statement emitter that + consumes ``ta_results`` outside this expression, so a site moved inline + could advance twice. + """ + body = _eval_body(transpile(_strategy( + "var float acc = 0.0\n" + "acc := acc + 1.0\n" + 'v = request.security(syminfo.tickerid, "60",' + " close > open ? ta.ema(close, 20) + acc : 0.0," + " lookahead=barmerge.lookahead_off)" + ))) + assert _hoisted(body) == ["_secval_0"] + assert _inlined(body) == [] + + +# ---------------------------------------------------------------------- +# Chart context: already inline, and must stay that way +# ---------------------------------------------------------------------- + + +def test_chart_context_conditional_ta_is_inline(): + """The chart path never hoisted; its ``?:`` / ``&&`` are the short-circuit.""" + cpp = transpile(_strategy( + "c = close > open\n" + "v = c ? ta.ema(close, 20) : 0.0\n" + "w = c and ta.rsi(close, 14) > 50.0" + )) + assign = next(ln for ln in cpp.splitlines() if ln.strip().startswith("v = (")) + assert "? ((history_advances_new_bar() ? _ta_ema_1.compute" in assign + assert "_secval_" not in assign + w_assign = next(ln for ln in cpp.splitlines() if ln.strip().startswith("w = (")) + assert "_ta_rsi_2.compute" in w_assign + + +def test_chart_context_unconditional_ta_unchanged(): + """An unconditional chart ``ta.*`` still evaluates once per bar, inline.""" + cpp = transpile(_strategy("v = ta.ema(close, 20) + ta.sma(close, 50)")) + assign = next(ln for ln in cpp.splitlines() if ln.strip().startswith("v = (")) + assert "_ta_ema_1.compute" in assign + assert "_ta_sma_2.compute" in assign