Repository navigation
Operator typing: refuse an operator pair TradingView refuses at the operand - #305
luisleo526 wants to merge 2 commits into
Conversation
…perand One analyzer relation types every unary and binary operator, the ?: condition and the compound assignment. A pair is refused only when both operand types come from a certain source (a literal, a declared hint, a typed parameter, a structured type, a variant-resolved builtin call); coarse registry results, VOID and UNKNOWN stay unresolved and are accepted. The proofs live in a side table that only the refusal relation reads: the symbol types, callable return types and storage facts that code generation reads keep their previous values, so no emitted C++ changes. A direct comparison with na is refused from the comparison start. A history read of an array, matrix, map or user-type field used as an operator operand is refused, and twenty such sources are pinned with their TradingView code family. A proven operand that fits no overload of an operator is refused whatever the other operand is, so a collection read beside an unresolved operand no longer reaches code generation. A comparison always has bool type. The call-site check resolves a function's free names in its definition scope. A while condition of type na and a switch discriminator that fits no case type are refused as TradingView refuses them. The operand-pair refusal has its own catalog code, pinned in the digest fixture. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The positional-argument change keeps PF-E2074 on main; the operator typing codes stay PF-E2076 to PF-E2078. The catalog, its digest pin and the code-delta list carry both sets. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; formal verification proved 0 of 19 changed function(s) (19 not verified).
Formal verification. PR-changed functions: 0/19 verified (0 proven, 0 may-equivalent, 0 distinguished) · 19 not verified (19 unsupported).
Not verified on this run: \_\_init\_\_ (unsupported), \_check\_incompatible\_binary\_operands (unsupported), \_numeric\_condition\_type (unsupported), \_visit\_Assignment (unsupported), \_visit\_BinOp (unsupported), \_visit\_FuncCall (unsupported), \_visit\_FuncDef (unsupported), \_visit\_IfStmt (unsupported), \_visit\_MethodDef (unsupported), \_visit\_Subscript (unsupported), and 9 more.
Graphify review — findings
Adds a single operator-typing relation, resolve_operator, that replaces the old ad-hoc bool/string/numeric pair check. Binary, unary, compound-assignment (+=, -=, …), switch-case == and ?: operands are now checked against structured type facts rather than a float fallback. _operator_types_by_node records a type per declaration, so an explicitly unresolved initializer stays open instead of recovering a storage type, and if/ternary conditions route through the same relation. The first ?: condition error is held back until the whole tree has been visited, and the unary and binary mismatch reporting in _check_operator_operands is where the new diagnostics are produced.
Worth a look
- Unary operand mismatch crashes on
operands[-2:]unpack —pineforge_codegen/analyzer/base.py· Escalate · high- NOT verified (no proof, no reproducing execution) — needs human review
- Numeric conditions are rejected outright for ternary,
and,orandnot, whatever the Pine version —pineforge_codegen/analyzer/operators.py:205· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- Specific binary diagnostics are followed by a duplicate generic 'cannot combine' error —
pineforge_codegen/analyzer/base.py· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
notmismatch emits both the bool error and the numeric-operand error —pineforge_codegen/analyzer/base.py· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 3490 functions depend on the 736 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
Analyzer— 61 callers, 13 callees - worse:
resolve()— 22 callers, 3 callees - new:
resolve_operator()— 16 callers, 3 callees - new:
test_polymorphic_call_facts_survive_aliases_without_a_scalar_default()— 0 callers, 9 callees - new:
test_math_result_retains_number_domain_when_width_is_open()— 0 callers, 8 callees - new:
test_comparison_result_is_bool_even_when_operand_types_are_unresolved()— 0 callers, 8 callees - new:
test_callable_numeric_return_types_remain_per_written_call()— 0 callers, 8 callees - new:
test_typed_missing_string_retains_its_analyzer_domain()— 0 callers, 8 callees - …and 1 more — each is listed as a finding
Verification — 3490 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 2846 function(s) in the blast radius were not formally verified this run
Cross-repo impact
Cross-repo impact
No cross-repo impact detected among the indexed peer repo(s).
1 peer repo(s) in this group are not yet indexed — cross-repo impact may be incomplete.
Cross-repo linkage is derived (SCIP-moniker matches are sound; name/contract matches are heuristic) and ADVISORY — it never affects this check's conclusion.
Docs that may be stale (advisory)
AGENTS.md§ Known codegen quirks (read before changing) (lines 483-941): references changed symbols_type_spec_from_exprCHANGELOG.md§ Compatibility and migration (lines 868-914): references changed symbols__init__CLAUDE.md§ Known codegen quirks (read before changing) (lines 483-941): references changed symbols_type_spec_from_exprREADME.md§ How it works (lines 507-533): references changed symbols__init__
Formal verification
Could not verify: Could not verify \_\_init\_\_.
The verifier did not have enough to check \_\_init\_\_, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_check\_incompatible\_binary\_operands.
The verifier did not have enough to check \_check\_incompatible\_binary\_operands, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_numeric\_condition\_type.
The verifier did not have enough to check \_numeric\_condition\_type, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_Assignment.
The verifier did not have enough to check \_visit\_Assignment, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_BinOp.
The verifier did not have enough to check \_visit\_BinOp, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_FuncCall.
The verifier did not have enough to check \_visit\_FuncCall, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_FuncDef.
The verifier did not have enough to check \_visit\_FuncDef, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_IfStmt.
The verifier did not have enough to check \_visit\_IfStmt, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_MethodDef.
The verifier did not have enough to check \_visit\_MethodDef, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_Subscript.
The verifier did not have enough to check \_visit\_Subscript, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_SwitchStmt.
The verifier did not have enough to check \_visit\_SwitchStmt, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_Ternary.
The verifier did not have enough to check \_visit\_Ternary, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_UnaryOp.
The verifier did not have enough to check \_visit\_UnaryOp, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_VarDecl.
The verifier did not have enough to check \_visit\_VarDecl, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_WhileStmt.
The verifier did not have enough to check \_visit\_WhileStmt, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify analyze.
The verifier did not have enough to check analyze, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_handle\_user\_func\_call.
The verifier did not have enough to check \_handle\_user\_func\_call, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_assignment\_use.
The verifier did not have enough to check \_assignment\_use, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_use.
The verifier did not have enough to check \_use, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
· 7 grounded finding(s) anchored inline below; 2 more finding(s) on lines outside this diff (see the check run).
| return TypeSpec(left.kind, name=left.name or right.name, **arguments) | ||
|
|
||
|
|
||
| def resolve_operator( |
There was a problem hiding this comment.
resolve_operator()
16 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| "math.max(close, nz(open))", "math.min(close, nz(open))", | ||
| "math.sign(nz(close))", | ||
| ]) | ||
| def test_math_result_retains_number_domain_when_width_is_open(expression): |
There was a problem hiding this comment.
test_math_result_retains_number_domain_when_width_is_open()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
|
||
|
|
||
| @pytest.mark.parametrize("operator", ["==", "!=", "<", "<=", ">", ">="]) | ||
| def test_comparison_result_is_bool_even_when_operand_types_are_unresolved(operator): |
There was a problem hiding this comment.
test_comparison_result_is_bool_even_when_operand_types_are_unresolved()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| assert analyzer._operator_operand_type(result.value) == TypeSpec.primitive("bool") | ||
|
|
||
|
|
||
| def test_invalid_comparison_still_has_a_bool_result_family(): |
There was a problem hiding this comment.
test_invalid_comparison_still_has_a_bool_result_family()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
|
||
|
|
||
| @pytest.mark.parametrize("row", ANALYZER_ACCEPTED, ids=lambda row: row["probe"]) | ||
| def test_typed_missing_string_retains_its_analyzer_domain(row): |
There was a problem hiding this comment.
test_typed_missing_string_retains_its_analyzer_domain()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
|
||
|
|
||
| @pytest.mark.parametrize("float_first", [False, True]) | ||
| def test_callable_numeric_return_types_remain_per_written_call(float_first): |
There was a problem hiding this comment.
test_callable_numeric_return_types_remain_per_written_call()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| ("v = math.max(1, 2)\nr = v == 2\n", "int"), | ||
| ], | ||
| ) | ||
| def test_polymorphic_call_facts_survive_aliases_without_a_scalar_default(body, expected): |
There was a problem hiding this comment.
test_polymorphic_call_facts_survive_aliases_without_a_scalar_default()
fans out to 9 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Superseded by #308, which carries this change together with the related one on current main. |
Operator typing: refuse an operator pair TradingView refuses at the operand
One analyzer relation types every unary and binary operator, the ?: condition
and the compound assignment. A pair is refused only when both operand types
come from a certain source (a literal, a declared hint, a typed parameter, a
structured type, a variant-resolved builtin call); coarse registry results,
VOID and UNKNOWN stay unresolved and are accepted. The proofs live in a side
table that only the refusal relation reads: the symbol types, callable return
types and storage facts that code generation reads keep their previous values,
so no emitted C++ changes.
A direct comparison with na is refused from the comparison start. A history
read of an array, matrix, map or user-type field used as an operator operand
is refused, and twenty such sources are pinned with their TradingView code
family. A proven operand that fits no overload of an operator is refused
whatever the other operand is, so a collection read beside an unresolved
operand no longer reaches code generation. A comparison always has bool type.
The call-site check resolves a function's free names in its definition scope.
A while condition of type na and a switch discriminator that fits no case type
are refused as TradingView refuses them. The operand-pair refusal has its own
catalog code, pinned in the digest fixture.
main (with the positional-after-keyword change) merged in; diagnostic codes PF-E2076, PF-E2077, PF-E2078.
🤖 Generated with Claude Code