engine: migrate assert_compiles to incremental path and delete

docs/design-plans/2026-03-07-assert-compiles-migration.md

@@ -0,0 +1,115 @@
+# Assert-Compiles Migration Design
+
+## Summary
+
+Simlin's simulation engine has two compilation paths: a legacy monolithic path (`compile()`) and the current incremental path (`compile_incremental()`) built on salsa for fine-grained caching. Twenty-six tests still exercise the monolithic path through the `assert_compiles()` helper, keeping the old code alive. This plan eliminates that technical debt by migrating every test to `assert_compiles_incremental()` and then deleting the dead monolithic helpers entirely.
+
+Twenty-three of the 26 tests already pass on the incremental path and need only a mechanical one-line change. The remaining three expose two feature gaps in the incremental compiler: (1) the `@N` position syntax (e.g., `arr[@1]`) fails when the left-hand side is scalar because the subscript lowering pipeline does not resolve positional indices to concrete element names outside of an apply-to-all context, and (2) the `MEAN` builtin rejects dynamic-range subscripts that other array-reduce builtins like `SUM` and `STDDEV` already handle, due to an overly narrow pattern match in the codegen layer. The plan fixes both gaps with targeted changes -- resolving `@N` to named elements early in the AST lowering chain, and extracting a shared `emit_array_reduce` helper that all six array-reduce builtins use -- then switches the remaining tests and removes the old code.
+
+## Definition of Done
+
+All 26 tests currently using `assert_compiles()` are switched to `assert_compiles_incremental()` and pass. This requires: (1) switching the 23 already-working tests, (2) implementing incremental support for `@N` position syntax and `MEAN` with dynamic ranges so the remaining 3 tests pass, and (3) deleting `assert_compiles()` and any code that becomes dead after its removal (but keeping the AST interpreter and its supporting infrastructure).
+
+## Acceptance Criteria
+
+### assert-compiles-migration.AC1: @N position syntax works on incremental path
+- **assert-compiles-migration.AC1.1 Success:** `arr[@1]` in scalar context compiles and selects the first dimension element
+- **assert-compiles-migration.AC1.2 Success:** `cube[@1, *, @3]` with mixed position/wildcard compiles on incremental path
+- **assert-compiles-migration.AC1.3 Failure:** `arr[@0]` produces a compile error (1-based, 0 is invalid)
+- **assert-compiles-migration.AC1.4 Failure:** `arr[@N]` where N exceeds dimension size produces a compile error
+
+### assert-compiles-migration.AC2: MEAN with dynamic ranges works on incremental path
+- **assert-compiles-migration.AC2.1 Success:** `MEAN(data[start_idx:end_idx])` with variable bounds compiles and simulates correctly
+- **assert-compiles-migration.AC2.2 Success:** Existing SUM, SIZE, STDDEV, VMIN, VMAX behavior unchanged after refactoring to shared helper
+
+### assert-compiles-migration.AC3: assert_compiles fully removed
+- **assert-compiles-migration.AC3.1 Success:** All 26 formerly-monolithic tests pass with `assert_compiles_incremental()`
+- **assert-compiles-migration.AC3.2 Success:** `assert_compiles()` method deleted from `test_common.rs`
+- **assert-compiles-migration.AC3.3 Success:** `compile()` method deleted from `test_common.rs` (if no other callers)
+- **assert-compiles-migration.AC3.4 Success:** `cargo test -p simlin-engine` passes with zero regressions
+
+## Glossary
+
+- **Incremental compilation / salsa**: The engine's current compilation strategy, where each variable is compiled independently and results are cached using salsa (a Rust framework for demand-driven incremental computation). Only variables whose inputs change are recompiled.
+- **Monolithic compilation**: The older compilation path that compiles an entire project in one pass without caching. It is the code being removed by this migration.
+- **AST lowering chain (Expr0 -> Expr1 -> Expr2 -> Expr3 -> Expr)**: The sequence of intermediate expression representations that an equation passes through during compilation. Each stage resolves more syntactic sugar and subscript information until the final `Expr` form is ready for code generation.
+- **@N position syntax / DimPosition**: A subscript notation (e.g., `@1`, `@3`) that refers to the Nth element of a dimension by numeric position rather than by name. `DimPosition` is the corresponding AST node.
+- **A2A (apply-to-all)**: A system dynamics concept where a single equation defines all elements of an arrayed variable. In the compiler, A2A context is tracked via `active_dimension`.
+- **Scalar context**: The opposite of A2A context -- when the left-hand side of an equation is a plain (non-arrayed) variable, so there is no active dimension to iterate over.
+- **Array-reduce builtins**: Built-in functions (SUM, SIZE, STDDEV, VMIN, VMAX, MEAN) that collapse an array argument into a single scalar value.
+- **`walk_expr_as_view` / `PopView`**: Internal codegen methods that set up and tear down a "view" -- a runtime slice of an array -- so that an array opcode can operate over the selected elements.
+- **`ViewRangeDynamic`**: A VM opcode that constructs an array view whose bounds are determined at runtime (from variable values) rather than at compile time.
+- **`lower_index_expr3`**: The function in `context.rs` responsible for resolving subscript expressions during the Expr3 lowering stage, including dimension positions and dynamic subscripts.
+- **`codegen.rs`**: The compiler module that translates the final AST (`Expr`) into stack-based bytecode instructions for the VM.
+
+## Architecture
+
+The migration has three independent workstreams that converge on deleting `assert_compiles()`:
+
+1. **Trivial test switch (23 tests):** `builtins_visitor.rs` (20), `db_tests.rs` (2), `db_prev_init_tests.rs` (1) -- change `assert_compiles()` to `assert_compiles_incremental()` with no compiler changes.
+
+2. **@N position syntax in scalar context:** When `arr[@1]` appears with a scalar LHS (no active A2A dimension), `lower_index_expr3` in `context.rs` currently returns `ArrayReferenceNeedsExplicitSubscripts`. The fix resolves `DimPosition(n)` to a concrete element name early in the lowering pipeline: look up the referenced dimension's element list and replace `@N` with `elements[n-1]`. No VM changes or new opcodes needed. A2A context (`matrix[@2, @1]`) already works via the existing dynamic subscript path.
+
+3. **MEAN with dynamic ranges + array-reduce abstraction:** The `BuiltinFn::Mean` handler in `codegen.rs` gates the array path on `matches!(arg, Expr::StaticSubscript | Expr::TempArray)`, missing `Expr::Subscript` with `Range` indices. SUM, SIZE, and STDDEV work because they call `walk_expr_as_view` unconditionally. The fix introduces a shared `emit_array_reduce` helper and a robust `is_array_expr` check, eliminating the 6x copy-paste of `walk_expr_as_view + ArrayOp + PopView` across SUM, SIZE, STDDEV, VMIN, VMAX, and MEAN.
+
+4. **Cleanup:** Delete `assert_compiles()` and its sole dependency `compile()` from `test_common.rs`. Verify no other callers exist before removing.
+
+## Existing Patterns
+
+The incremental compilation pipeline (`compile_var_fragment` in `db.rs`) compiles one variable at a time through the AST lowering chain: `Expr0 -> Expr1 -> Expr2 -> Expr3 -> Expr`. Subscript resolution happens in `context.rs` via `normalize_subscripts3` (static path producing `IndexOp`) and `lower_index_expr3` (dynamic fallback). The `@N` fix follows the existing pattern of resolving indices to concrete element names during lowering.
+
+The array-reduce builtins (SUM, SIZE, STDDEV, VMIN, VMAX, MEAN) all share the same terminal pattern: `walk_expr_as_view(arg)` then emit an `Array*` opcode then `PopView`. This pattern is currently duplicated 6 times with no shared helper. The refactoring extracts this into a single method, which is a new pattern but one that follows the codebase's general preference for reducing duplication.
+
+The `ViewRangeDynamic` opcode already exists in the VM and handles dynamic range bounds. The MEAN fix leverages this existing infrastructure.
+
+## Implementation Phases
+
+<!-- START_PHASE_1 -->
+### Phase 1: @N Position Syntax in Scalar Context
+
+**Goal:** Make `arr[@N]` work on the incremental compilation path when the LHS is a scalar variable.
+
+**Components:**
+- `src/simlin-engine/src/compiler/context.rs` -- modify `lower_index_expr3` to resolve `IndexExpr3::DimPosition(pos)` to `IndexExpr3::Named(elements[pos-1])` when `active_dimension.is_none()`, with a compile error for out-of-range positions
+- `src/simlin-engine/src/array_tests.rs` -- switch `dimension_position_single` (line 342) and `dimension_position_and_wildcard` (line 1366) from `assert_compiles()` to `assert_compiles_incremental()`
+
+**Dependencies:** None
+
+**Done when:** `dimension_position_single` and `dimension_position_and_wildcard` pass with `assert_compiles_incremental()`. Covers `assert-compiles-migration.AC1.*`.
+<!-- END_PHASE_1 -->
+
+<!-- START_PHASE_2 -->
+### Phase 2: Array-Reduce Abstraction and MEAN Dynamic Ranges
+
+**Goal:** Unify the array-reduce codegen pattern across all builtins and fix MEAN with dynamic range arguments.
+
+**Components:**
+- `src/simlin-engine/src/compiler/codegen.rs` -- extract `emit_array_reduce(arg, opcode)` helper; refactor SUM, SIZE, STDDEV, VMIN, VMAX, MEAN handlers to use it; fix MEAN's array detection to include `Expr::Subscript` with `Range` indices
+- `src/simlin-engine/src/array_tests.rs` -- switch `mean_with_dynamic_range` (line 2023) from `assert_compiles()` to `assert_compiles_incremental()`
+
+**Dependencies:** None (independent of Phase 1)
+
+**Done when:** `mean_with_dynamic_range` passes with `assert_compiles_incremental()`. All existing array-reduce tests continue to pass. Covers `assert-compiles-migration.AC2.*`.
+<!-- END_PHASE_2 -->
+
+<!-- START_PHASE_3 -->
+### Phase 3: Bulk Test Migration and Cleanup
+
+**Goal:** Switch all remaining 23 tests and delete `assert_compiles()`.
+
+**Components:**
+- `src/simlin-engine/src/builtins_visitor.rs` -- switch 20 `assert_compiles()` calls to `assert_compiles_incremental()`
+- `src/simlin-engine/src/db_tests.rs` -- switch 2 `assert_compiles()` calls to `assert_compiles_incremental()`
+- `src/simlin-engine/src/db_prev_init_tests.rs` -- switch 1 `assert_compiles()` call to `assert_compiles_incremental()`
+- `src/simlin-engine/src/test_common.rs` -- delete `assert_compiles()` method and `compile()` method (if no other callers remain)
+
+**Dependencies:** Phases 1 and 2 (all tests must pass on incremental path first)
+
+**Done when:** Zero calls to `assert_compiles()` remain in the codebase. `assert_compiles` and `compile()` are deleted from `test_common.rs`. All engine tests pass. Covers `assert-compiles-migration.AC3.*`.
+<!-- END_PHASE_3 -->
+
+## Additional Considerations
+
+**Out-of-range @N:** If a user writes `arr[@5]` on a 3-element dimension, the compiler should emit a clear error at compile time rather than silently producing wrong results. This is a new error path that needs a test.
+
+**Other builtins with dynamic ranges:** The array-reduce abstraction in Phase 2 should be verified against all builtins that accept array arguments, not just MEAN. If any other builtin has the same narrow `is_array` check, fix it in the same pass.

