Skip to content

engine: remove legacy "0+0" LOOKUP_SENTINEL read-shim from lookup-only detection (gated on datamodel migration) #608

@bpowers

Description

@bpowers

Summary

Remove the legacy "0+0" (crate::mdl::LOOKUP_SENTINEL) read-shim that lookup-only detection still accepts, after migrating any already-serialized datamodel instances that carry it.

Issue #606 (just implemented) made a standalone lookup-only table a non-value-bearing static table and changed the MDL importer to emit the canonical EMPTY-equation form for a bare lookup, instead of the "0+0" sentinel. But two read paths still ACCEPT "0+0" as a lookup-only marker, as a back-compat shim for datamodel instances already serialized before #606 (the project has a DB with serialized protobuf instances, per the repo hard rule that protobuf/DB back-compat must be preserved):

  • crate::variable::is_empty_or_sentinel (src/simlin-engine/src/variable.rs:455-458): trimmed.is_empty() || trimmed == crate::mdl::LOOKUP_SENTINEL. This atom backs is_lookup_only / var_is_lookup_only, which classify a variable as a lookup-only static table.
  • The MDL writer (src/simlin-engine/src/mdl/writer.rs:823-830, is_lookup_only_equation) delegates to the same is_empty_or_sentinel, so it emits native Vensim name(body) syntax for both the empty and "0+0" forms.

The surrounding code already documents this as a deliberate, temporary back-compat shim (e.g. variable.rs:439-441: "New imports emit the empty form; the "0+0" arm is a back-compat read-shim for models already serialized with the sentinel"; mdl/mod.rs:43-45).

Follow-up work

  1. Migrate any serialized datamodel instances that still carry equation = "0+0" together with a graphical function for a bare lookup to the canonical empty-equation form.
  2. Then remove the == LOOKUP_SENTINEL arm from is_empty_or_sentinel (and update is_lookup_only / var_is_lookup_only / mdl::writer::is_lookup_only_equation and their doc comments + the related #[cfg(test)] assertions in variable.rs and mdl/writer_tests.rs that currently pin the "0+0" acceptance) so the sentinel is no longer a load-bearing detection input for lookup-only variables.

After the migration + removal, lookup-only detection keys solely on the canonical empty/whitespace-only equation form.

Important scope boundary (do NOT remove)

LOOKUP_SENTINEL ("0+0") is still legitimately PRODUCED for MdlEquation::EmptyRhs -- a defined-but-unspecified variable that should evaluate to 0 rather than error (src/simlin-engine/src/mdl/convert/variables.rs:681, MdlEquation::EmptyRhs(_, _) => Ok((LOOKUP_SENTINEL.to_string(), None, None)); see also mdl/mod.rs:37-48). That use is separate from the lookup-only read-shim and stays. The has_tables guard already keeps an empty-RHS aux (no graphical function) off the lookup-only path, so the two uses are distinguishable. Only the lookup-only read-shim (the == LOOKUP_SENTINEL arm in is_empty_or_sentinel, reached when has_tables is true) is what this follow-up removes.

Why it matters

Maintainability / clarity. As long as is_empty_or_sentinel treats "0+0" as a lookup-only marker, two distinct string forms are load-bearing inputs to lookup-only classification, and the sentinel constant is overloaded for two unrelated meanings (empty-RHS-evaluates-to-0 vs. legacy-lookup-only). Collapsing detection to the single canonical empty form removes that overload and the back-compat branch once no serialized instance needs it. No correctness impact today -- the shim is correct; this is debt cleanup gated on a data migration.

Component(s) affected

  • src/simlin-engine/src/variable.rs (is_empty_or_sentinel, is_lookup_only, var_is_lookup_only, and their #[cfg(test)] assertions)
  • src/simlin-engine/src/mdl/writer.rs (is_lookup_only_equation) and src/simlin-engine/src/mdl/writer_tests.rs
  • src/simlin-engine/src/mdl/mod.rs (LOOKUP_SENTINEL doc comment; constant itself stays for the EmptyRhs producer)
  • Plus whatever owns the datamodel/protobuf migration (the serialized-instance migration step is the gating prerequisite)

Possible approaches

  • Write a one-shot migration that rewrites any persisted datamodel instance whose variable has equation == "0+0" AND a graphical function attached to the canonical empty-equation form (leaving genuine empty-RHS "0+0" auxes, which have no graphical function, untouched).
  • After the migration has run against all stores, delete the == LOOKUP_SENTINEL arm from is_empty_or_sentinel, simplify is_lookup_only_equation in the writer to the empty-equation check, and update the doc comments + the "0+0"-acceptance tests accordingly.

Severity

low (tech debt / cleanup, gated on a data migration).

Discovery context

Identified as a follow-up while implementing #606 (standalone lookup-only variable lowered to gf(Time) -> made a non-value-bearing static table; importer switched from the "0+0" sentinel to the canonical empty-equation form). #606 deliberately kept the "0+0" read-shim for back-compat with already-serialized instances; this issue tracks finishing that removal once the data is migrated.

Reference: origin issue #606.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions