engine: delete the fragment-compiler address round trip (row C)

docs/array-design.md

@@ -105,7 +105,7 @@ Indexed dimensions (e.g., numeric dimensions like `Periods(5)`) can use position
 - All expression types: `StaticSubscript`, `TempArray`, `TempArrayElement`, `AssignTemp`
 
 #### Bytecode VM
-- View stack operations: `PushVarView`, `PushTempView`, `PushStaticView`
+- View stack operations: `PushStaticView`, `PushTempView`, `PushVarViewDirect`
 - View manipulation: `ViewSubscriptConst/Dynamic`, `ViewRange/Dynamic`, `ViewStarRange`, `ViewWildcard`, `ViewTranspose`
 - Iteration: `BeginIter`, `LoadIterElement`, `StoreIterElement`, `NextIterOrJump`, `EndIter`
 - Reductions: `ArraySum`, `ArrayMax`, `ArrayMin`, `ArrayMean`, `ArrayStddev`, `ArraySize`

docs/design-plans/2026-03-06-finish-salsa-migration.md

@@ -62,6 +62,20 @@ only re-parse variables that actually reference the affected dimensions.
 - **finish-salsa-migration.AC3.2 Success:** `PREVIOUS(x)` where `x = SMTH1(input, 1)` compiles to module expansion (not `LoadPrev`) through the salsa incremental path.
 - **finish-salsa-migration.AC3.3 Success:** Editing an unrelated variable does not trigger re-parse of variables in the same model (salsa cache stability preserved).
 
