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:
-
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.
-
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.
Problem
The dependency graph carries edges for a variable whose equation the compiler refuses.
db::variable_direct_dependenciesclassifies a variable's reads once, on the typedExpr1tier (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.DimensionInScalarContextatast/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:var_minandvar_maxare scalar equations that reference the dimension namedim1inside a subscript; the bounds tier refuses both withDimensionInScalarContext. The graph nonetheless carries dt/init edgesvar_max <- var1,var_min <- var1and 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 toExpr2under 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 libsimlinsimlin_model_get_incoming_linkssurface, 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_implsrc/simlin-engine/src/variable.rs--classify_dependenciessrc/simlin-engine/src/db/dep_graph.rs--ordering_edgesand the causal-edge builder (consumers)src/simlin-engine/src/ast/expr2.rs-- the bounds-tier refusals that the typed tier does not seesrc/simlin-engine/CLAUDE.md-- the "One dependency representation" bullet and the salsa-layer consumer list, which should state the contractdocs/design-plans/2026-08-25-compiler-unification.md-- Phase 8.5 paragraph ("the dependency classification runs on the typedExpr1and needs no scope")Possible approaches
Decide the contract, document it in the engine
CLAUDE.mdbullet forvariable_direct_dependencies, and pin it with themin_max_1argmodel (a test that reads the dependency query / dependency graph forvar_min/var_maxand asserts the chosen behavior alongside theDimensionInScalarContextrefusal viaTestProject::error_diagnostics).Two defensible choices:
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.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 isErr, 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
Expr1and 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 (commit1e77b089, "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.