Skip to content

engine: lower_implicit_var drops helper-fragment parse errors silently #666

@bpowers

Description

@bpowers

Problem

In src/simlin-engine/src/db/fragment_compile.rs (lower_implicit_var, around lines 276-281), a synthesized implicit helper (SMOOTH/DELAY/TREND internal var) whose parse fails bails to None:

if parsed_implicit
    .equation_errors()
    .is_some_and(|e| !e.is_empty())
{
    return None;
}

The underlying parse errors (e.g. BadDimensionName) are never emitted as diagnostics -- they are discarded before any accumulator sees them. The only thing that surfaces is assemble_module's aggregate "failed to compile fragments for variables: ..." missing_vars string, which names the parent variable but not the specific helper or the actual parse error.

Why it matters

Diagnostic visibility. This opacity hid a C-LEARN-breaking regression from every diagnostic-based gate: commit 1736f9a broke C-LEARN entirely (the helper-fragment parse started failing) and the only signal was the opaque aggregate missing_vars string -- it passed the full pre-commit gate + adversarial review and was caught only later (fixed by commit 206f8b9). Surfacing the per-helper parse errors as structured diagnostics naming the helper and its parent variable would have made the regression legible immediately.

Relationship to #581

Same general area ("assembly-stage diagnostics not reaching collect_all_diagnostics") and conceptually adjacent, but a distinct root cause:

A fix for #581 alone would not surface these errors, because no diagnostic is emitted in the first place.

Components affected

  • src/simlin-engine/src/db/fragment_compile.rs (lower_implicit_var, the return None on equation_errors)

Possible approaches

Accumulate the per-fragment parse errors as diagnostics naming the helper and its parent (parent_source_var) variable before returning None, so they reach collect_all_diagnostics. Coordinate with the #581 IN_TRACKED_CONTEXT decision (the emission needs to happen where salsa accumulation is legal, or be relocated into a tracked frame).

Severity

Medium (diagnostic visibility; the demonstrated 1736f9a miss shows this hides real regressions from every diagnostic gate).

Discovery context

Identified during an LTM-hardening session; motivated by the 1736f9a C-LEARN regression that this opacity masked.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingengineIssues with the rust-based simulation engine

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions