engine: clear C-LEARN unit-error flood (481 -> 14)

[bpowers]

Problem

Loading the C-LEARN v77 model in the Simlin UI produced an orange error box that overflowed off the screen with 481 unit diagnostics. Vensim reports essentially none (only ~21 "No units specified for X" warnings, no mismatches). Every Simlin unit mismatch on C-LEARN was a false positive.

Investigation

A diagnostic dumper (tests/clearn_unit_errors.rs, #[ignore]) bucketed the 481 by message template, revealing four independent root causes:

Count	Family	Root cause
~100	yr vs year mismatches	unit-alias handling (F1+F2)
374	"can't find or no units for dependency"	hard-error on unknown deps (F3)
~8	macro-body inference errors	checking template models (F4)

Fixes (each TDD: RED → GREEN + regression guard)

1. Context::new self-alias bug (units.rs). C-LEARN's footer declares 22:Yr,year,years,yr,Year,Years. The primary Yr canonicalizes to yr, which also appears in its own alias list — so yr became an alias of itself, took the duplicate-conflict branch, and the prime unit was never registered. lookup() then returned None and year/yr resolved to distinct unit maps. Now only an alias of a different unit is a conflict; a self-alias still registers the prime unit.

2. Inference ignored the time-unit alias (units_infer.rs). Inference built the Time variable and stock/flow constraints from the raw sim_specs.time_units string, while units_check resolved through the Context alias map — so an aliased time name (year) clashed with a declared yr flow. Inference now resolves the time unit via ctx.lookup too.

3. Hard error on unknown dependency units (units_check.rs). A units-declared variable referencing a dependency whose units were unknown (a module output or synthesized helper that inference left unresolved) raised can't find or no units for dependency. Unknown units are not a dimensional mismatch — the arrayed-element path already skipped them, and Vensim only warns on the dependency itself. The scalar / apply-to-all / arrayed-default paths now skip too, via a shared check_against_expected helper.

4. Unit-checked macro-marked models (db_units.rs::check_model_units). Macros are generic templates whose formal parameters are unitless; checking them in isolation only produces noise. They are now skipped exactly as stdlib models are.

Result

481 → 14 unit diagnostics (97% reduction). The remaining 14 are genuine-looking dimensional subtleties Vensim happens to tolerate (permafrost-methane coefficient cancellation, a ph dmnl vs 1/ppm term, an IF-branch unit difference) — tracked in #613. The all-or-nothing inference discard those ride on is tracked in #614.

Refactor

The macro-skip pushed db.rs over the 6000-line lint cap, so the unit-checking orchestration (check_model_units + its value-equivalence helper) moves to a new db_units.rs sibling, mirroring db_macro_registry / db_dep_graph. No logic change in the move.

Testing

• New: units::self_aliased_prime_unit_is_registered, tests/unit_alias_module_inference.rs (F2/F3 + a genuine-mismatch-still-caught guard), macro_expansion_tests::macro_body_units_are_not_checked (F4), tests/clearn_unit_errors.rs::clearn_unit_error_flood_is_cleared (regression assertion of all four invariants + a coarse total bound).
• World3 unit tests (the sibling large-model suite) stay green.
• Full pre-commit suite passes (Rust fmt/clippy/test, TypeScript, WASM, Python).

[claude]

Review: engine: clear C-LEARN unit-error flood (481 → 14)

I reviewed all four fixes plus the db.rs → db_units.rs refactor against the source. No blocking issues found.

What I verified:

• F1 (units.rs self-alias): matches!(aliases.get(&unit_name), Some(target) if target != &unit_name) correctly narrows the conflict branch to cross-unit aliases only. A self-alias (Some(self)) and a fresh name (None) both now fall to the else and register the prime unit. A genuine cross-unit collision (one unit's primary name already an alias of a different unit) still pushes DuplicateUnit. Builtin units that would double-declare year/yr are filtered out in new_with_builtins, so no double-registration. Same logic mirrored correctly in step 2.

• F2 (units_infer.rs time-unit alias): Resolving time_units through ctx.lookup before building the stock/flow expected and the synthetic time var aligns inference with units_check. combine(UnitOp::Div, {@stock:1}, time_units) is equivalent to the prior hand-built map for the single-component case and handles the resolved map generally; the default "time" path falls back to {time:1} unchanged.

• F3 (units_check.rs check_against_expected): The only behavioral delta vs. the old per-arm code is that Err(ConsistencyError(DoesNotExist, …)) now returns None (skip) instead of being pushed. Genuine internal mismatches still surface as Err(UnitMismatch) via the Err(err) => Some(err) arm, and the Ok(_) arm correctly absorbs Constant/matching-Explicit. This makes the scalar/A2A/arrayed-default paths consistent with the pre-existing arrayed-element path. The old code never reported a UnitMismatch for an expression that short-circuited to DoesNotExist, so no mismatch signal is newly lost.

• F4 (db_units.rs macro skip): if model.macro_spec(db).is_some() { return; } only skips macro-marked targets; macro models remain in all_s0/models_s1 for cross-module resolution, so their handling as submodels is unchanged. Narrowly scoped to unit checking (other diagnostic passes are unaffected).

• Refactor: All db.rs items imported by db_units.rs (project_units_context, project_datamodel_dims, parse_source_variable_with_module_context, model_module_ident_context, the Diagnostic* types) are pub; test_common is pub mod; new TestProject helpers and the unit_with_aliases helper exist. The C-LEARN integration tests are #[ignore]d so they don't affect CI time budgets.

Overall correctness verdict: correct. The change is well-isolated, each fix is backed by a TDD red→green test plus a regression guard, and the diagnostic-suppression choices (F3/F4) are intentional and consistent with existing patterns, with residuals tracked in #613/#614.

[codecov]

Codecov Report

❌ Patch coverage is 48.59813% with 220 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.67%. Comparing base (a8cd392) to head (8b25a4a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/simlin-engine/tests/clearn_unit_errors.rs	0.62%	159 Missing ⚠️
src/simlin-engine/src/db_units.rs	78.37%	32 Missing ⚠️
...simlin-engine/tests/unit_alias_module_inference.rs	68.75%	25 Missing ⚠️
src/simlin-engine/src/units.rs	50.00%	2 Missing ⚠️
src/simlin-engine/src/units_check.rs	88.23%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #615      +/-   ##
==========================================
- Coverage   82.86%   82.67%   -0.20%     
==========================================
  Files         261      264       +3     
  Lines       69836    70053     +217     
==========================================
+ Hits        57871    57915      +44     
- Misses      11965    12138     +173     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.