engine: SourceVariable.model_name is a misnamed correctness trap (it is the Module reference target, empty for non-Module vars -- not the owning model) #558

Problem

SourceVariable.model_name (salsa input field, src/simlin-engine/src/db.rs, definition at ~line 244) is misleadingly named. It does not hold the name of the model that owns/contains the variable. It holds a Module variable's referenced target model name, and is String::new() (empty) for every non-Module variable.

The population site already names the local correctly -- source_variable_from_datamodel (db.rs:2582-2591) computes referenced_model_name:

let (module_refs, referenced_model_name) = match var {
    datamodel::Variable::Module(m) => (
        m.references.iter().map(SourceModuleReference::from).collect(),
        m.model_name.clone(),                 // <- Module's *target* model
    ),
    _ => (Vec::new(), String::new()),         // <- empty for everything else
};

...but it is then stored into the salsa field named model_name, with no doc comment at the field definition. The name is the only signal a future reader gets, and it points the wrong way.

The trap is reinforced by the rest of the codebase, where model_name consistently means "referenced/target model": SourceModuleReference.model_name, datamodel::Module.model_name, bytecode.rs module descriptors, errors.rs, etc. So source_variable.model_name reads very naturally as "the model this variable belongs to" -- which is exactly what it is not.

Existing readers are only correct because they defensively guard on kind/emptiness, e.g. db_analysis.rs:1013-1016:

let model_name = source_var.model_name(db);
if !model_name.is_empty() {                   // only meaningful for Module vars
    dynamic_modules.insert(name.clone(), model_name.clone());
}

A future caller that reasonably reads source_variable.model_name(db) expecting the owning model will get the empty string for all ordinary stocks/flows/auxes and be silently wrong -- no panic, no type error, just a plausible-looking empty value.

Why it matters

• Silent correctness trap: a wrong-but-plausible field whose misuse fails quietly (empty string for non-Module vars) rather than loudly. This is the worst failure mode for a shared low-level layer.
• The salsa source layer (db.rs) is a foundational, widely-touched module; anyone extending LTM, analysis, or compilation may reach for this field.
• Concrete cost: during the GH engine: macro whose body wraps a same-canonical-name intrinsic (INIT = INITIAL(x)) causes false recursion cycle, blocking C-LEARN macro expansion #554 macro fix, code needed a variable's owning macro model. model_name looked like exactly the right field but is not, costing a debugging cycle. The workaround was to introduce a separate salsa-tracked db_macro_registry::macro_body_owner map plus an enclosing_macro_for_var accessor -- machinery that exists partly because this field could not be (and must not be) used for "owning model."

Component(s) affected

• src/simlin-engine -- SourceVariable salsa input in src/simlin-engine/src/db.rs (field def ~line 244; populated in source_variable_from_datamodel at db.rs:2582-2618).
• All readers of SourceVariable::model_name (currently all correctly kind-guarded): db.rs (multiple), db_analysis.rs:1013, db_ltm.rs:783, db_ltm.rs:1847.

Possible approaches for resolution

Either of:

1. Rename the field to something accurate, e.g. referenced_model_name (it is the Module reference target). This matches the already-correct local variable name at the population site and makes the empty-for-non-Module behavior self-documenting. Update the ~handful of .model_name(db) call sites.
2. Add a precise doc comment at the field definition (src/simlin-engine/src/db.rs ~line 244) stating that it is the Module-reference target model name, is empty (String::new()) for all non-Module variables, and is NOT the model that owns/contains this variable -- and point readers who need the owning/enclosing model at the appropriate accessor (e.g. the macro enclosing_macro_for_var / macro_body_owner path, or the model that lists the variable in its variable_names).

Option 1 (rename) is preferred -- a name that can't be misread beats a comment that can be skipped -- but either resolves the trap.

Context / how discovered

Pre-existing tech debt surfaced (not introduced) during the GH #554 macro fix (false-recursion-cycle / C-LEARN macro expansion work) when code needed a variable's owning macro model and this field looked correct but was not. Filed for tracking only; out of scope for the macro epic and intentionally not fixed as part of #554.