Skip to content

engine: dependency graph carries edges for a variable the bounds tier refuses (min_max_1arg) #1043

Description

@bpowers

Problem

The dependency graph carries edges for a variable whose equation the compiler refuses.

db::variable_direct_dependencies classifies a variable's reads once, on the typed Expr1 tier (ast::typed_ast, variable::classify_dependencies), before the bounds tier (Expr1 -> Expr2, ast/expr2.rs) has run. The typed tier records every read; the bounds tier is where a class of equations is refused (e.g. DimensionInScalarContext at ast/expr2.rs:583). So a variable the bounds tier refuses still has a full dependency set, and the dependency graph (db/dep_graph.rs) and the causal-edge builder wire it in.

Concrete instance -- test/test-models/tests/min_max_1arg/test_min_max_1arg.xmile:

<aux name="var_min"><eqn>MIN(var1[dim1])</eqn></aux>
<aux name="var_max"><eqn>MAX(var1[dim1])</eqn></aux>
<aux name="var1"><dimensions><dim name="dim1"/></dimensions><eqn>1, 2, 3</eqn></aux>

var_min and var_max are scalar equations that reference the dimension name dim1 inside a subscript; the bounds tier refuses both with DimensionInScalarContext. The graph nonetheless carries dt/init edges var_max <- var1, var_min <- var1 and the matching causal edges (Phase 8.5 corpus dump: this is the one model whose scheduling dump changed versus the pre-8.5 base, which lowered every variable to Expr2 under an empty scope for its dependencies and so got no AST and no deps for a bounds-refused equation).

Why it matters

Harmless today: the refusal is a compile error that fails the project before anything is scheduled, and the model is refused with the same diagnostic before and after 8.5. But "a refused variable with dependencies" is a state no consumer of the graph is written to expect, and nothing in the graph's contract says which way it goes. A future consumer that reads the graph before consulting refusals -- LTM discovery (db/ltm/loops.rs, analysis.rs), layout (a module read is drawn to the module box), the libsimlin simlin_model_get_incoming_links surface, or the incremental invalidation walk -- would act on edges for an equation that never compiles. The only observable effect today is diagnostics ordering on refused models, and that is exactly the kind of quiet drift that a documented contract plus a pin prevents.

This is a design-contract question, not a live bug: low priority.

Component(s) affected

  • src/simlin-engine/src/db/query.rs -- variable_direct_dependencies, variable_direct_dependencies_impl
  • src/simlin-engine/src/variable.rs -- classify_dependencies
  • src/simlin-engine/src/db/dep_graph.rs -- ordering_edges and the causal-edge builder (consumers)
  • src/simlin-engine/src/ast/expr2.rs -- the bounds-tier refusals that the typed tier does not see
  • src/simlin-engine/CLAUDE.md -- the "One dependency representation" bullet and the salsa-layer consumer list, which should state the contract
  • docs/design-plans/2026-08-25-compiler-unification.md -- Phase 8.5 paragraph ("the dependency classification runs on the typed Expr1 and needs no scope")

Possible approaches

Decide the contract, document it in the engine CLAUDE.md bullet for variable_direct_dependencies, and pin it with the min_max_1arg model (a test that reads the dependency query / dependency graph for var_min/var_max and asserts the chosen behavior alongside the DimensionInScalarContext refusal via TestProject::error_diagnostics).

Two defensible choices:

  1. Keep the edges; state that dependencies are a property of the spelling, not of compilability. The dependency set answers "what does this equation read as written", which is well-defined even for an equation the compiler refuses, and is what patch.rs's rename and a future "show me what this broken variable references" UI want. Under this contract every consumer that needs compilable-only edges must consult the lowering memo (lowered_source_variable) or the diagnostics first, and the CLAUDE.md consumer list should say which ones do. Cheapest; matches the current code.

  2. Drop the edges for a refused variable. Make variable_direct_dependencies (or the graph builder) return the empty set when the variable's lowering memo is Err, so the graph is always the graph of what will execute. Restores the pre-8.5 shape for this class, but couples the dependency query to the lowering memo (a second typing plus the shape resolution) and reintroduces the "no AST -> no deps" behavior the 8.5 review called out as the base dropping real reads by spelling.

Whichever is chosen, the other tier of refusal (a typed-tier refusal, e.g. unknown builtin or arity) already yields no Expr1 and therefore no deps, so the contract must also say what happens there, and the pin should cover both arms (typed-tier refused vs bounds-tier refused) rather than one.

Context

Identified during PR #1040 (branch compiler-unification-v2) by the Phase 8.5 adversarial review (commit 1e77b089, "engine: one dependency representation, classified once"), as an out-of-scope discovery: "min_max_1arg-class dependencies exist on the tree for equations the bounds tier refuses; harmless, but the dependency graph now carries edges for variables that have no AST." The review confirmed the model is refused on both binaries with the same diagnostic, so the change is inert for behavior; this issue is about naming the contract.

Activity

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

    engineIssues with the rust-based simulation enginetech debt

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions