Skip to content

engine: model-level diagnostics leak the internal ModelError{code: details} Display form into user-facing messages #952

Description

@bpowers

Problem

format_diagnostic's DiagnosticError::Model arm renders the whole common::Error value through its Display impl, leaking a developer-facing struct-shaped form into user-facing messages.

src/simlin-engine/src/errors.rs:376-381:

message: Some(format!(
    "{} in model '{}': {}",
    severity_word(severity),
    diag.model,
    err          // <-- whole Error, via Display
)),

impl fmt::Display for Error (src/simlin-engine/src/common.rs:736-749) renders write!(f, "{}{{{}: {}}}", kind, self.code, details), i.e. ModelError{<code>: <details>}.

So cargo run -p simlin-cli -- simulate --ltm test/conveyors/minimal_conveyor.xmile prints:

warning in model 'main': ModelError{conveyor_ltm_degraded: LTM (Loops That Matter) analysis over conveyor stock 'Students' is degraded: ...}

The ModelError{ / } wrapper is pure noise. It names an internal enum variant (ErrorKind::Model) that the sentence already conveys ("in model 'main'"), and it braces otherwise human-readable prose.

Why it matters

User-facing, across every surface that formats diagnostics:

  • simlin-cli prints the string directly.
  • libsimlin carries it in SimlinErrorDetail.message, so it reaches pysimlin's check() and the TypeScript engine.

Every model-level diagnostic prints this way: the conveyor/queue LTM-degraded advisories, the conveyor spec advisories, the unit-inference umbrella, circular dependency, and duplicate variable.

Inconsistent with every sibling arm

The unit arms already render code and details separately and readably (errors.rs:239, :266, :301-312):

units warning in model 'main' variable 'bad_units': unit_mismatch -- computed units 'people' don't match specified units

The Assembly arm interpolates a plain String and is fine. The Model arm is the only one that stringifies a whole Error.

The data needed for the readable shape is already in hand: err.details is cloned into FormattedError.details a few lines below, under a comment that states the invariant outright:

// Model-level Error.details is a bare reason by construction (e.g. the unit-inference umbrella built in db/units.rs), so it rides in details for GUI consumers just like per-variable unit errors.

Evidence of fixture drift

src/diagram/tests/project-controller.test.ts:743 already asserts the target shape, with no ModelError{} wrapper:

"warning in model 'main': unit_mismatch -- unit checking failed; inconsistent constraints:\n    1 == x"

That mock has silently diverged from real engine output, so the TS side is already written against the message this issue asks for.

Suggested fix

Mirror the unit arms in the Model arm of format_diagnostic: emit "{severity} in model '{model}': {code}", appending " -- {details}" when err.details is Some. Interpolate err.code rather than err.

Then update the engine tests that assert on the message. src/diagram/tests/project-controller.test.ts likely needs no change (see fixture drift above) but should be re-checked.

Related

  • Tech-debt entry 64 (docs/tech-debt.md) — doubled ImportError{generic: ...} wrapping in CLI errors. Same root cause in Error's Display shape, but a different code path (simlin-cli's die! on the import path) and a different fix. Fixing this issue does not fix entry 64, and vice versa. A broader fix to Error::Display itself would subsume both.
  • cli: Warning-severity diagnostics are printed with an "error" prefix #919 (warning-severity diagnostics printed with an "error" prefix) — same format_diagnostic function, orthogonal defect.

Discovery

Found while fixing #919. Deliberately left out of that PR: it changes a different, broader set of message strings and is orthogonal to the severity conflation.

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