Skip to content

engine: per-source fan-out of unit inference diagnostics (enables per-field underlines deferred from #836) #837

Description

@bpowers

Problem

Unit inference conflicts are surfaced only as a single model-level umbrella diagnostic with variable: None, so the diagram cannot attribute an inference error to any variable, let alone underline the offending span. While fixing #836 we chose "Option A": inference errors get no per-field underline at all, because with only the umbrella diagnostic there is nothing to attribute. Two engine-side follow-ups were identified and deliberately deferred; they are the same work area, so they are tracked together here.

1. Per-source fan-out of unit inference diagnostics

UnitError::InferenceError.sources (src/simlin-engine/src/common.rs:2449) already carries a Vec<(variable_name, Option<Loc>)> -- one loc per involved variable's equation. But the salsa pipeline (src/simlin-engine/src/db/units.rs, the has_declared_units && !inference.conflicts.is_empty() block around line 279) collapses all conflicts into one model-level diagnostic with variable: None. No per-variable diagnostic is ever emitted, so the diagram has nothing to place a per-field underline on.

Emitting one properly-located per-source diagnostic per involved variable would enable correct per-field underlines (the deferred "Option D" of #836).

Constraint: the umbrella exists deliberately -- macro-heavy models can produce hundreds of internal constraint contradictions, and one-diagnostic-per-conflict would flood the report (see the comment in that block). A per-source fan-out needs dedup/capping thought: e.g. dedupe by variable, cap the count, or emit per-variable diagnostics alongside (not instead of) the umbrella. The full conflict list already remains available on InferenceResult for callers that want it.

2. format_unit_error picks the wrong source's Loc

The InferenceError arm of format_unit_error (src/simlin-engine/src/errors.rs:216) sets start/end offsets from sources.first()'s Loc:

let (start, end) = sources
    .first()
    .and_then(|(_, loc)| *loc)
    .map(|loc| (loc.start, loc.end))
    .unwrap_or((0, 0));

But the first source may be a different variable than the var_name the error is being attributed to, so the offsets can point into another variable's equation. If per-variable inference diagnostics are emitted (item 1), the formatter should select the loc of the source matching var_name instead of first().

Why it matters

Diagnostic correctness and user experience: today the per-variable inference path is latent (nothing emits per-variable inference diagnostics), so this is low urgency. But it blocks ever giving inference errors a correct per-field underline in the diagram, and item 2 is a landmine that would produce wrong-equation offsets the moment item 1 lands.

Components affected

  • src/simlin-engine/src/db/units.rs (umbrella diagnostic emission)
  • src/simlin-engine/src/errors.rs (format_unit_error InferenceError arm)
  • src/simlin-engine/src/common.rs (UnitError::InferenceError.sources -- already carries the needed data)
  • Downstream: src/diagram per-field underline placement (consumer, once per-variable diagnostics exist)

Context

Identified during the fix for #836 (July 2026), where the diagram-side placement bug was resolved by giving inference errors no per-field underline ("Option A"). Related: #614 (partial inference results, closed -- this builds on that work), #613 (residual C-LEARN unit-diagnostic strictness).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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