ltm: a NaN literal in a scored equation defeats salsa backdating for typed LtmEquation (avoidable fragment recompiles)

[bpowers]

Problem

LtmSyntheticVar::equation is a typed LtmEquation whose arms carry a parsed Expr0 AST (src/simlin-engine/src/db/ltm/equation.rs). Expr0::Const(String, f64, Loc) (src/simlin-engine/src/ast/expr0.rs:76) holds an f64, and LtmArm / LtmEquation get derived PartialEq and salsa::Update:

#[derive(Clone, PartialEq, salsa::Update)]
pub struct LtmArm { /* text: String, expr: Option<Expr0>, ... */ }

nan is a lexer keyword (("nan", Nan) in the KEYWORDS table, src/simlin-engine/src/lexer/mod.rs:86), so a generated LTM equation can legitimately embed f64::NAN. Since NaN != NaN, two arms built from identical text compare unequal.

Consequence: salsa cannot backdate the affected query result. link_score_equation_text_shaped re-runs and reports a changed value on every unrelated edit, so the expensive dependent compile_ltm_var_fragment (src/simlin-engine/src/db/ltm/compile.rs) re-executes even though the equation is semantically and textually identical. This is a performance/incrementality regression, not a wrong answer.

It also falsifies a claim in commit dea42188 ("engine: store LTM synthetic equations as typed ASTs"), whose message states: "Salsa incrementality is preserved: the shaped text query backdates on the PartialEq-equal typed equation, so unrelated edits reuse the expensive fragment (pinned by the existing per-link caching test)." That is true for ordinary equations and false for NaN-bearing ones -- worth noting so the next reader does not trust that sentence unconditionally.

Measured (verified by running code, not by inspection)

A throwaway probe constructing values through the public API (since removed, nothing committed):

• LtmArm::new("NaN") parses successfully (expr.is_some() == true).
• Two LtmArm::new("NaN") compare unequal (a == b is false).
• Two LtmEquation::scalar("1 + NaN") compare unequal.
• Control: two LtmEquation::scalar("1 + 2") compare equal.
• Critically, <LtmEquation as salsa::Update>::maybe_update(...) returns true (CHANGED) for identical "1 + NaN" text -- so the actual backdating mechanism is affected, not merely PartialEq.

That last point matters for whoever fixes this: implementing only a manual PartialEq is insufficient, because salsa's derived Update compares fields independently.

Reachability

Narrow but legal. It needs a NaN literal in a user equation that feeds a scored LTM link. Note that Vensim's :NA: does not produce NaN in this engine -- crate::float::NA is the finite sentinel -2^109 (src/simlin-engine/src/float.rs:24, see also #586) -- so MDL imports are unaffected. A hand-written XMILE equation using NaN is the realistic path.

Given #977 (LTM link-score generation is already quadratic in an arrayed target's element count), losing fragment reuse on top of that is a real if narrow cost.

Components affected

• src/simlin-engine/src/db/ltm/equation.rs (LtmArm, LtmEquation derived PartialEq / salsa::Update)
• src/simlin-engine/src/db/ltm/compile.rs (link_score_equation_text_shaped, compile_ltm_var_fragment)
• src/simlin-engine/src/ast/expr0.rs (Expr0::Const float equality)

Possible approaches

1. Compare on text alone for LtmArm. expr is a pure function of text (expr = Expr0::new(text), built together at construction and documented as never drifting -- see the LtmArm type docs), so text is the canonical identity and this is NaN-stable. But it requires hand-writing BOTH PartialEq and salsa::Update, and salsa::Update is an unsafe trait whose maybe_update contract, if got wrong, makes salsa MISS real changes -- stale results, a correctness bug strictly worse than the performance issue being fixed. So it deserves its own careful change with tests, not a drive-by.

2. Compare the f64 by bit pattern (to_bits) inside Expr0's equality. This fixes the problem generally for every Expr0 consumer rather than only LTM, but it changes equality semantics for +0.0/-0.0 too, so it needs its own analysis.

Relationship to #642

#642 covers the same root class (derived PartialEq over f64 defeating salsa backdating) for the compiled bytecode types (ByteCode.literals, ByteCodeContext.graphical_functions, vm::Specs). #642 argues the defect is benign there because the only consumer (compile_project_incremental) is non-tracked. That mitigation does not apply here: the consumer of the typed LtmEquation is a tracked query, so the lost backdating has an actually observable cost. The fix in approach 2 above would address both.

Severity

Low -- performance/incrementality only, narrow reachability. No wrong answers.

Context

Identified while reviewing the typed-equation storage introduced by #965 / commit dea42188. Cross-references: #642, #965, #977, #586.

[bpowers]

Cross-reference: #987 records the same f64-in-a-derived-PartialEq root for ModelStage0 / ModelStage1 / db::query::ParsedVariableResult, found while implementing #966.

Filed separately rather than folded in here because the reachability argument is different in kind: this issue's analysis concludes a NaN needs a hand-written NaN in a user equation feeding a scored LTM link, but every stdlib SMOOTH/DELAY/TREND template declares initial_value = NAN (stdlib/smth1.stmx:29 and four siblings), and db::sync splices those into every project -- so the core-pipeline instances are non-reflexive unconditionally, with no user action.

The fix is plausibly shared: approach 2 in this issue (bit-pattern f64 comparison inside Expr0/Expr2 equality) would resolve #981, #987, and #642 together.