+> **Annotation (2026-07-26, GH #568).** This document is a record of what was
+> decided in March, not a live contract; it is annotated rather than rewritten.
+> AC4.3, AC4.5 and AC5.2 have been *partly* overtaken and must not be actioned
+> as written. `all_deps` (and `direct_deps`/`module_deps`/`module_output_deps`)
+> is gone, but `set_dependencies` deliberately REMAINS as a thin adapter that
+> reads `db::dep_graph::model_dependency_graph` -- it is the monolithic
+> `compiler::Module`'s only source of runlists, and that monolith is the live
+> differential oracle for the run-invariance classifier
+> (`db::invariance::salsa_and_monolithic_paths_agree`). Deleting it per AC4.3
+> costs that oracle. AC4.5 is answered in `docs/tech-debt.md` item 17: the
+> `Variable::errors`/`unit_errors` fields are a LIVE error channel the salsa
+> path reads, not legacy residue, and removing them without first replacing the
+> channel silently drops diagnostics.
+
 ### finish-salsa-migration.AC4: Monolithic compilation path removed
 - **finish-salsa-migration.AC4.1 Success:** `compile_project` (free function in interpreter.rs) does not exist.
 - **finish-salsa-migration.AC4.2 Success:** `Simulation::compile()` does not exist.

docs/design/engine-performance.md

@@ -202,12 +202,14 @@ Op2`, 2→1). A leaf *assignment* `dst = a op b` keeps the existing
 (`BinOpAssignCurr` ≪ `Op2`).
 
 **Where it runs.** A late `ByteCode::fuse_three_address` pass applied to the Vm's
-flow/stock execution bytecode at `Vm::new`, reusing `peephole_optimize`'s
+flow/stock execution bytecode at `Vm::new`, reusing the symbolic peephole's
 jump-target guard + old→new PC remap and preserving `max_stack_depth`. It runs at
-`Vm::new` rather than compile time deliberately: the `CompiledSimulation` stays a
-pure, *symbolizable*, salsa-cached artifact (the symbolic roundtrip tests
-symbolize it; the fused opcodes have no symbolic form), and the `Vm`'s execution
-copy is where the optimization lives. Per-`Vm` fusion is a linear scan, negligible
+`Vm::new` rather than compile time deliberately: the fused opcodes have no
+symbolic form (`SymbolicOpcode` deliberately has no 3-address variants), so
+running the pass earlier would produce bytecode the salsa-cached artifact cannot
+represent -- and the `CompiledSimulation` must stay the pure resolution of the
+cached symbolic fragments. The `Vm`'s private execution copy is where the
+optimization lives. Per-`Vm` fusion is a linear scan, negligible
 vs a run. Initials are left unfused (run once; `extract_assign_curr_offsets` reads
 their `AssignCurr` targets).
 
@@ -330,9 +332,12 @@ only conclusive for effects that exceed ~4%.
 
 ### R4. `RuntimeView` allocation + `flat_offset` (~20% of post-win run)
 
-`PushVarView`/`PushTempView` rebuild `SmallVec`s (dims, strides, dim_ids) on every
-execution; `flat_offset` (10.3%) recomputes row-major offsets per element. For
-arrayed models this is now the #2 run cost.
+`PushTempView`/`PushVarViewDirect` rebuild `SmallVec`s (dims, strides, dim_ids)
+on every execution; `flat_offset` (10.3%) recomputes row-major offsets per
+element. For arrayed models this is now the #2 run cost. (This item was written
+when a third opcode, `PushVarView`, shared the cost; it was deleted in GH #964's
+stage 2 as unemittable by codegen, which is part (a) of the proposal below
+already realized for the whole-array case.)
 
 Proposal: (a) push more views through the compile-time `PushStaticView` path
 (precomputed `StaticArrayView`) and store dynamic view descriptors in the
@@ -400,8 +405,9 @@ re-derivation. (b) is broader but touches many call sites.
 2. ~~**R1 (bounds-check elimination)**~~ — INVESTIGATED, dropped: measured
    sub-noise (~0) ceiling; bounds checks are effectively free at opt-level=3.
 3. ~~**R2 (3-address binop fusion)**~~ — DONE. Flow opcodes −23.5%, run −6.8% on
-   C-LEARN; a late `fuse_three_address` pass at Vm::new (the `CompiledSimulation`
-   stays symbolizable). A full register VM would cut more but is a large rewrite.
+   C-LEARN; a late `fuse_three_address` pass at Vm::new (the fused opcodes have no
+   symbolic form, so they must not exist before assembly). A full register VM would
+   cut more but is a large rewrite.
 4. ~~**R4 (RuntimeView)**~~ — largely DONE via round 2's `dense_linear_start`
    fast paths (`flat_offset` 8.2% -> ~4% of a smaller run); the residual is
    the strict-slice `vector_elm_map` base and `offset_for_iter_index`'s

docs/tech-debt.md

@@ -143,13 +143,19 @@ Known debt items consolidated from CLAUDE.md files and codebase analysis. Each e
 - **Owner**: unassigned
 - **Last reviewed**: 2026-02-15
 
-### 17. Remove Legacy Error Fields from Variable/ModelStage Types
+### 17. Embedded Error Fields on Variable/ModelStage Types
 
 - **Component**: simlin-engine
-- **Severity**: low
-- **Description**: The `errors` and `unit_errors` fields on `Variable`, and the `errors` field on `ModelStage0`/`ModelStage1`, are now redundant with the salsa incremental compilation pipeline. Diagnostics are collected via `collect_all_diagnostics` / `collect_model_diagnostics` from tracked functions, making the embedded error fields dead weight carried through the monolithic compilation path. Removing them would simplify the data model and reduce confusion about the source of truth for errors. This cleanup was identified as Step 13 in the incremental compilation design (`docs/design/incremental-compilation.md`) but is not required by any acceptance criterion.
+- **Severity**: RESOLVED (2026-07-26) -- **the original premise was half wrong; do not act on it**
+- **Description**: This entry used to claim that the `errors`/`unit_errors` fields on `Variable` and the `errors` field on `ModelStage0`/`ModelStage1` were all "redundant with the salsa incremental compilation pipeline ... dead weight carried through the monolithic compilation path", and that removing them would simplify the data model. Every read site was classified before acting, and the four fields fall into two very different groups.
+
+  **`Variable::errors` and `Variable::unit_errors` are LIVE error channels, not residue.** They are how parsing and lowering report a failure on a variable, and the salsa diagnostics are *downstream* of them: `db::var_fragment::lower_var_fragment` reads `parsed.variable.unit_errors()` (the malformed-`<units>`-string rows), `parsed.variable.equation_errors()` (parse errors, where the conveyor/queue driven-flow `EmptyEquation` suppression applies) and `lowered.equation_errors()` (`MismatchedDimensions` and everything else `lower_ast` raises), and turns each entry into a `Diagnostic`. Emptying the lowered read makes every dimension-mismatch diagnostic vanish; emptying the parsed read turns a spec-sanctioned empty equation on a conveyor-driven flow into a phantom error. Deleting these fields without first replacing the channel would silently drop diagnostics. The claim is now pinned as a test rather than left as prose: `db::diagnostic_tests::variable_error_fields_are_the_lowering_channel` asserts both that the stage's value carries the error in the field and that the matching diagnostic reaches `collect_all_diagnostics`.
+
+  **`ModelStage0::errors` and `ModelStage1::errors` are read only by test-only code**, which is the half of the claim that held. `ModelStage0::errors` carries the duplicate-canonical-ident collision (GH #891) and is read only by `ModelStage1::new`; `ModelStage1::errors` is the monolithic path's simulatability gate (`compiler::Module::new` refuses a model with a non-empty list) and is otherwise read only by tests. They are kept and documented as such on the types: the gate is the reason they exist, `compiler::Module::new` has no salsa database to consult instead, and it is the only thing stopping that path from mis-compiling a resolved recurrence SCC (see `project::tests::build_module_refuses_a_resolved_recurrence_scc`). What WAS dead has been deleted -- `Variable::push_error` and `ModelStage1::get_unit_errors`, which existed for the mutation-based error collection the second dependency walk did before GH #568 unified the cycle gate, plus `Variable::push_unit_error` and the write-only `ModelStage1::unit_warnings`, which had no caller and no non-`None` writer respectively even before that.
+
+  Test helpers that used to report the monolithic path's embedded errors (`test_common::TestProject::compile`/`build_module`) now report `collect_all_diagnostics` instead: the production source of truth, and a different and better set rather than a strict superset. It gains the unknown-dependency, bare-lookup-table and assembly diagnostics and a source location on every row; it loses the model-level `VariablesHaveErrors` roll-up, the `("project", UnitDefinitionErrors)` row, and the old path's cycle verdict on element-acyclic recurrences -- the last being the whole point of deleting it.
 - **Owner**: unassigned
-- **Last reviewed**: 2026-02-22
+- **Last reviewed**: 2026-07-26
 
 ### 18. Dimension-Granularity Incremental Invalidation
 

scripts/check-docs.py

@@ -137,11 +137,15 @@ def main() -> int:
 
     # All CLAUDE.md files
     for claude_md in repo_root.rglob("CLAUDE.md"):
-        # Skip generated/noise directories
+        # Skip generated/noise directories. `.claude-scratch` is git-excluded
+        # agent scratch space INSIDE the repo, so a working copy of a CLAUDE.md
+        # routinely lands there mid-task; walking it reports every relative path
+        # in that copy as broken (~150 errors) and fails the pre-commit hook on
+        # a tree that is actually fine.
         rel = claude_md.relative_to(repo_root)
         parts = rel.parts
         if any(p in ("node_modules", "target", "build", "lib", "lib.browser", "lib.module",
-                     "third_party")
+                     "third_party", ".claude-scratch")
                for p in parts):
             continue
         files_to_check.append(claude_md)