Skip to content

Interpreter: Pine v6 na propagation + na comparison semantics#51

Open
FellowTraveler wants to merge 5 commits into
ferranbt:mainfrom
FellowTraveler:pine-v6-na-semantics
Open

Interpreter: Pine v6 na propagation + na comparison semantics#51
FellowTraveler wants to merge 5 commits into
ferranbt:mainfrom
FellowTraveler:pine-v6-na-semantics

Conversation

@FellowTraveler

Copy link
Copy Markdown

Summary

This is the na-semantics slice of #49, split out (as offered there) so each piece can be reviewed on its own. The code is unchanged from #49 — the commits are cherry-picked as-is. The three split PRs are independent: each compiles and passes the full workspace suite standalone (cargo test, just fmt-check, just clippy all clean on this branch), and any merge order is conflict-free.

Both changes were found while running real Pine strategies through pinecone bar-by-bar and diffing against TradingView's strategy tester.

1. feat(interpreter): Pine v6 na propagation

Operators previously raised Expected number type errors when an operand was na, and NaN was truthy (n != 0.0 is true for NaN in IEEE 754). TV's semantics (na in the reference):

  • arithmetic and ordering comparisons with na yield na;
  • and/or use three-valued logic — false and na is false, true or na is true, otherwise na;
  • an na condition in if/while/ternary takes the false/else branch;
  • na doubles as a function: na(x) tests whether x is na.

Internally, new to_number()/to_bool() return Option (None = na) so na can propagate; the existing as_number()/as_bool() keep their signatures for builtins (na → NaN / false), so builtin implementations are unaffected.

Test: new integration script testdata/basics/na_propagation.pine pins all of the above.

2. fix(interpreter): ==/!= with an na operand yield na

==/!= were missed by the na-propagation commit because they route through values_equal (structural equality) rather than to_number(), so 1 != na returned true — e.g. dayofweek != dayofweek[1] fired on the first bar of a series. values_equal itself is unchanged (switch keeps structural matching).

Tests: crates/pine-interpreter/tests/na_comparison.rs (4 tests).

Note: the two commits are ordered — one of the na_comparison.rs tests exercises an na != result used as an if condition, which requires the na-falsy-in-if behavior from the first commit — so they travel together in this PR.

Validation

