engine: needs_quoting ignores equation-language keywords, so a variable named `if`/`or`/`mod` prints bare and fails to re-parse

[bpowers]

Problem

ast::needs_quoting (src/simlin-engine/src/ast/mod.rs:471) is the single "can this canonical name be spelled bare in an equation" predicate, but it only tests the XID_Start/XID_Continue character classes (plus _, plus the leading-character rule added for 1stock). It does not check whether the name collides with an equation-language KEYWORD.

The lexer resolves a word against its keyword table FIRST (src/simlin-engine/src/lexer/mod.rs:157-162, identifierish): the word is lowercased and matched against KEYWORDS (if, then, else, not, mod, and, or, nan — lexer/mod.rs:78-87), falling back to Ident(word) only when there is no match. So a bare keyword can never lex as an identifier.

XMILE permits quoting arbitrary names, so "if" is a spellable variable name and canonicalization preserves it as if. print_ident gates on needs_quoting, so every printed reference to that variable comes out bare — and that text does not re-parse as a reference to the variable.

ltm_augment::quote_ident (src/simlin-engine/src/ltm_augment.rs:3023) shares the hole: it requires both "all chars alphanumeric or _" and !needs_quoting(..), and a keyword satisfies both.

Verified reachability (probe against HEAD of roundtrips-track-a, commit 17d4e7c)

A keyword-named variable is admitted end to end — it is not rejected earlier by a reader or by compilation. Model: if = 3, b = "if" * 2, c = b + 1.

• Project::from compiles it with zero errors or diagnostics.

• patch.rs rename corrupts the saved model (loud, but only on recompile). RenameVariable { from: "b", to: "bee" } reprints every dependent equation through expr2_to_string -> print_ident. Result:

  if  = Scalar("3")
  bee = Scalar("if * 2")     <- quotes dropped
  c   = Scalar("bee + 1")
  

  Recompiling the patched datamodel gives EquationError { start: 3, end: 4, code: UnrecognizedToken } on bee. The corruption is written into the persisted datamodel::Project, so an unrelated rename permanently breaks a previously-valid model. Renaming to a keyword (b -> or) produces the same bare-keyword text.

• LTM degrades two link scores. A loop s -> if -> inflow -> s with ltm_enabled produces two Warning diagnostics and no usable score:

  • $⁚ltm⁚link_score⁚s→if — fragment failed to compile, keeps its slot, evaluates to a constant 0.0 for every step (the model_ltm_fragment_diagnostics warning).
  • $⁚ltm⁚link_score⁚if→inflow — skipped by the ltm-augment: parse fallback can silently break ceteris-paribus logic #311 guard: "the ceteris-paribus partial requires parsing the target's equation, but 'if * 2' did not parse."

  No loop_score variable is emitted for the loop at all. So on this surface the failure is warned (thanks to the ltm-augment: parse fallback can silently break ceteris-paribus logic #311 / fragment-diagnostic machinery) but the analysis is silently useless unless the caller reads warnings.

• MDL export is not affected for references: project_to_mdl_with_warnings emits b = "if" * 2 (the MDL writer has its own quoting rules). It does emit the definition LHS as bare if = 3, which is a separate MDL-writer question and not this issue.

Not verified: whether the XMILE or MDL readers would produce a keyword-named variable from a real file (the probe built the datamodel directly). Since canonicalization preserves quoted names and the compiler accepts the result, a reader that admits <aux name="if"> would reach the same state; that path was not exercised.

Why it matters

• Correctness: a rename silently rewrites a valid model into an unparseable one, and the damage is persisted.
• Single source of truth: the printer and the lexer disagree about what "bare-spellable" means. That is the same class of printer/parser asymmetry as engine: three more print_eqn/parser asymmetries (unary !, !=, and right-nested ^ re-parsing to a different AST) #913 (unary !, !=, right-nested ^) and exactly what ltm: replace string-backed synthetic equations with typed IR #965 cites as the hazard motivating the removal of LTM's print->reparse round trips.

Components affected

• src/simlin-engine/src/ast/mod.rs (needs_quoting, print_ident)
• src/simlin-engine/src/lexer/mod.rs (KEYWORDS, identifierish)
• src/simlin-engine/src/patch.rs (expr2_to_string, rename path)
• src/simlin-engine/src/ltm_augment.rs (quote_ident)

Suggested fix direction

Add a keyword check to needs_quoting, tested against the lexer's own KEYWORDS table (not a duplicated list), so the one "can this name be spelled bare" predicate is complete and the printer and lexer cannot disagree. That keeps the single source of truth that commit 17d4e7c established for the leading-digit case (1stock: a canonical name whose first character is not XID_Start was printed bare and failed to re-parse; fixed by making ltm_augment::quote_ident delegate to needs_quoting).

Note identifierish lowercases before matching, so the predicate should compare case-insensitively (canonical names are already lowercased, but the check should not depend on that).

A proptest asserting parse(print_eqn(e)) == e over identifiers drawn from a name pool that includes keywords would be the natural guard — the same guard #913 suggested for the operator cases.

Discovery context

Found by code reading while working on the LTM print->reparse round-trip removal (branch roundtrips-track-a), immediately after 17d4e7c consolidated the leading-digit case onto needs_quoting. Verified with a throwaway integration-test probe (not committed). Related: #965 (tracking issue for removing LTM's print->reparse round trips), #913 (historical instance of the same asymmetry class).