docs/implementation-plans/2026-03-08-assert-compiles-migration/phase_01.md

@@ -0,0 +1,125 @@
+# Assert-Compiles Migration -- Phase 1: @N Position Syntax in Scalar Context
+
+**Goal:** Make `arr[@N]` work on the incremental compilation path when the LHS is a scalar variable, and when mixed with wildcards.
+
+**Architecture:** Modify `lower_index_expr3` in `context.rs` to resolve `DimPosition(pos)` to a concrete 1-based offset when in scalar context (no active A2A dimension), rather than returning `ArrayReferenceNeedsExplicitSubscripts`. For mixed contexts (DimPosition + wildcard, like `cube[@1, *, @3]`), the same resolution applies when the active subscript at position `pos` doesn't match the target dimension.
+
+**Tech Stack:** Rust (simlin-engine)
+
+**Scope:** Phase 1 of 3 from design plan
+
+**Codebase verified:** 2026-03-08
+
+**Reference files for executor:**
+- `/home/bpowers/src/simlin/src/simlin-engine/CLAUDE.md` -- engine architecture and module map
+- `/home/bpowers/src/simlin/CLAUDE.md` -- project-wide development standards
+
+---
+
+## Acceptance Criteria Coverage
+
+This phase implements and tests:
+
+### assert-compiles-migration.AC1: @N position syntax works on incremental path
+- **assert-compiles-migration.AC1.1 Success:** `arr[@1]` in scalar context compiles and selects the first dimension element
+- **assert-compiles-migration.AC1.2 Success:** `cube[@1, *, @3]` with mixed position/wildcard compiles on incremental path
+- **assert-compiles-migration.AC1.3 Failure:** `arr[@0]` produces a compile error (1-based, 0 is invalid)
+- **assert-compiles-migration.AC1.4 Failure:** `arr[@N]` where N exceeds dimension size produces a compile error
+
+---
+
+## Codebase Verification Findings
+
+- **Confirmed:** `lower_index_expr3` at `src/simlin-engine/src/compiler/context.rs:2062-2070` handles `IndexExpr3::DimPosition(pos, dim_loc)` starting at line 2131. When `self.active_dimension.is_none()`, it returns `sim_err!(ArrayReferenceNeedsExplicitSubscripts, ...)` at line 2134. This is the fix site.
+- **Discrepancy:** Design says "resolve to `IndexExpr3::Named(elements[pos-1])`" but `IndexExpr3::Named` does not exist. The correct approach: resolve to `SubscriptIndex::Single(Expr::Const((pos) as f64, loc))` (1-based offset), consistent with how the A2A path resolves at line 2148.
+- **Confirmed:** `dimension_position_single` test at `array_tests.rs:334` (inside `mod dimension_position_tests` starting at line 330). `dimension_position_and_wildcard` test at `array_tests.rs:1357`.
+- **Confirmed:** `Dimension::Named(_, named_dim)` has `named_dim.elements: Vec<CanonicalElementName>` and `Dimension::Indexed(_, size)` has `size: u32`. `Dimension::len()` exists at `dimensions.rs:108` and returns the element count for both variants.
+- **Confirmed:** For out-of-range errors, existing code uses `ErrorCode::MismatchedDimensions` (consistent with A2A path at line 2142). No dedicated `ArrayIndexOutOfBounds` variant.
+- **Gap:** No `assert_compile_error_incremental()` method exists on `TestProject`. For error tests (AC1.3, AC1.4), call `compile_incremental()` and verify the error message contains the expected context (e.g., the variable name) to avoid false positives from unrelated compilation errors.
+
+---
+
+<!-- START_SUBCOMPONENT_A (tasks 1-3) -->
+
+<!-- START_TASK_1 -->
+### Task 1: Modify `lower_index_expr3` to resolve scalar @N
+
+**Verifies:** assert-compiles-migration.AC1.1, assert-compiles-migration.AC1.2
+
+**Files:**
+- Modify: `src/simlin-engine/src/compiler/context.rs:2131-2161` (the `DimPosition` match arm in `lower_index_expr3`)
+
+**Implementation:**
+
+Replace the `IndexExpr3::DimPosition(pos, dim_loc)` match arm (lines 2131-2161) with logic that handles three cases:
+
+1. **Scalar context** (`self.active_dimension.is_none()`): Resolve `@N` to a concrete 1-based element offset in `dims[i]`. Check bounds: `pos == 0` or `pos > dim_size` produces `MismatchedDimensions`. Otherwise return `SubscriptIndex::Single(Expr::Const(*pos as f64, *dim_loc))`.
+
+2. **A2A context, within active subscript range, subscript matches dimension** (existing behavior): `active_subscripts[pos-1]` resolved via `dim.get_offset(subscript)` returns the concrete offset. This is the existing dimension-reordering path (e.g., `matrix[@2, @1]`).
+
+3. **A2A context, but subscript doesn't match dimension or pos out of range** (new fallback for mixed cases like `cube[@1, *, @3]`): Fall back to the same concrete element offset as the scalar case.
+
+Bounds check (applies to scalar context and A2A fallback):
+```rust
+let pos_val = *pos as usize;
+if pos_val == 0 || pos_val > dims[i].len() {
+    return sim_err!(MismatchedDimensions, id.as_str().to_string());
+}
+```
+
+**Verification:**
+Run: `cargo test -p simlin-engine dimension_position`
+Expected: Existing `dimension_position_reorder` and `dimension_position_3d` tests still pass (they use the A2A path unchanged).
+
+**Commit:** `engine: resolve @N position syntax to concrete offset in scalar context`
+<!-- END_TASK_1 -->
+
+<!-- START_TASK_2 -->
+### Task 2: Switch existing tests to `assert_compiles_incremental()`
+
+**Verifies:** assert-compiles-migration.AC1.1, assert-compiles-migration.AC1.2
+
+**Files:**
+- Modify: `src/simlin-engine/src/array_tests.rs:334` (`dimension_position_single` test)
+- Modify: `src/simlin-engine/src/array_tests.rs:1357` (`dimension_position_and_wildcard` test)
+
+**Implementation:**
+
+In `dimension_position_single`: change `project.assert_compiles()` to `project.assert_compiles_incremental()`. Remove the comment about @N not being supported on the incremental path. Keep `assert_sim_builds()` and `assert_scalar_result()` calls unchanged (they use the AST interpreter for cross-validation).
+
+In `dimension_position_and_wildcard`: change `.assert_compiles()` to `.assert_compiles_incremental()`. Remove the comment about @N not being supported.
+
+**Verification:**
+Run: `cargo test -p simlin-engine dimension_position_single`
+Run: `cargo test -p simlin-engine dimension_position_and_wildcard`
+Expected: Both pass.
+
+**Commit:** `engine: switch @N position tests to incremental path`
+<!-- END_TASK_2 -->
+
+<!-- START_TASK_3 -->
+### Task 3: Add error tests for out-of-range @N
+
+**Verifies:** assert-compiles-migration.AC1.3, assert-compiles-migration.AC1.4
+
+**Files:**
+- Modify: `src/simlin-engine/src/array_tests.rs` (add new tests in `dimension_position_tests` module)
+
+**Implementation:**
+
+Add two new tests in the `dimension_position_tests` module that verify the incremental compiler rejects invalid @N positions:
+
+- `dimension_position_zero_is_error`: Build a `TestProject` with an indexed dimension (size 3), an array, and a scalar aux using `arr[@0]`. Assert `project.compile_incremental()` returns `Err` and the error message contains the variable name (`first_elem` or similar) to confirm the error is specifically about the @0 reference, not an unrelated compilation issue.
+
+- `dimension_position_out_of_range_is_error`: Build a `TestProject` with an indexed dimension (size 3), an array, and a scalar aux using `arr[@5]`. Assert `project.compile_incremental()` returns `Err` and the error message contains the variable name to confirm it is specifically about the out-of-range @N.
+
+Follow the existing `TestProject` builder pattern from tests like `dimension_position_single`.
+
+**Verification:**
+Run: `cargo test -p simlin-engine dimension_position`
+Expected: All dimension_position tests pass (existing + new).
+
+**Commit:** `engine: add error tests for out-of-range @N position syntax`
+<!-- END_TASK_3 -->
+
+<!-- END_SUBCOMPONENT_A -->