Beyond the tests in this PR, the combined change set (this PR plus its two siblings from #49) was validated externally: we run full Pine strategies through pinecone one bar at a time and diff the emitted entries/exits against TradingView's own strategy-tester CSV export for the same symbol/timeframe. With all of them in place, two real strategies match TV 100% — 56/56 signals on a 4H strategy and 21/21 on a daily strategy, including exit-reason order comments — over their full comparison windows.

Arithmetic, comparison, and unary operators previously raised
'Expected number' type errors when an operand was na; conditions
treated na as an error too, and NaN was truthy (n != 0.0 is true for
NaN in IEEE 754). Pine v6 semantics:

- to_number()/to_bool() return Option (None = na) so callers can
  propagate na; as_number()/as_bool() keep their signatures for
  builtins (na -> NaN / false).
- Arithmetic and ordering comparisons with na yield na.
- and/or use three-valued logic: false absorbs na, true absorbs na;
  otherwise na.
- if/while/ternary conditions use truthy_for_condition(): na takes the
  false/else branch.
- NaN is no longer truthy in numeric->bool coercion.
- The na keyword doubles as a function: na(x) tests whether x is na.

New integration script basics/na_propagation.pine pins all of the
above (arithmetic, comparison, three-valued and/or, na(), na-condition
if-expression).
Follow-up to the na-propagation pass: == and != were missed because they
route through values_equal (structural equality) rather than to_number(),
so a comparison with na leaked a structural bool ('1 != na' -> true,
'na == na' -> true). Pine semantics: any comparison with an na operand
yields na (falsy in conditions); testing for na requires the na()
function. Reference: TradingView Pine Script v6 User Manual, 'na value'
(https://www.tradingview.com/pine-script-docs/language/type-system/#na-value).

The practical failure mode is first-bar seeding: series like
'dayofweek != dayofweek[1]' must be na (falsy) on the first bar, where
[1] is na — with structural equality they evaluate true and fire
new-day/new-session logic one bar early.

values_equal itself is unchanged: switch-pattern matching still uses
structural equality, matching Pine's switch behavior.

New integration test crates/pine-interpreter/tests/na_comparison.rs pins
eq/neq with na operands (na), na == na (na), non-na comparisons (bool),
and the falsy-in-if regression.
na in Pine is float NaN, and NaN can reach Eq/NotEq wrapped as a plain
Number rather than the Na value — e.g. ta functions folding an all-NaN
window return NaN as a number. The existing guard only short-circuited
on the Na value, so `NaN != NaN` evaluated true via structural
comparison instead of yielding na (TradingView yields na for any ==/!=
with an na operand, and a ternary on that condition takes the else
branch).

Adds an is_na_operand helper (Na or NaN-valued Number) guarding both
arms; values_equal and the relational operators are unchanged. Tests
cover the direct comparison results and the ternary-else behavior.
@FellowTraveler

Copy link
Copy Markdown
Author

Pushed a follow-up commit (835ecd4) that extends the na-operand handling to the Value::Number(NaN) case — the companion to the earlier Value::Na-operand fix. In Pine na is float NaN, and NaN can reach ==/!= wrapped as a plain Number rather than the Na value (e.g. ta.* functions folding an all-NaN window return NaN as a number). The prior guard only short-circuited on the Na value, so NaN != NaN still evaluated true via structural comparison instead of yielding na. The commit adds an is_na_operand helper (matches Na or a NaN-valued Number) guarding both the Eq and NotEq arms; values_equal and the relational operators are unchanged. New tests cover the direct comparison results and the ternary-else behavior.

Note: not (x == y) where the comparison yields na now differs from a strict two-valued reading — na propagates through not (na stays falsy in conditions), matching TradingView.

…ds as na

to_number() maps Value::Na to None, which the relational match arms
already turn into Value::Na — but a Value::Number(NaN) operand (a
computed NaN such as math.sqrt(-1.0), or a ta function folding an
all-NaN window) passes through as Some(NaN), and the raw float
comparison yields Bool(false). That structural bool leaks into boolean
context — e.g. `not (x < y)` flips it truthy — where TradingView
produces na (falsy in conditions).

Guards Less/Greater/LessEq/GreaterEq with the same is_na_operand check
Eq/NotEq already use; the to_number match is kept as the fallback for
Series-wrapped na. Also corrects the comment above Eq, which claimed
the relational arms already returned Value::Na for these operands —
true only for the Na value, not for Number(NaN).

Tests cover relationals with an na operand, with a NaN-valued Number
operand (Bool(false) pre-fix), and the no-na boolean cases.
In Pine, `x / 0` and `x % 0` evaluate to na — a plain value, not a
runtime error; the script continues. The Div and Mod arms instead
returned Err(RuntimeError::DivisionByZero), aborting execution of a
script TradingView runs to completion.

The errors-corpus fixture testdata/errors/division_by_zero.pine pinned
that must-error behavior, which does not conform to TradingView; it is
reclassified to testdata/basics/division_by_zero.pine asserting the na
results (same pattern as basics/na_propagation.pine). This includes
integer-literal division: `1 / 0` yields na on TradingView as well —
asserted here from Pine's na semantics (na is the result of a zero
divisor) rather than any int-specific rule, since Pine defines no
int-division error either.

The RuntimeError::DivisionByZero variant is retained: it is public API
of this crate, and removing it is an upstream decision. It simply has
no construction sites after this change.

Tests: tests/div_by_zero.rs pins 1/0, 0/0, -3%0, and na/0 to na, plus
non-zero division/modulo sanity checks.
@FellowTraveler

Copy link
Copy Markdown
Author

Pushed two more follow-up commits:

9a1ddf5 — relational operators treat NaN-valued Number operands as na. This completes the comparison class started in 835ecd4: to_number() maps Value::Na to None (which the relational match arms already turn into Value::Na), but a Value::Number(NaN) operand — a computed NaN such as math.sqrt(-1.0), or a ta.* all-NaN window — passed through as Some(NaN) and the raw float comparison yielded Bool(false), which e.g. not then flips truthy where TradingView produces na (falsy). The four relational arms now get the same is_na_operand guard Eq/NotEq use; the to_number match remains as the fallback for Series-wrapped na. All six comparison operators now handle both na forms uniformly.

d100a99 — division and modulo by zero yield na. In Pine, x / 0 and x % 0 evaluate to na — a plain value, not a runtime error; the script continues. The Div/Mod arms instead returned Err(RuntimeError::DivisionByZero), aborting the whole script. The errors-corpus fixture testdata/errors/division_by_zero.pine pinned that must-error behavior, which doesn't conform to TradingView, so it's reclassified to testdata/basics/division_by_zero.pine asserting the na results (same pattern as basics/na_propagation.pine). One honest caveat: the integer-literal case (1 / 0 → na) is asserted from Pine's general na-on-zero-divisor semantics — Pine defines no int-specific division error — rather than from an int-specific documented rule. The RuntimeError::DivisionByZero variant is retained since removing public API feels like your call; it just has no construction sites now.

New tests: relational × na, relational × Number(NaN) (failed pre-fix), no-na boolean sanity; div_by_zero.rs pins 1/0, 0/0, -3%0, na/0 → na plus non-zero sanity. Full workspace suite, fmt, and clippy clean at each commit.

@@ -0,0 +1,54 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write this as interpreter testdata cases.

@@ -1,5 +0,0 @@
// Test division by zero - should fail

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this test removed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants