engine: performance pass on compile, simulation, and the LTM path - #1021
Conversation
The clearn_profile harness backed its counting allocator with the system allocator, but every native binary that embeds the engine (simlin-cli, simlin-serve, simlin-mcp, and libsimlin under the mimalloc feature that pysimlin's build turns on) installs mimalloc. Since the compile path is allocation-bound, that measured an allocator no shipped native build runs and over-credited any change that only moves malloc traffic: on C-LEARN the same compile is 194 ms on system malloc vs 171 ms on mimalloc. Allocation counts remain the allocator-independent metric, and the one that carries over to the wasm bundle, which links neither. CLEARN_LTM=1 compiles with Loops That Matter enabled so the LTM-augmented compile and run are measurable through the same per-phase accounting as the ordinary path, rather than only through ltm_full_bench (which stops at compile and never simulates).
`compiler::codegen`'s `Expr::If` arm emits `SetCond` and `If` in one breath and is the sole producer of either, so the pair is adjacent by construction -- measured on C-LEARN as exactly equal executed counts (1,874,169 each, 6.38% of dispatches apiece). Neither `peephole_optimize` nor `fuse_three_address` can separate them: both only ever REPLACE an adjacent run, and `SetCond` is neither a leaf load nor a combiner, so no fusion window can absorb it. An `AssignCurr` follows ~91% of executed `If`s (LBR-sampled, corrected for window truncation against the structurally-known 100% SetCond->If pair), so the three-opcode form is the dominant shape and gets its own fused opcode. The pass lives in `ByteCode::fuse_three_address` (Vm-local) rather than the symbolic layer. The rule that decides this is worth stating in its reusable form: a fusion may live in the symbolic layer iff the fused opcode has a `SymbolicOpcode` form, because `CompiledSimulation` must stay the pure resolution of the salsa-cached symbolic fragments. That is why `peephole_optimize` can fuse LoadConstant+AssignCurr and Op2+AssignCurr (both have symbolic twins) while the 3-address family cannot. A symbolic home was possible here -- add `SymbolicOpcode::SelectIf` -- but it would rewrite every golden in `src/db/fragment_char_golden/`, change the cached artifact, and need `resolve` + wasmgen arms, all for the same dispatch reduction. The wasm backend therefore does not inherit this win; it already documents that late-fusion superinstructions never reach it and returns a loud `WasmGenError::Unsupported` if one ever did, so the premise fails noisily rather than mis-lowering. Three properties checked before adding opcodes: `fuse_three_address` runs only on the Vm's private `flows`/`stocks` copies, initials are left unfused, and both `invariant_flow_offsets` and `collect_constant_info` read the PRE-fusion cached bytecode -- so neither the GH #712 invariance oracle nor the constant-override set can be weakened by a new fused form. Stack effects are asserted, not assumed: `SelectIf` is (3,1) and `SelectIfAssignCurr` is (3,0), identical in net effect and peak to the `SetCond`(1,0) + `If`(2,1) [+ `AssignCurr`(1,0)] sequences they replace, which is what keeps `resolve_bytecode`'s fixed-stack safety proof valid. Measured (perf stat, 26 runs/side, one build pair): C-LEARN instructions -4.00%, branches -6.98%, cycles -6.26%, IPC 3.04->3.12 WORLD3 instructions -2.24%, branches -5.87%, cycles +0.62% (flat) One expectation this refutes. I predicted the removed dispatches would also remove indirect-branch mispredicts (GH #604's threshold hypothesis). They did not: C-LEARN branch-misses were flat (+0.7%) while branches fell 7.0%, so the miss RATE rose slightly. In hindsight this is what should have been predicted -- `SetCond`'s dispatch always jumps to `If`'s arm, making it among the most perfectly-predicted indirect branches in the program. Fusing away predictable dispatches buys instructions and branches, not mispredicts. The remaining mispredicts sit in the genuinely-unpredictable dispatches, which this class of superinstruction cannot reach. Behaviour-preserving: 5473 engine lib tests and 631 integration tests pass, including `clearn_residual_exactness` (which pins the exact SET of residual variables, not just a tolerance), `oracle_clearn`, and `vdf_parity`.
…operands `Opcode::Apply` unconditionally popped three operands, so codegen pushed `LoadConstant(0.0)` padding for every shorter call: two pads for the 14 single-operand builtins, one for the four two-operand ones. `vm::apply` then discarded them. Measured on C-LEARN that is 0.73 wasted pads per executed `Apply` -- 583k dispatches per run, 2.0% of all dispatches -- and on WORLD3 the padding is a smaller share only because WORLD3 barely uses builtins. The arity turns out to be a property of the BUILTIN, not of the call site. I had assumed otherwise (that `PULSE`/`RAMP`/`SAFEDIV` with an omitted third argument would need a per-site count) but reading `vm::apply` arm by arm settles it: codegen substitutes a real VALUE for those three, not a pad -- `PULSE`'s third defaults to `0` and `apply` reads it, `SAFEDIV`'s third IS the divide-by-zero result, and `RAMP`'s defaults to `final_time` via `LoadGlobalVar`. So they are genuinely 3-operand and the count is a pure function of `BuiltinId`. No opcode payload is needed. `BuiltinId::arity()` is that one table, and its three consumers all read it -- codegen (how many to push), `Opcode::stack_effect` (how many the opcode pops, which is what keeps `max_stack_depth` and `resolve_bytecode`'s fixed-stack proof in step with what is actually pushed), and the `Apply` arms in `vm.rs` and `wasmgen::lower`. Keeping it in one place is what makes the three unable to disagree; the match is exhaustive with no `_`, so a new builtin cannot be added without deciding its arity. Two test fixtures hand-built the padded shape and had to be rebuilt from the arity, which is the point of the repo's rule that a fixture must construct what production supplies: `apply_eval` pushed three `LoadConstant`s unconditionally, and `apply_inside_if_does_not_clobber_condition` spelled out two pads. Both now derive the operand count, so they exercise a stream codegen can actually emit -- before, they built one it cannot, and the wasm module failed validation with `EndInvalidValueStack` rather than telling us anything about padding. Two LTM goldens regenerate. The regeneration was inspected rather than blessed: computing the net change per opcode mnemonic across both files, the ONLY non-zero entry is `LoadConstant` (-6 in ltm_loop_exhaustive, -10 in ltm_loop_discovery). Every other opcode's count is unchanged; the rest of the diff is position shift. That is exactly the padding and nothing else. Measured cumulatively with the preceding SetCond;If fusion (perf stat, 26 runs/side, one build pair; instructions and branches are the layout-insensitive numbers, cycles bounce several percent between builds): C-LEARN instructions -4.25%, branches -7.72%, cycles -5.38% WORLD3 instructions -2.35%, branches -6.12%, cycles -7.58% Behaviour-preserving: 5475 engine lib tests and 631 integration tests pass, including `clearn_residual_exactness`, `oracle_clearn`, and `vdf_parity`.
`compile_ltm_synthetic_fragment` routed only the scalar `Bare` from->to link score through a memoized query; every element-pinned, aggregate-touching, A2A and loop score took the plain-function `compile_direct` path. Two walkers over the LTM variable list -- `assemble_module`'s pass 3 and `model_ltm_fragment_diagnostics` -- therefore compiled those fragments twice, independently. On C-LEARN that is 5,985 of 7,125 variables, whose compilation callgrind measures at ~9.98e9 instructions, roughly half of a full LTM compile stage. The duplication is paid in shipped flows, not just in principle: libsimlin's `simlin_project_get_errors` (and so pysimlin's `Project.get_errors`) runs the diagnostic pass after `simlin_sim_new` has already assembled, MCP `read_model` runs it once, and MCP `edit_model` runs it on a pre-edit and a post-edit database and then assembles again. Both walkers now reach a fragment through `compile_ltm_fragment_at`, keyed by the variable's index into `model_ltm_variables(..).vars`. The index is what both walkers already hold, and it keeps the query a salsa firewall: it reads the whole-model variable list so it re-executes on any edit, but its value is one fragment, so salsa backdates it whenever that fragment is unchanged and assembly is not re-run -- the same shape as `reconstruct_named_variable` over `reconstruct_model_variables`. Behaviour-preserving: C-LEARN's LTM bytecode is byte-identical at 1,238,728 opcodes and its slot count is unchanged, and WORLD3's LTM compile and run are unmoved. The cost is retention -- roughly 7k symbolic fragments now stay live in the database rather than being dropped after assembly, measured at +57 MiB peak on C-LEARN (438.8 -> 496.3 MiB) for +0.07% allocations. The compile-only path gains nothing from this change and pays one fragment clone per variable; the win is entirely in the second walk. `FragmentExecKind::LtmBody` records the fragment-compile body wherever it runs, which is what lets the reuse be tested at all: a memo hit is invisible to a timing but so is a cache miss, and pointer equality cannot see it because salsa backdates a re-executed query whose value compares equal.
`AssignCurr` is 10.68% of executed dispatches on C-LEARN, and the measured bigrams account for essentially all of it: `If` 5.64% (taken by the preceding conditional-select fusion), `LoadModuleInput` 1.48%, `LoadVar` 1.41%, `LoadInitial` 1.34%, `Apply` 0.85%. The three leaf loads that feed a store get a fused register-style form here -- `AssignVarCurr` (a slot-to-slot copy, which is what an alias or pass-through variable compiles to), `AssignInitialCurr`, and `AssignModInputCurr`. `Apply; AssignCurr` is deliberately NOT fused. The `Apply` arm inlines every builtin body, so duplicating it to fold a store would be the largest code growth in the hot function for the smallest member of the set, and `eval_bytecode` is already >= 66 KB against a 32 KB L1i. Separately, `LoadModuleInput` was not a fusible leaf at all, despite being 4.68% of C-LEARN dispatches and 5.8% of WORLD3's -- the 2-window handled `LoadVar`/`LoadConstant`/`LoadGlobalVar` and stopped there. It now joins them with `BinStackModInput` and `AssignStackModInputCurr`. Module inputs stay 2-window only: 3-window leaf forms would need one opcode per (leaf x leaf) pairing for a measured minority of the bigrams, and each new arm costs icache in the hot function permanently. `LoadConstant; AssignCurr` is absent from the leaf-store set because it never reaches this pass -- the symbolic `peephole_optimize` already folds it into `AssignConstCurr`. Stack effects are asserted: the three stores are (0,0), exactly the net of the `LoadX`(0,1) + `AssignCurr`(1,0) they replace, so a program's peak depth cannot move; `BinStackModInput` is (1,1) and `AssignStackModInputCurr` is (1,0), mirroring their var/const twins. A test drives all three stores through `max_stack_depth` and pins 1 before, 0 after. Operand order is pinned for the non-commutative Sub and Div, where a swapped encoding would be a silent miscompile rather than a loud failure. Measured for the whole Stage A bundle against the pre-bundle baseline (perf stat, 26 runs/side, two interleaved rounds; instructions and branches are the layout-insensitive numbers, cycles moved several percent between builds of identical source so they are reported but not leaned on): C-LEARN instructions -6.22%, branches -6.34%, cycles -5.20% WORLD3 instructions -4.07%, branches -3.67%, cycles -0.82% This commit is the largest single contributor: it took C-LEARN from -4.25% to -6.22% and WORLD3 from -2.35% to -4.07%. Behaviour-preserving: 5482 engine lib tests and 631 integration tests pass, including `clearn_residual_exactness`, `oracle_clearn`, and `vdf_parity`.
`compiler::context`'s `is_dimension_name` canonicalized every declared dimension's name on every call, to compare it against the canonicalized subscript. A `Dimension`'s name is a `CanonicalDimensionName`, canonical at every site that builds one -- both arms of `From<&datamodel::Dimension>` and every other production construction go through `CanonicalDimensionName::from_raw` -- so that inner call could not change its input, and `canonicalize` still scanned the whole string to decide that. The predicate runs once per bare-identifier subscript per reference (`IndexExpr3::from_index_expr2` asks it whether the index names a dimension), so the redundant scan was paid once per DECLARED DIMENSION per subscript. On C-LEARN, whose project declares 126 dimensions, that one call site was 1.09M of the compiler's 2.15M `canonicalize` calls; removing it measures -5.0% of a cold compile's instructions (interleaved A/B, three rounds: 8.528G -> 8.102G over four compiles). The premise is what makes this safe rather than merely faster, so it is pinned directly: `dimension_name_is_canonical_for_every_constructor` asserts `Dimension::name()` equals `canonicalize(raw)` over both the Named and the Indexed arm (they canonicalize at separate call sites) for the shapes canonicalization actually changes -- case, interior whitespace, padding, and a dotted name. A constructor that stopped canonicalizing would red that test rather than silently making this predicate miss a dimension. The compiled artifact is unchanged: C-LEARN still assembles 5215 slots and 58291 opcodes (31525 flow + 1477 stock + 25289 initial), 2196 literals, 162 graphical functions over 37065 points, 28 temp slots, 126 dimensions, 643 static views, 371 names and 7 modules.
Every emitted link score is cloned out of the `link_score_equation_text_shaped` memo into `model_ltm_variables`' own variable list (`db/ltm/link_scores.rs`), so each generated equation's parsed tree was retained TWICE for the life of the database. On C-LEARN the generation stage retains +273 MiB for 12.78 MB of equation text, and the ASTs dominate that. `LtmArm::expr` is now an `Arc<Expr0>`, so that clone is a refcount bump and one copy is retained. Measured on C-LEARN: peak live bytes 496.3 -> 440.7 MiB (-55.6 MiB) and 1.3M fewer allocations, with the LTM bytecode byte-identical at 1,238,728 opcodes and the root slot count unchanged. `Arc<Expr0>` still compares BY VALUE. That is load-bearing rather than incidental: salsa backdates a re-executed query whose value compares equal, and that backdating is what lets an unrelated edit reuse the expensive downstream fragment (GH #981). The existing NaN-equality test pins it, and the new sharing test asserts value equality alongside pointer identity so a future change cannot buy sharing by weakening comparison. Building an `Ast<Expr0>` in `to_flow_ast` still unshares, because that type owns its tree. That is the right split: the result is consumed by the fragment compile and dropped, while the arm is retained for the life of the database -- so sharing bounds retention rather than avoiding a transient copy. Pointer identity is the only way to observe this, since both copies compare equal either way, so a value assertion would pass on a deep copy. The new test was verified to constrain the code by temporarily reintroducing the deep copy in `retarget_dims` and confirming it fails.
`svg_to_png` built a fresh `usvg::fontdb::Database` on every call, copying the 170 KB embedded Roboto Light with `to_vec()` and re-parsing the face each time. The database holds exactly one face and never changes, so every render after the first re-derived an identical immutable value; a `simlin_project_render_png` FFI caller rendering N diagrams paid it N times. This is a shipped-path cleanup, not a test-speed change, and the measurement says so: the 16 `render_png` unit tests take 2.51 s before and 2.53 s across three alternating runs -- indistinguishable. Those tests are dominated by SVG filter rasterization (`perf`: ~80% in `resvg::filter::morphology::apply`), so the font parse was never a visible share of them. What the change buys is bounded by that same measurement: under 3 ms and one 170 KB allocation per render. `usvg::Options::fontdb` is already an `Arc`, so sharing costs a refcount bump and no caller can observe the difference.
`compiler::codegen` emits a `LoadConstant` for a lookup's element offset before the index expression, and for a scalar table that constant is always a literal 0 -- a push the VM immediately pops and range-checks. It is 429k dispatches per C-LEARN run and 5.1% of WORLD3's, where roughly 70% of ALL constant loads are these zeros. The push is not adjacent to its `Lookup` (the index expression sits between), so no peephole can remove it; codegen has to not emit it. `LookupDirect` carries the resolved element instead. `const_element_offset` accepts only a non-negative integral constant strictly inside `[0, table_count)` that fits `u8`, and each condition is load-bearing: the VM's runtime path truncates with `as usize` after rejecting negatives, so a fractional or negative constant would fold to a different table than the runtime rule picks; an out-of-range constant must keep the general form so the VM still yields its documented NaN; and `u8` is the width the 8-byte `Opcode` budget leaves, so an arrayed GF with 256+ elements keeps the runtime push. Every rejected shape falls back to the unchanged `Lookup`. THE DURABLE PART OF THIS COMMIT IS THE MERGE TEST, not the 1-3.6pp. `gf_blocks_of_fragment` reconstructs each fragment's GF block layout by scanning its opcodes for `(base_gf, table_count)` runs, and its match ends in `_ => continue`. A lookup-family opcode it does not know about is therefore skipped SILENTLY: the referenced runs stop being seen, collapse into one maximal un-referenced GAP block, and the de-duplicated table layout comes out wrong with no diagnostic anywhere -- wrong numbers, not an error. That is a defect in its own right and it is why this change needed an audit rather than a patch. `test_gf_block_scan_sees_lookup_direct_runs` is built to fail on exactly that. Two distinct single-table blocks in one fragment, both read through `LookupDirect`, merged with a fragment holding only the second table's content. With the scan correct there are two deduped tables; with `LookupDirect` unknown the two runs collapse into one gap block keyed by its whole content, the shared table stops matching, and the merge yields three. Asserting the deduped COUNT is what makes it discriminating -- a single-block fixture dedups identically either way and would pin nothing. The `_ => continue` now carries the obligation in a comment naming this test. Nine `base_gf` sites, audited by hand because that one is silent. Eight are exhaustive matches and two of them caught the omission at compile time -- `db::fragment_char_tests`' opcode renderer and `symbolic_merge_proptest`'s shrinker -- which is the mechanism working as intended. The ninth, `per_element_gf_tests`' nameless-opcode enumeration, is a test whose claim `LookupDirect` strengthens rather than weakens: it asserts the per-element reorder is materialized at compile time and the hot path does no name lookup, and this opcode resolves the element at compile time too. The wasm backend splices the constant in beneath the index and reuses `emit_lookup` rather than growing a second copy of the directory-read plus helper-call sequence. It passes `table_count = elem + 1`, making that lowering's range check vacuously true -- sound rather than a fudge, since codegen only emits the opcode when `elem < table_count`, so the check was already discharged at emit time. Two goldens regenerate and the regeneration is fully explained: net per opcode mnemonic is `Lookup` -5 / `LoadConstant` -5 / `LookupDirect` +5 in graphical_functions.txt and -2/-2/+2 in lookup_only_table.txt. Every lookup became a direct lookup and its paired constant push vanished 1:1; no other opcode moved. Stage A bundle measured against the pre-bundle baseline (perf stat, 26 runs/side, two interleaved rounds). Instructions and branches are the layout-insensitive numbers; the machine was heavily contended for round 2, where the SAME baseline binary measured 17.95e9 cycles against 14.24e9 in round 1, so cycles are reported and not leaned on: C-LEARN instructions -7.22%, branches -6.49% WORLD3 instructions -7.66%, branches -5.69% This commit contributed +1.0pp on C-LEARN and +3.6pp on WORLD3 -- well above the ~1.4pp I projected for WORLD3, because the projection counted only the dispatch and not the popped-and-discarded operand work it takes with it. Behaviour-preserving: 5483 engine lib tests and 631 integration tests pass, including `clearn_residual_exactness`, `oracle_clearn`, `vdf_parity`, and the wasm parity corpus.
`variable_dimensions` asked for a parse under an EMPTY `ModuleIdentContext`, noting that the module context does not affect dimension extraction. That is true, and it was also the problem: the context is part of the parse's salsa cache key, and this query takes no `model`, so the empty context is the only one it could name. Every variable was therefore parsed a second time under a key nothing else in the pipeline uses -- on C-LEARN, 1,910 executions of `parse_source_variable_with_module_context` for 934 variables. The declared dimensions are available without parsing anything: they are the dimension-name list on `datamodel::Equation` itself, which `variable_relevant_dimensions` already reads, resolved through the same `variable::get_dimensions` the parse calls. The derivation mirrors `parse_source_variable_impl`'s narrowed dimension context exactly -- the relevant names widened by `expand_maps_to_chains` and filtered out of `project_datamodel_dims` -- so a name resolves here iff it resolves there. Keeping that narrowing rather than reading the whole-project `project_dimensions_context` is what preserves dimension-granularity invalidation: a scalar takes the early return and never depends on the project's dimensions at all. Measured -3.5% of a cold C-LEARN compile and -6.7% on WORLD3 (interleaved A/B, three rounds), the larger share on WORLD3 because it declares no dimensions and so gains nothing from the sibling `is_dimension_name` change. ONE arm deliberately differs from the parse. `parse_equation` builds an A2A as `ast.map(|ast| Ast::ApplyToAll(dims, ast))`, so a variable whose equation does not parse produced no `Ast` and reported no dimensions -- giving it a `variable_size` of 1 despite a declared extent. This reports the declared shape. The divergence is confined to a project that already fails to assemble (the parse error still reaches `compile_var_fragment`, which drops the fragment and accumulates the diagnostic), and it moves the reported size from a wrong 1 toward the declaration, so nothing that compiled before reads a different slot. `Arrayed` is unchanged in both its failure modes, and so is the unresolvable-dimension-name arm. The tests assert against the previous implementation as an ORACLE rather than against hand-written expectations, which is what makes the agreement claim mean anything: the first draft's hand-written row for a canonically spelled reference to a `DimA`-cased dimension was wrong in a way only the oracle caught. Both paths share a pre-filter that seeds `expanded` with the equation's RAW dimension names and then filters by display name, so such a reference resolves to nothing on either path -- a property of the shared narrowing rather than of either implementation, and left exactly as it was. The compiled artifact is unchanged: C-LEARN still assembles 5215 slots and 58291 opcodes (31525 flow + 1477 stock + 25289 initial) with the same literal, graphical-function, temp, dimension, view, name and module counts.
The `diagram::render_png` tests rasterize a real diagram through resvg, and at opt-level 0 that is almost entirely SVG filter code: `perf` on the slowest of them attributes ~80% to `resvg::filter::morphology::apply`, with the un-inlined `<u8 as Ord>::max` and `core::cmp::max::<u8>` inside it accounting for ~35% between them. None of that is engine code, so no amount of engine work moves it. Pinning the stack to opt-level 3 takes the 16 `render_png` tests from 2.54s to 0.33s (-87%, three alternating runs each) and takes the engine's lib-test binary's longest single test from 2.35s to 1.55s. That second number is the structural one: a test binary's parallel wall is max(longest test, cpu_sum/threads), so the PNG test was setting a floor no amount of parallelism could get under. It no longer is -- the vdf truncation sweep is now the longest test. Whole-binary CPU drops 61.5s to 57.2s, which is what a 4-core CI runner (throughput-bound, not floor-bound) actually collects. The cost is one rebuild of these crates and nothing thereafter: a dependency is not recompiled by an edit to a workspace crate, so unlike an opt-level on a workspace crate this never touches the edit-compile-test loop. Measured: a cold `cargo test -p simlin-engine --no-run` is 61.3s unpinned vs 58.7s pinned, inside run-to-run noise. All nine crates are reached through libsimlin's default `png_render` feature, so a workspace build always has them. A build that does not -- `cargo check -p simlin-engine --lib --no-default-features` and the `--no-default-features` wasm32 bundle build -- emits no unmatched-spec warning; both were checked.
Every model any test compiles goes through salsa's query engine and its indexmap-backed dependency edges, so unlike the render pin -- which is concentrated in 16 tests -- this one is spread thinly across the entire suite. Measured on the engine's two test binaries, three runs each: lib-test CPU 57.0s -> 46.5s and integration CPU 64.8s -> 54.0s, about -17% on both. That is the number a 4-core CI runner collects, since both binaries are throughput-bound rather than floor-bound there. The cost is one rebuild of these three crates and everything above them: 64.8s under `taskset -c 0-3`, i.e. paid once per CI cache generation and never again. It does NOT recur on the edit-compile-test loop, because a dependency's artifacts are keyed by its own fingerprint and an edit to a workspace crate cannot invalidate them. Checked rather than assumed: a real content edit to `src/simlin-engine/src/vm.rs` followed by `cargo test -p simlin-engine --no-run` averages 10.8s with the pin and 12.0s without it -- the same number, within noise, and certainly not worse. That asymmetry is why pinning a DEPENDENCY and pinning a WORKSPACE crate are different trades, and why only the former is done here. `hashbrown` here is the standalone crate behind indexmap and salsa, not the copy vendored into std; `std::collections::HashMap` is untouched.
Between one Euler step and the next, exactly three classes of slot carry a
value forward rather than being rewritten by the Flows or Stocks phase:
1. The `IMPLICIT_VAR_COUNT` implicit globals. `run_initials` pre-fills
DT/INITIAL_TIME/FINAL_TIME across EVERY chunk of the slab once, after
which `run_to` advances only TIME. A run-initialization invariant, not a
per-step one.
2. Stocks, written into `next` by the Stocks phase and reaching the next
step's `curr` through the chunk ring.
3. Standalone lookup-only table holders (#606) -- excluded from every runlist
AND from the saved output, their data reached through `base_gf` into
`graphical_functions` and never through the slot. Storage no consumer can
observe.
`Vm::poison_next_chunk_for_test` fills `next` past the implicit prefix with a
sentinel at the top of every Euler step, so a slot that carries forward
silently surfaces as the sentinel in the saved results. The test compares the
slots reachable through `Results::offsets`, which is exactly the set a consumer
can name.
Preserving the prefix rather than poisoning it and ignoring it afterwards is
the whole point of the fixture. `Context::build_stock_update_expr` emits
`stock + (inflows - outflows) * Expr::Dt` and `Expr::Dt` lowers to a
`LoadGlobalVar { off: DT_OFF }` read of `curr[DT_OFF]`, so poisoning `dt`
corrupts every stock in the model. A whole-chunk poison reports widespread
staleness across 28 corpus models that is really one slot, which is precisely
the false signal this fixture has to avoid producing.
Class 3 was found by running the fixture, not by reading: with the prefix
preserved, no model diverges on a stock and exactly one does on two unnamed
slots -- the two `<gf>` holders in `lookups_simlin/test_lookups.xmile`.
The invariant is load-bearing for any change that stops carrying a chunk's
contents forward: swapping the chunk indices instead of copying, hoisting
run-invariant work out of the per-step program, or partially evaluating a step.
Each must carry all three classes explicitly, and none of them has a test of
its own that would notice.
The expansion tier was three overlapping bulk tests: `metasd_expansion_tier`
over the light models, an `#[ignore]`d `metasd_expansion_tier_heavy` over
the rest, and `metasd_expansion_tier_full` over all 17. `_full` and the
light subset both ran by default, so every light model was compiled and
diagnosed twice per suite run; `_heavy`'s own doc comment already made the
argument for why -- "`_full` is a strict superset, so running both would buy
nothing" -- and it applied just as well to the light one.
It is now one `#[test]` per corpus model. The reason is the rule in
docs/dev/rust.md: a binary's parallel wall is
`max(longest test, total/threads)`, so a serial loop over a corpus sets a
floor no number of cores can get under. `metasd_expansion_tier_full` was the
second-longest test in the whole integration harness at 3.20s solo. The
longest of the 17 per-model tests is now 0.892s (scirev8), and the module's
total drops from 3.77s to 2.64s -- the difference being the duplicate light
pass. A failure also now names the model in the test name rather than only
inside an accumulated list.
Coverage goes UP, not down: `_heavy`'s five models were only ever run on
demand and now run by default. They fit easily -- scirev8 0.892s, scirev7
0.766s, free6 0.258s, beer-game 0.250s, covid19 0.081s -- which is also why
the `heavy` field is gone rather than reworded. Its per-entry annotations
("~3.6s compile" for scirev8) were stale by 4x after the compile work of the
last few rounds, and nothing read the field once both filters went away; a
stale measurement in a comment is worse than no comment.
The one way a per-model split can silently under-cover is a CORPUS entry
added without a matching test, so `corpus_is_exactly_the_17_macro_using_metasd_files`
now asserts the generated test-name set equals the corpus name set in both
directions (and that the names are unique). Checked by mutation: deleting
one name from the `expansion_tier_tests!` list reds that guard.
`simulate_path_with_excluding` compiled and ran every corpus model three times on the VM -- the original, the protobuf round-trip, and the XMILE round-trip -- plus once through the wasm backend. The protobuf leg already asserts `datamodel_project == datamodel_project2` before recompiling, so what the recompile-and-compare added was the question "is compilation a function of the datamodel?", asked once per corpus model. That is a real property but a different one, and it is owned by `db::fragment_determinism_tests`, which asserts it far more directly: byte-identical compiled output from independent fresh databases, twelve repeats, on fixtures chosen to expose the specific HashMap orderings that can break it. The XMILE leg keeps simulating, and the comment now says why: it asserts no datamodel equality (the reader legitimately normalizes), so simulating the re-read project is the only thing pinning its behaviour. Measured on the 58 `simulate::corpus::` tests -- the ones that actually go through this helper -- 0.965s to 0.796s solo, about -17%. Across the whole `simulate::` module it is -0.4s, since that module is dominated by the C-LEARN tests, which do not use this path. A small win, reported at its size.
The ~4% figure this file records is a WALL-CLOCK/CYCLES floor, and it has been getting applied to instruction counts, where the measured sd across independent builds is 0.026% -- three orders of magnitude apart. Retired instructions are a property of the program; cycles are a property of the machine executing it, and only the second is subject to the binary-layout lottery the figure describes. Conflating them is expensive in both directions. It makes a real instruction-level win look unmeasurable and get abandoned, and it sends anyone who wants an instruction-count result into a multi-build A/B that one build pair would have settled. The new section states the three channels and what each answers, the measured per-channel floors, and the rule that a cycles claim must clear the SAME session's null control rather than any floor recorded here -- machine conditions vary hour to hour, and taking a historical figure for the current one is how noise becomes a reported result. The null control is the evidence for all of it: the identical binary run as both sides of an interleaved A/B reports -0.003% on instructions and -1.540% on cycles, i.e. the cycles channel manufactures a 1.5% "win" out of nothing at load average 4-9. Two general techniques ride along because they are what actually decides these questions. Prefer a structural check to a statistical one where the change admits it: a change confined to an `#[inline(never)]` function with an unchanged signature should leave its callers byte-identical, which is a binary answer rather than a sample and directly detects the leak-into-`eval_bytecode` failure mode this file has recorded more than once. And decide the falsification signatures before measuring, so the eventual number is a result rather than a reading. The closing convention is the common cause of the whole problem: a recorded verdict should name the channel its number came from. A bare "only ~1.5%" invites the next reader to compare it against whatever floor they have in mind. The "methodology consequence" note under the #712 B2 result is rewritten to point here and to say which claim its ~4% bounds, rather than giving one threshold for every channel.
`compile_implicit_var_fragment` was a plain function, on the reasoning that "the parent variable's parse result already provides salsa caching". That is true of the PARSE and of nothing else: the lowering (`lower_implicit_var` -> `parse_var` -> `lower_variable`) and the per-phase codegen ran on every call, and both production call sites call it once per helper per assembly. So every SMOOTH/DELAY/TREND/PREVIOUS/INIT helper in a model was recompiled from scratch whenever `assemble_module` re-ran -- which any equation edit causes. On C-LEARN that is 651 helper compiles per cold assembly (~12% of a cold compile) and ~28% of the cost of a WARM single-equation edit, by far the largest share of a recompile that should have touched one variable and its consumers. Keyed on the helper's own canonical name. That is the only identity a helper has -- it exists solely inside its parent's parse -- and it is the key `model_implicit_var_info` files it under, so `model_implicit_var_by_name` resolves the metadata inside the query instead of the caller passing a borrowed `&ImplicitVarMeta` no salsa key could carry. `ImplicitVarMeta::name` already carries the argument for a name over a position, and bounds the one case where a name resolves to a different helper than the metadata meant -- a case that fails to compile regardless. The runlist gate reads a new `implicit_var_runlist_membership` rather than the whole `ModelDepGraphResult` the callers used to pass in, for the reason `compile_var_fragment` reads `var_runlist_membership`: a three-bit projection backdates when this helper's membership is unchanged, where depending on the whole result would re-execute every helper's fragment whenever any variable's dependencies moved -- reintroducing the coarseness this change removes. The two keyed projections share one `membership_in` body so they cannot answer the same question differently. Measured on C-LEARN (40 single-equation edits + 5 no-op recompiles, interleaved, three rounds): 20.49G -> 14.33G retired instructions, -30% of the whole warm workload and -32% per edit. Wall-clock median for one edit falls from ~40 ms to ~5-9 ms in the uncontended rounds. Cold compile and the compiled artifact are unchanged (5215 slots, 58291 opcodes). `implicit_and_ltm_fragment_cache_granularity` was a characterization pin on the old behaviour and is restated rather than deleted: it asserted that every helper recompiles on an edit to a variable none of them reads, and now asserts that none does. Its new complement -- editing the variable a helper DOES read -- turned out to recompile only ONE of the fixture's two helpers, which is correct and finer than expected: `builtins_visitor` passes a bare `Var` argument through by name and synthesizes a helper only for a computed one, so `SMTH1(src, 2)` wires `src` into the module instance and captures only the literal `2`. The granularity is per helper, not per parent.
The verdict is UNCHANGED. Only its stated reason is corrected; this is not a reopening of #711. The recorded reason was that the ~1.5% instruction share is "below the ~4% layout-noise measurement floor". That applies a cycles/binary-layout floor to an instruction-count measurement. The instruction channel's sd across independent builds is ~0.026%, so ~1.5% there is roughly 58 sigma -- comfortably measurable. The number was never the problem. Two reasons survive and are sufficient on their own. The return is small against the highest design cost of the three candidates: forward-jump opcodes touching codegen, `max_stack_depth` join validation, the peephole/fusion jump maps, the symbolic layer and wasmgen parity. And the cheap part of the win has since been taken without any of that machinery -- the `SetCond;If[;AssignCurr]` fusion removes 12.0% of executed dispatches against the projected 15.9% -- so what is left here is the residual, not the headline. Worth correcting rather than leaving inert because a wrong measurement premise is not caught by review: reviewers check the reasoning against the stated premise, not the premise against the world. "Below the measurement floor" reads as a fact and ends the enquiry, and this one had already carried a verdict. The same correction is posted on the issue, since that is where someone picks the work up and a doc-only fix would leave the trap in the more-read place.
`pnpm js-needs-format` runs on every commit, on pipeline B, which is the pre-commit hook's critical path. It was ~6s of that pipeline (and ~11s on a busier run) purely to re-decide, from scratch, that 379 unchanged files are still formatted. The suspicion that it was scanning generated output does not hold, and it is worth writing down so nobody re-checks it: of the files `find` emits, the existing filter already drops `lib/`, `lib.browser/` and `lib.module/`, and of the 379 that survive, ZERO are under `node_modules`, `build`, `dist` or `coverage` (pnpm's per-package `node_modules` are symlinks, which `find` does not follow). They are 379 real sources, 9 of them hand-maintained `.d.ts`. The cost is prettier itself at roughly 10ms per file. So cache it. Standalone the step goes 3.8s -> 0.9s warm (-76%, three runs). What this does NOT do, measured, is move the hook's wall clock: a same-session A/B of the whole hook is 29-30s with and without the cache, because inside pipeline B the binding costs are `pnpm build` (~15s) and tsc+test (~8s), and the format check was never the constraint. Landed because it is a one-line change that strictly removes work, not because a developer will feel it. `--cache-strategy content` rather than the default `metadata`: it keys on a content hash instead of mtime+size, so a checkout that rewrites mtimes cannot produce a false "clean". Both invalidation axes were checked by mutation rather than assumed -- unformatting a file with a warm cache still reports it, and narrowing `printWidth` to 40 reports 366 files, identical to an uncached run. (The first attempt at that second check appended a duplicate `printWidth` key, which YAML ignored, and briefly looked like a cache bug; the real mutation edits the existing line.) Also replaces `egrep` with `grep -E`, which drops an "egrep is obsolescent" warning that printed on every hook run.
`var_phase_symbolic_fragment_prod` is the engine's own per-variable lowering plus codegen -- the same work `compile_var_fragment` does, under the no-module-input wiring -- and it was a plain function called once per recurrence-SCC member per phase by the cycle gate's element-order probe. Instrumented on C-LEARN, the probe called it **135 times per cold compile for 57 distinct `(variable, phase)` keys**. The 2.4x duplication is structural, not incidental: `refine_scc_to_element_verdict`'s dt arm verifies the init element graph as a precondition, and `resolve_recurrence_sccs` is then run again for the init phase, so every init-phase member fragment is built twice. The probe is ~16% of a cold C-LEARN compile, and all of it recurred on every recompile of an unchanged model. The body is now a `#[salsa::tracked]` query keyed on `(model, project, var_name, phase)` -- the arguments it already varied over -- behind an unchanged wrapper that clones the memo out, so every call site keeps the ownership it had. `SccPhase` gains `Hash` to serve as a key. The `#[cfg(test)]` `UnsourceableVarsGuard` short-circuit stays OUTSIDE the memo, which is the one thing that could have gone quietly wrong here. Inside the tracked body its verdict would be cached against a key the guard is not part of, so a guard toggled between two calls on one `db` would be ignored by the second -- and in the direction that makes the AC3.2 loud-safe regression test (`unsourceable_in_scc_node_falls_back_to_circular_no_panic`) pass for the wrong reason. Short-circuiting in the wrapper keeps the override exactly as immediate as it was. Measured on C-LEARN, interleaved A/B/A over four rounds at load average 6.9-12.2 (so retired instructions, not cycles: 18.769G -> 16.293G over nine compiles = **-275.1M instructions per compile, -14.3%**). The repeated baseline arm spread 0.21% across all eight of its runs, against an effect ~60x that. Wall clock is not resolvable at this load and is not quoted. The compiled artifact is unchanged: 5215 slots, 58291 opcodes (31525 flow + 1477 stock + 25289 initial), same literal, GF, temp, dimension, view, name and module counts.
For a per-element (`Ast::Arrayed`) link-score target, one arm is shaped per target element whether or not that element's equation reads the link's source. An arm with no live source reference has every occurrence frozen by the ceteris-paribus wrap, so it recomputes the value that produced `PREVIOUS(target)` and its guarded numerator is identically zero -- yet it was still printed, parsed, lowered and executed every timestep. On C-LEARN those arms are 7.63 MB of 9.63 MB of generated arm text (GH #977). `partial_is_provably_previous_target` decides when such an arm may be dropped, and `build_arrayed_link_score_equation` drops it by leaving the slot ABSENT from the element map; `compiler::expand_arrayed_with_hoisting` already lowers an absent slot to a single `AssignCurr(off, Const(0.0))`. Absence is deliberately a different channel from an arm whose generated text is empty, so a generator bug stays distinguishable from an intended zero slot. Omission is gated on `apply_default_to_missing == false`: under EXCEPT semantics a missing slot takes the DEFAULT equation rather than zero. The predicate is POSITIVE -- outside every `PREVIOUS`/`INIT` subtree the arm holds only literals, operators, keywords and pure builtins. The obvious negative criterion, "the wrap froze every occurrence of the link's source" (`WrapOutcome::live_ref == None`), is UNSOUND, and that is the most important thing here for a reviewer, because the cheap version looks obviously correct: it says nothing about what else the arm reads. #977 measured it as changing 187 result slots across 35 link-score variables on C-LEARN, 151 of them by >= 1.0, worst case 8,086.97 -> 0. The wrap does not freeze everything that varies -- a live `time()` survives it, and a raw-vs-canonical element-spelling mismatch can leave the source itself unwrapped -- so the negative criterion is also unstable under fixes to either. The positive test asks about the emitted tree instead of the wrap's bookkeeping, so it stays correct whether or not those are fixed. The pure-builtin allowlist is deliberately broader than the minimum this corpus needs, and what admits a builtin is a PROPERTY rather than membership in a list: a builtin may join iff it is deterministic in its arguments and reads neither the clock, nor state, nor a table. Extending it is then a rule to apply rather than a taste call. `lookup` stays out even though a graphical function is a compile-time constant, because #977 measured that relaxation as buying exactly zero additional arms. The structure follows #977's standing constraints: every match arm returns a named `Reach` verdict, the builtin-argument walk is a verdict-returning fold so `=> {}` is a type error, and the matches carry no catch-all so a new `Expr0` variant is a compile error rather than a silent `Established`. The predicate lives in a `#[path]`-mounted sibling, `ltm_augment_zero_slot.rs`, following the six siblings already split off `ltm_augment.rs` for the per-file line cap -- adding it inline put that file at 6,059 lines against a 6,000-line threshold. Splitting it out also repaired a doc-comment detachment the inline version introduced: `shaped_guard_form_text`'s rustdoc, which describes its three freeze conventions and its `gf_table_ref` parameter, had been left documenting the interposed enum instead of the function. Measured on C-LEARN v77 with LTM in discovery mode: flow opcodes 1,208,106 -> 976,581 (-19.2%) total opcodes 1,238,728 -> 1,007,203 (-18.7%) The control that shows the predicate does not fire where it must not: WORLD3-03 with LTM is UNCHANGED -- 23,165 flow opcodes both ways, with the full opcode histogram, all 23 fused-binop counts and the 1,202-slot result geometry identical. `clearn_ltm_var_count_guardrail` stays green, so the C-LEARN variable count and slot width are unmoved as well. Value-neutrality was measured over the WHOLE result slab rather than over base slots, since comparing only `offsets`' base slots is blind to exactly the per-element arms at issue: 251 steps x 30,123 slots = 7,560,873 slot-steps, of which 51,358 differ (0.68%) in exactly one bit-pattern pair, `-0.0 -> +0.0`. Numerically zero differ. That probe is not checked in, and a value-level LTM gate at any scale is still missing -- see the note on goldens below. The one bit-pattern pair has a disclosed cosmetic surface. Rust's `Display` preserves the sign of zero (`format!("{}", -0.0f64)` is `-0`, verified), and `simlin-cli` prints result values with `{}` at src/simlin-cli/src/main.rs:337, :398 and :407 -- so a column that printed `-0` now prints `0`. Nothing in production branches on the sign of zero: `vm.rs:4160` pins `eval_op2(Eq, 0.0, -0.0) == 1.0`, `vm.rs:4129` pins `!is_truthy(-0.0)`, and `float.rs:201` pins `approx_eq(0.0, -0.0)`. Peak live bytes during `compile_project_incremental`, via the counting allocator in `examples/clearn_profile` -- stable to +/-0.2 MiB across runs, where peak RSS on this box varies by 10% and cannot carry a percentage: 5a82634 (merge base) 438.8 MiB + 4c68ef3 + 9a9934e 440.7 MiB (+1.9 MiB: the memoization and the shared equation ASTs cost essentially nothing in memory) + this commit 353.2 MiB (-19.9% here, -19.5% cumulative) Allocation count across the same three points: 50.80M -> 49.55M -> 41.37M. Four pre-existing tests asserted on arms that are now correctly absent. Each was repointed at the property it guards rather than at materialization: * `test_arrayed_link_score_stock_to_flow_per_element_partials` (ltm-503-cross-element-agg.AC1.3) guards a generator giving up and emitting a literal "0". Its assertion could no longer tell that from a deliberate omission, so it now derives the present-slot set, asserts it is exactly the live-source arms, and requires every present arm to be non-empty and free of the `((0) - ` give-up form. * `a_colliding_index_name_is_resolved_against_the_axis_it_indexes` and `an_index_naming_the_axis_own_element_stays_a_static_selector` (GH #986). Repointing these onto slot absence would have silently gutted them: under the #986 defect the index is rewritten to a static element selector, which the wrap freezes as an other-dep, so the arm is provably `PREVIOUS(target)` and is omitted just the same. The control test demonstrates that directly -- its correctly-qualified `q[slot.s1]` arm was omitted too. After the change no emitted arm on that fixture carried the index at all. Both boston equations therefore gain `+ 0 * pop[nyc]`, the idiom the neighbouring `0 * ctr` already uses: it gives the arm a live reference to the link's source, which is exactly the materialization condition, at zero numeric weight. The documented double-lag residual series reproduces bit-identically, so every original assertion -- `PREVIOUS(ctr, ctr)`, the `bucket.ctr` negative, and the two-variant series equality -- stands unchanged. * `test_disjoint_dim_arrayed_target_per_source_element_link_scores` described its `[a,y]` slot as "the trivial-zero guard form", which is the omitted class exactly. Its runtime claim is untouched, so the structural half now asserts absence and the VM assertion is TIGHTENED from `abs < 1e-6` to exactly zero. `[b,y]` on the same variable must stay materialized, so "every slot but `[a,x]` vanished" cannot pass. The characterization goldens do not move, and that is a coverage hole rather than good news: of the 16 fixtures exactly one carries an `Ast::Arrayed` link score and it is an EXCEPT-default target, hence pinned to `Materialize`, so the omission had no characterization coverage at all. A new `char_arrayed_target_no_default_slot_scores` fixture covers it at `apply_default=false`. That is structural; the value-level gate #977 asks for does not exist yet.
`Opcode::LoadPrev` pops its PREVIOUS() fallback off the arithmetic stack, so
codegen emits a `LoadConstant` immediately before every one: on C-LEARN 171,120
of 177,106 LoadPrev sites (96.6%) are preceded by a literal-0 load that exists
only to be popped. `LoadPrevConst { off, lit }` reads the fallback from the
literal table instead, folding the pair 2->1. Neither existing window can reach
it -- the 3- and 2-window combiners are `Op2`/`BinOpAssign`, and `LoadPrev` is
neither a combiner nor a leaf load, since it pops.
`ApplyTerConst { func, lit }` does the same for a 3-arity builtin whose trailing
argument is a literal. This is NOT the operand padding removed in 2d34ba4: that
commit established the arity is a property of the builtin and stopped emitting
pads, but a 3-arity builtin's third operand is a real value the builtin reads --
for SAFEDIV it is the divide-by-zero result, which the LTM guard form supplies
as a literal at 21,040 sites on C-LEARN. That load survives the arity fix and is
still worth folding. The `arity() == 3` guard is load-bearing rather than a
narrowing: for a 1- or 2-arity builtin the preceding `LoadConstant` is one of the
operands the builtin actually reads, so folding it as a "third operand" would
consume a real argument and leave the stack short. A test pins that.
Both are late-fusion forms, created only by `ByteCode::fuse_three_address` on the
Vm's private execution copy. They never enter the symbolic layer and never reach
wasmgen, which lowers the pre-fusion bytecode. Both absorbed instructions are
guarded against jump targets like every other fusion in the pass, and both fused
forms LOWER the peak stack depth (an operand that no longer transits the stack),
which is the safe direction for `resolve_bytecode`'s fixed-stack proof -- that
proof is computed on the pre-fusion stream, so fusion must never raise the peak.
Measured against e74d4d6, retired instructions per run and post-fusion flow
opcodes (instructions have an across-build sd of 0.026%, so one build pair
resolves an effect this size):
C-LEARN +LTM 920,966 -> 728,352 opcodes -14.15%
WORLD3 +LTM 17,415 -> 14,041 opcodes -16.29%
C-LEARN -LTM -0.41%
Non-LTM barely moves because that program has few PREVIOUS sites; this is an
LTM-shaped win.
Behaviour-preserving: 5487 engine lib tests and 632 integration tests pass,
including `clearn_residual_exactness`, `oracle_clearn`, and `vdf_parity`.
`topo_sort_str` and its `build_scc_grouping` helper kept four probe-only collections on `std`'s default SipHash: the allowed-name set, the visited set, and the two resolved-SCC lookup maps. The sort runs once per phase per model per module-input set and probes them once per dependency edge, which on C-LEARN is 136,116 hashes per compile -- the largest single SipHash site left in the compile path. All four are probed by key and never iterated for output (`scc_members`' values are pre-sorted `Vec`s, and `root_shifted`-style map-to-map copies do not depend on order), so the hasher is invisible to the result. The runlists this produces are byte-stable for the reason they already were: the visit order is a pre-sorted `names` list and each dependency set is a `BTreeSet`. FxHash's fixed seed additionally makes these maps' iteration order reproducible across processes, which is the direction GH #595 wants; the `IdentMap` alias's rustdoc carries the constraint this obeys -- the keys are variable names out of a model file, supplied by the party paying for the compile. Measured on C-LEARN (retired instructions, interleaved A/B, three rounds at load average 3.7-10.3): 16.283G -> 15.980G over nine compiles = -33.7M instructions per compile, -1.8%. The compiled artifact is unchanged.
The LTM link-score guard computes `v - PREVIOUS(v)` four times per link -- twice
for the target and twice for the source -- and on C-LEARN its 80,952 evaluations
cover only 4,166 distinct deltas. Each is four dispatches (`LoadVar;
LoadConstant; LoadPrev; Op2 Sub`), which no existing window matches: their
combiner is an `Op2`/`BinOpAssign` over two leaf LOADS, and `LoadPrev` is not a
leaf load -- it pops.
`SubVarPrev { l, r, lit }` folds that 4-window to one dispatch, and
`BinStackPrev { r, lit, op }` folds the 3-window `LoadConstant; LoadPrev; Op2`
where the lhs is already on the stack. `SubVarPrev` keeps its operator in the
variant tag (only Sub occurs) so the payload stays 3xu16 = 6 bytes and
`size_of::<Opcode>()` stays at 8 -- the same trick `AssignSubVarVarCurr` uses.
A test pins that any other operator falls through rather than being silently
encoded as a subtraction.
Both match the ORIGINAL windows rather than a `LoadPrevConst`-rewritten stream:
`fuse_three_address` is a single greedy left-to-right pass, so when the window at
i is tested, position i+1 has not been rewritten. They are therefore independent
of `LoadPrevConst` and compose with it in either order -- the longer windows
claim the sites they cover, and `LoadPrevConst` mops up the rest.
Score a helper-variable proposal against the POST-fusion stream. Hoisting a
repeated subexpression into a shared aux replaces each use with a `LoadVar` --
one dispatch, exactly what a fused opcode costs -- so the hoist is worth zero
wherever a superinstruction can match the pattern, while still paying for a store
and a slot. Scored against the pre-fusion stream the same hoist looks like a
3-to-1 win. Measured here: hoisting the delta is worth nothing next to
`SubVarPrev`; hoisting `ABS`/`SIGN` of it is a net LOSS on WORLD3 (375 uses over
163 distinct); and hoisting `TIME = INITIAL_TIME` is worth nothing at all,
because the pass already folds it to one `BinGlobalGlobal`.
Measured on top of the previous commit, retired instructions per run and
post-fusion flow opcodes:
C-LEARN +LTM 728,352 -> 513,054 opcodes -16.32%
WORLD3 +LTM 14,041 -> 9,896 opcodes -20.74%
Cumulative over both commits, against e74d4d6: C-LEARN +LTM 920,966 -> 513,054
opcodes and -28.3% retired instructions; WORLD3 +LTM 17,415 -> 9,896 and -33.6%.
Behaviour-preserving: the engine lib and integration suites pass, including
`clearn_residual_exactness`, `oracle_clearn`, and `vdf_parity`.
`canonicalize`'s fused fast-path scan decodes every non-ASCII byte and asks `changes_when_lowercased` whether lowercasing would change it. The engine writes a non-ASCII character into identifiers itself -- the module-hierarchy separator `·` is in every `submodel·var` ident, and LTM's synthetic names carry `⁚` and `→` -- so that question was reaching the Unicode case tables 342,138 times per C-LEARN compile to re-derive that a middle dot is not an uppercase letter. Short-circuiting those three characters is sound only because the case tables agree, so `engine_separators_are_lowercase_invariant` asks the tables rather than restating the answer: it checks `char::to_lowercase` yields exactly the same single character for each, and reds if a separator is ever added to the list that lowercasing does change. Any character not listed still takes the general path, so this narrows the work without narrowing the domain. Measured on C-LEARN (retired instructions, interleaved A/B, three rounds at load average 5.6-9.8): 15.994G -> 15.590G over nine compiles = -44.9M instructions per compile, -2.4%. The compiled artifact is unchanged.
Three things this file could not tell a reader before. The compile-side proposals C1/C2/C3 were written against a pre-salsa profile and each is now answered, two of them differently than proposed: C2 is moot (`reconstruct_variable` is salsa-cached and off the ordinary compile path entirely), and C3's interning half is already done while its ASCII-fast-path half reaches only the 4.6% of `canonicalize` calls that allocate. Left as written, they would send the next reader at a correctness-critical function guarded by the GH #559 idempotence proptests to chase a twentieth of the cost that call elimination reached without touching it. C4 records the parallel fan-out: designed, prototyped and measured (2.23x achieved parallelism but only 1.34x wall, against an Amdahl ceiling of 1.44x over the then-serial 68%), and deliberately not implemented. It carries the structural constraint that decides its shape -- `salsa::Database` is `Send` but not `Sync`, so the fan-out cannot live inside the query graph at all -- and the two hazards that were found by running the suite rather than by thinking: a prewarm placed ahead of the module-cycle gate reopens GH #806's process abort, and an ungated prewarm regresses the fully-cached recompile 2.5-4x. Both are silent. Determinism, the hazard that was expected, is recorded as measured-absent so nobody spends the round re-establishing it. C5 records why the top allocation site was left alone: `NameId` assignment order is part of the compiled artifact. The round-3 section states its two findings as standing constraints rather than as a list of fixes -- a per-variable helper needs a key of its own, and a projection is what keeps a per-variable query per-variable -- because both are cheap to violate and neither is visible in a diff that violates them.
Nothing asserted what an LTM link score is WORTH. `clearn_residual_exactness` never enables LTM; `clearn_ltm_var_count_guardrail` pins the emitted variable count and the slot width, and neither moves when an arm's value is rewritten. The characterization goldens are text, so they catch an arm whose spelling changes and say nothing about an arm whose spelling is right and whose value is not. A change that rewrote 149 C-LEARN LTM slots to zero passed every named C-LEARN gate (GH #977). Two halves, split by runtime rather than by coverage. `db::ltm_value_gate_tests` is the sub-second default-suite half: a three-arm `Ast::Arrayed` target with no EXCEPT default, so `OmitStructuralZero` is live and each arm's fate is decided independently, and each arm is one of the ways an arm-level change goes wrong. nyc pop[nyc] * 0.01 + TIME * 0.002 live TIME, no live source for the other links -- must NOT be omitted boston alt[a1] * 0.02 source reached through a bare element subscript of a DISJOINT dimension -- must NOT be omitted la base * 0.03 genuinely source-free and invariant -- must be EXACTLY +0.0 `alt` is wired back through `pop_total = SUM(pop[*])` so the edge sits on a real loop; the first draft omitted that and emitted no `alt[a1]→growth` score at all, which would have failed on a missing variable rather than on a wrong value. The whole LTM slab is pinned as a golden, but a golden alone would not do this job: per the root CLAUDE.md, a golden that pins an artifact is blind to that artifact being stably absent, and a careless `UPDATE_LTM_VALUE_GOLDEN=1` re-capture would bless a zeroed slot. So each mechanism also carries a named assertion that does not read the golden. The structural-zero row asserts EXACT equality rather than a tolerance, because a near-zero residual passing a tolerance is precisely the signal that the arm was not provably `PREVIOUS(target)`. The `boston` row's rustdoc is explicit about what it does NOT establish: it pins the access SHAPE in which #977's 322 unwrapped-bare-variable arms arise, not the raw-vs-canonical mismatch itself. `alt[a1]` is that link's own source, so the occurrence match and the emitted tree agree about it. No fixture reproduces the mismatch yet, and claiming one would be worse than having none. `simulate_ltm::clearn_ltm_slot_maxima_digest` is the C-LEARN half, `#[ignore]`d purely for runtime (~3.5 s release, but it needs a release build; the debug build is far past the 3-minute cap in docs/dev/rust.md). It is a digest rather than a slab because 30k slots x 251 steps is 60 MB of golden nobody would read: `nonzero_slots` (which a silent zeroing moves DOWN), `finite_slots` (so a regression to NaN cannot hide behind an unchanged non-zero count), and an order-independent sum of per-slot maximum magnitudes quantized to nine significant digits -- fine enough to catch any real zeroing, coarse enough not to red on last-bit drift, which is what turns a pin into something people re-capture without reading. Its teeth were measured, not assumed. Same binary, three runs differing only in `ltm_augment_zero_slot`: predicate as shipped (1369, 7000, 10_248_673_492_482_445_132_733_301) omission disabled (Materialize) identical in all three numbers predicate forced true (1287, 7000, 10_248_673_492_258_319_975_585_940) The second run is the point: this digest is the reproducible, checked-in form of the whole-slab differential that established 0fa2621's value-neutrality on C-LEARN, which until now existed only as a throwaway probe nobody could re-run. The third run is what makes the second meaningful -- 82 slots carrying real scores go to zero and the digest reds, so "unchanged when the omission is disabled" is not merely a digest that cannot see the omission.
On an engine change that alters emitted code, `src/engine/build.sh` takes
188.7s, and `wasm-opt -O3` is ~170s of it (90.6s + 76.2s for the two blobs).
The hook needs an artifact that builds and passes the TypeScript tests, not
a size-optimized one: the engine suite runs 1.26s/1.29s against the
unoptimized blob and 1.40s/1.42s against the optimized one -- no measurable
cost. With this, a whole-workspace `pnpm build` after an engine change is
11.1s.
This mirrors `.github/workflows/ci.yaml`, which already sets
DISABLE_WASM_OPT=1 in both of its build steps, one of them with the same
reasoning written out ("release-quality WASM is irrelevant for a smoke
test").
The variable belongs on THIS call site and not in `src/engine/build.sh` or
`package.json`, which is the obvious way to do it and is wrong. Six callers
run `pnpm build`: the two CI steps (already opted out), this hook, and
`scripts/deploy-web.sh`, `scripts/deploy-web-staged.sh` and the release
workflow -- which must keep the optimized artifact, because the browser
bundle is download-size-dominated, the same reason `.cargo/config.toml`
forces opt-level=z on wasm32. Flipping the default would route through all
of them and ship a 24% larger bundle (5.00MB -> 6.20MB) to every user.
What this gives up, stated plainly (GH #1019): `build.sh` runs wasm-opt IN
PLACE over `core/<name>.wasm`, the file the TypeScript tests load, so a
developer's machine was until now the only place the optimized bundle was
ever executed under test -- neither automated lane runs it, and only
`ts-release.yml` installs binaryen at all. After this, nothing does until an
npm publish. That coverage was accidental (it depended on every developer
running the hook) and sat in the wrong place, but it was real; #1019 tracks
converting it into a deliberate CI lane that builds with wasm-opt and runs
the engine suite against the optimized blob.
Verified by running the exact edited command after a codegen-altering change
(a new `#[no_mangle]` export in libsimlin): both blobs report "Skipping
wasm-opt". That indirection matters here -- an earlier probe that appended
an unused `pub const` recompiled Rust but produced a byte-identical wasm, so
build.sh's `cmp` guard skipped wasm-opt on its own and measured a build that
never did the expensive part. The hook this commit edits is also not the one
that runs on this commit: git resolves it through the main checkout's copy.
`compiler::symbolic::resolve_bytecode` proves the compiled stream fits `STACK_CAPACITY`, and `vm::Stack` uses unchecked access on the strength of that proof. But the proof is computed on the PRE-fusion stream while the Vm executes the fused one, so `ByteCode::fuse_three_address` carries a standing obligation: a fused opcode's `stack_effect` must account for every operand the sequence it replaces consumed, and a program's peak depth may fall but never rise. Nothing already in the suite covers that. The deepest stack any corpus model reaches is 8-12 against a `STACK_CAPACITY` of 64, so a wrong stack effect has more than 5x of headroom to hide in: it would not overflow, the arithmetic would still be correct, and every saved value would match. A passing suite and matching results fingerprints are strong evidence for other failure modes and weak evidence for this one -- comparing the two depths is what detects it. The `Err` arm of `max_stack_depth` covers the other half: an underflow means an opcode's declared effect is wrong rather than the program. `CompiledSimulation::fusion_depth_audit` reports both depths per (module, phase) alongside the opcode counts; it lives in `vm_profile.rs`, the diagnostics-only sibling that already exposes bytecode shape without leaking the private `Opcode` type. The test sweeps the curated corpus (`TEST_MODELS`), both executed phases, every module -- initials are excluded because `Vm::new` leaves them unfused. It covers the shapes the corpus curates rather than every file on disk, which keeps its cost proportional to a list someone maintains deliberately: 0.20s on a debug build, against the 2s per-test target in docs/dev/rust.md.
`src/engine/build.sh` runs `wasm-opt -O3` IN PLACE over `core/*.wasm`, the file the TypeScript tests load, so "did wasm-opt run" and "was the optimized bundle executed under test" are one question -- and until now nothing automated answered yes. Both of ci.yaml's build steps set DISABLE_WASM_OPT=1, `scripts/pre-commit` now does too, and `ts-release.yml` is the only workflow that installs binaryen at all, by which point the artifact is being published. The coverage that existed was a side effect of developers running the hook on machines that happened to have binaryen. This makes it deliberate: a path-filtered job on the sources that can change emitted WASM, which builds with wasm-opt on and runs the TypeScript suite against the result. It closes GH #1019 and is what makes the hook's DISABLE_WASM_OPT change a coverage improvement rather than a trade -- before it, nothing automated ran the pass before an npm publish; after it, an automated gate runs it on exactly the PRs that can break it. Its own workflow file rather than a job in ci.yaml because GitHub applies `paths` per workflow, not per job; filtering inside ci.yaml would mean taking a third-party paths-filter action. Two things the job does beyond the obvious, both because a silent pass here would be worse than no job at all: `build.sh` SKIPS wasm-opt and exits 0 when binaryen is missing, so a broken install would quietly turn this into a slower duplicate of ci.yaml's frontend job. The "Assert the blobs are actually optimized" step compares each blob against the `.raw` copy build.sh stages and fails if they are byte-identical. Checked by mutation in all three states: optimized (passes, reporting both sizes), blob copied from .raw (fails naming wasm-opt), .raw missing (fails naming build.sh). The header says what a failure MEANS. The same TypeScript tests run in ci.yaml against an unoptimized blob, so green there and red here isolates the difference to binaryen's -O3 pass -- a miscompilation, an unsupported feature, or a version incompatibility -- not a defect in the TypeScript under test, with the two commands to reproduce the pair locally. Also notes that it must not be made a required status check as-is: a path-filtered workflow reports nothing on a PR that touches none of its paths, and a required check that never reports blocks the PR forever. Cost: ~170s of wasm-opt (90s + 76s across the two blobs) plus the wasm cargo build, on engine PRs only, in parallel with jobs already minutes long. Fixes #1019 --- Also corrects a stale number in scripts/verify-deploy-build.sh, which said DISABLE_WASM_OPT "bumps it to ~12MB". The unoptimized opt-level=z blob is 7.9MB; 12.7MB is what opt-level=1 produces, which is not a configuration anything uses. Restates what that check does and does not gate, so nobody mistakes it for a wasm-opt gate -- it deliberately passes either way.
`clearn_ltm_slot_maxima_digest`'s rustdoc promised a tolerance of nine SIGNIFICANT digits -- a relative one, chosen so the pin would not red on the last-bit drift a benign allocator, layout or FP-association change produces, because "a digest that reds on that is a digest people learn to re-capture without reading". The code scaled by a fixed `1e9` and rounded, which quantizes ABSOLUTELY, at 1e-9 in value units. On this model those are not close. Measured over the 1,369 non-zero LTM slots: the largest peaks at 1.53e15, 30 slots sit above 1e12, and one ULP of the top slot is 0.25 -- which the `* 1e9` scale turns into a digest movement of 2.5e8. The pin was therefore maximally sensitive to exactly the noise its own rustdoc said it tolerated, and would have trained the re-capture reflex it was written to prevent. The same scale had a sensitivity hole at the other end, which is why this is a rewrite rather than a rescale. Summing raw magnitudes lets the three 1e15 slots dominate an aggregate that 743 slots near 1.0 also contribute to, and one slot whose maximum is 1e-14 quantized to exactly zero -- it could not move the digest at all, at any value below 5e-10. `nine_significant_digits` splits each slot's maximum into a 9-significant-digit mantissa in `[1e8, 1e9)` and a decimal exponent, summed separately. Every non-zero slot now contributes comparably regardless of scale, so a small slot is as visible as a large one; the exponent sum catches order-of-magnitude moves the mantissa alone would miss; and last-bit drift changes neither. It also removes the overflow hazard by construction rather than by clamping. The old form fed a saturating `f64 -> i128` cast, silent above ~1.7e29; the sums here are bounded by 7e12 and ~2.2e6 at 7,000 slots, both far inside `i64`. The new pins are `(1369, 7000, 371_710_864_477, 1019)`. The exponent digest was cross-checked against a decade histogram taken by a separate probe -- 3 slots at 1e15, 20 at 1e14, ..., 743 at 1e0, 144 at 1e-1, ..., 1 at 1e-14 -- which sums to exactly 1019 by hand, so the implementation agrees with an independent derivation rather than only with itself. The gate still catches what it exists for, and more loudly. Forcing `partial_is_provably_previous_target` to `true`, which drops 82 arms carrying real scores, moves it to `(1287, 7000, 350_469_799_138, 979)` -- a 5.7% shift in the mantissa digest where the old absolute form moved by 2.2e-11 of its own magnitude. The four lag-alignment coverage rows are unaffected and their per-clause mutation table is unchanged: reverting the original-`PREVIOUS` check reds `an_original_previous_arm_is_not_a_structural_zero` alone, and reverting the nested descent reds `a_nested_freeze_arm_is_not_a_structural_zero` and `pinned_double_lag_residual_is_not_a_structural_zero`.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1021 +/- ##
==========================================
+ Coverage 92.21% 92.24% +0.02%
==========================================
Files 247 248 +1
Lines 158247 159177 +930
==========================================
+ Hits 145935 146825 +890
- Misses 12312 12352 +40 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d97ecb6cc2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # on) installs mimalloc as its global allocator. The profiling examples and | ||
| # criterion benches therefore back their allocator with mimalloc too: the | ||
| # compile path is allocation-bound, so a harness on system malloc measures an | ||
| # allocator no shipped native build actually runs, and over-credits any change |
There was a problem hiding this comment.
When performance experiments use backend_bench, ltm_mem_bench, or the Criterion benches, adding mimalloc as a dev-dependency does not select it as the final binary's allocator: examples/backend_bench.rs:43 and examples/ltm_mem_bench.rs:35 explicitly retain System, while the benches declare no #[global_allocator]. Those compile-path measurements therefore still use the allocator this block says is unrepresentative and can over-credit changes that merely shift allocation traffic. Either install mimalloc in these harnesses or narrow the comment to clearn_profile, which is the only harness changed accordingly.
AGENTS.md reference: AGENTS.md:L118-L120
Useful? React with 👍 / 👎.
| # Resolved rather than assumed -- see scripts/cargo-target-dir.sh. A stale path | ||
| # here is quieter than the wasm one: the staleness check below simply never | ||
| # fires, so the CFFI extension is silently not rebuilt against a changed | ||
| # library. | ||
| LIBSIMLIN_A="$CARGO_TARGET_DIR_RESOLVED/release/libsimlin.a" |
There was a problem hiding this comment.
When CARGO_TARGET_DIR moves the workspace target directory, this now checks freshness against the correctly resolved archive but still invokes setup.py without SIMLIN_STATIC_LIB. simlin/_ffi_build.py::_get_library_path does not inspect CARGO_TARGET_DIR; it searches only the repository and crate-local target/ directories unless that environment variable is supplied. Consequently the rebuild either links an older default-target archive—allowing the Python suite to validate stale engine code—or fails to find the new archive, with that build failure suppressed by || true. Export SIMLIN_STATIC_LIB="$LIBSIMLIN_A" for the CFFI build so the tested extension uses the artifact built on line 14.
AGENTS.md reference: AGENTS.md:L100-L100
Useful? React with 👍 / 👎.
| - 'Cargo.lock' | ||
| - 'Cargo.toml' | ||
| - '.cargo/config.toml' | ||
| - '.github/workflows/wasm-opt.yml' |
There was a problem hiding this comment.
When a PR changes rust-toolchain.toml, this path-filtered workflow does not run even though the selected Rust compiler directly changes the emitted WASM and can change binaryen compatibility. The ordinary frontend CI build deliberately sets DISABLE_WASM_OPT=1, so such a compiler upgrade can merge without executing either optimized blob; the first optimized validation may then occur only during a release. Add rust-toolchain.toml to both path lists so the advertised pre-release optimized-bundle check covers compiler changes.
Useful? React with 👍 / 👎.
| let _ = crate::db::compile_implicit_var_fragment( | ||
| db, | ||
| model, | ||
| project, | ||
| name.clone(), |
There was a problem hiding this comment.
After this change, compile_implicit_var_fragment is a #[salsa::tracked] query, but the comment immediately above this call still says it is untracked, cached only through its parent parse, and recompiles every helper on each revision's first diagnostic collection. That is now the opposite of the performance behavior this query conversion is intended to establish, so anyone investigating warm-edit latency or accumulator replay will be directed toward a nonexistent 15 ms recompilation cost. Rewrite the block to describe per-helper memo reuse and the conditions that actually invalidate it.
AGENTS.md reference: AGENTS.md:L119-L119
Useful? React with 👍 / 👎.
…rchive Four corrections from review, each a case of an assertion the code did not implement. A dev-dependency does not select an allocator. The Cargo.toml comment claimed the profiling examples and criterion benches all ran on mimalloc; only clearn_profile did. backend_bench and ltm_mem_bench kept System as their counting allocator's backing and the four benches declared none at all, so the compile-path timings they report came from the allocator the comment calls unrepresentative -- over-crediting exactly the changes it warns about. All seven harnesses now install it, and the comment says a new harness has to do the same. pysimlin-tests.sh resolved the static library for its freshness check but did not pass it to the CFFI build, and _ffi_build.py::_get_library_path searches only the workspace and crate-local target directories unless SIMLIN_STATIC_LIB pins it. Under CARGO_TARGET_DIR that links a stale archive or fails -- the silent-stale-engine case GH #682 added the pin for. The build's failures are no longer discarded either: an unbuilt extension surfaced later as an import error that said nothing about the cause. The optimized-WASM lane did not run on rust-toolchain.toml. The compiler selects what WASM is emitted and whether binaryen can still read it, and the ordinary frontend lane runs with DISABLE_WASM_OPT=1, so a toolchain bump could reach a release without either blob having been optimized once. The diagnostic probe's comment still described compile_implicit_var_fragment as untracked and quoted a ~15ms per-revision helper recompile. It is a tracked query now, so the walk costs a memo lookup per helper; the comment pointed anyone investigating warm-edit latency at a cost that no longer exists. Also silences a release-only unused-variable warning: compile_ltm_fragment_for reads its expected argument solely from a debug assertion.
|
All four verified against the code and fixed in Harness allocators. Correct, and the sharper version is that a dev-dependency does not select an allocator at all. Only
The diagnostic probe's comment. Correct. It still described One thing found while fixing: Full pre-commit gate green (dependency policy, doc links, project lint, Rust fmt/clippy/test, TypeScript + WASM build, pysimlin). @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2afe6265fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| BuiltinReach::LagsOneStep => { | ||
| if args.iter().any(contains_previous_call) { | ||
| Reach::NotEstablished | ||
| } else { | ||
| Reach::Established |
There was a problem hiding this comment.
When a frozen target slot evaluates to NaN or infinity, reaching a PREVIOUS(...) node is not enough to prove that the materialized link score is zero: although the partial and PREVIOUS(target) have the same non-finite value, their subtraction produces NaN, the zero guards do not fire, and the original arm propagates NaN. Returning Established here lets shaped_guard_form_text omit that arm and replace the result with literal 0.0, silently turning an invalid/non-finite LTM attribution into a structural zero. Keep such arms materialized unless finiteness is guaranteed at runtime.
Useful? React with 👍 / 👎.
| // guarantee moved from the data into the `apply_*` tests in | ||
| // `lower_tests.rs`, which execute every builtin against the VM. They are | ||
| // the enforcement -- extend them when adding one. | ||
| let arity = func.arity(); |
There was a problem hiding this comment.
The Opcode::Apply dispatch comment above this function still says every call pops three padded operands and that codegen supplies padding, while this change makes emit_apply pop BuiltinId::arity() and removes those pads. That is the opposite of the new stack contract and can lead a maintainer adding or debugging a builtin to reason that unread locals are freshly initialized when they are actually stale. Update that dispatch comment to describe the shared arity table and variable operand count.
AGENTS.md reference: AGENTS.md:L119-L119
Useful? React with 👍 / 👎.
The GH #977 omission is bit-exact modulo sign-of-zero given lag alignment, with one exception that was not disclosed: when the target slot is NON-FINITE, it changes a value rather than a representation. A materialized arm over a `NaN` target computes `partial - PREVIOUS(target)` = `NaN - NaN`. The zero guards do not rescue it -- `NaN = 0` is false -- and `SAFEDIV`'s fallback fires on a zero denominator rather than a `NaN` one, so it returns `NaN / NaN`. The arm evaluates to `NaN`, where an omitted slot is `AssignCurr(off, Const(0.0))` and therefore `+0.0`. An infinite target collapses to the same case, since `inf - inf` is also `NaN`. Reproduced both ways rather than argued, on a fixture whose target really does go non-finite (a flow-less stock holding 0, so `zed / zed` is a runtime 0/0 that constant folding cannot reach): materialized gives `[0, NaN, NaN, NaN, NaN, NaN]`, omitted gives all zeros. Preserving the arms is not available. Non-finiteness is a runtime property, so declining to omit any target that COULD go non-finite means declining to omit at all. A cheap runtime sentinel is not available either: the nearest candidate, `0 * (target - PREVIOUS(target)) * SIGN(source - PREVIOUS(source))`, takes its sign-of-zero from the wrong delta, yields `NaN` at `TIME = INITIAL_TIME` where a materialized arm yields exactly `0`, and costs four to five opcodes against the one an omitted slot lowers to. Building a partially-equivalent form against a mechanism just learned is how the 2026-07 attempt failed seven times. So this discloses and pins rather than preserving, and records that whether `0` is the better answer is open (GH #1022). The two relevant positions disagree, which is why it is not settled here. `src/float.rs` holds that an engine-manufactured NaN is noise in a channel practitioners already debug by hand, and this NaN is engine-made -- the guard form's own subtraction -- on an arm with no causal dependence on its source, so `0` is the structurally known answer. GH #542 points the other way: `ltm_post::denom_summand` excludes a `NaN` summand from its partition denominator specifically so the bad entry's own numerator can stay `NaN`, described there as "the honest per-loop 'undefined here' signal" -- a deliberate decision that NaN scores carry meaning per entry. Two earlier looks at this qualifier cited only `float.rs` and neither weighed #542, which is the reason it goes to an issue rather than into this change's judgement. #542 also disposes of the argument that a NaN score poisons its partition's relative-score denominator: it does not, and has not since #542. `a_nonfinite_target_arm_is_omitted_to_zero_not_nan` is the one row in the value gate that pins the omission CHANGING a value rather than preserving one, and its rustdoc says so, since it reads as an anomaly otherwise. It asserts the fixture premise -- the target really is `NaN` -- so it cannot pass on a model that never went non-finite, and it asserts the counterweight that a live arm over the same `NaN` target still scores `NaN`. That bounds what changed: the signal survives on the target's own series and on every arm with a live source; only arms with no causal dependence move. Mutation-tested -- disabling the omission reds it.
…the loro unmaintained cluster Two CI failures, one of them a latent break the new optimized-WASM lane caught on its first run. apt's binaryen is older than the flags src/engine/build.sh passes, so the optimized build dies on 'Unknown option --enable-bulk-memory-opt'. That recipe was copied from ts-release.yml, which publishes to npm and would have failed identically on the next release -- the lane exists to catch a binaryen incompatibility before a release, and the first thing it caught was its own install recipe. Both workflows now share scripts/install-binaryen.sh, which pins an upstream release, refuses to proceed if that version cannot accept the flags build.sh passes, and fails at install rather than after build.sh has staged an unoptimized blob. The three RUSTSEC advisories are unrelated to this branch and pre-date it: bitmaps, im and sized-chunks were all declared unmaintained together, all reached through one edge (simlin-serve -> loro -> loro-internal -> im), and each says no safe upgrade exists. im's own announcement points at the imbl fork, which is loro's migration to make. Ignored with the rationale and a revisit condition, matching the three entries already there for the same class; all three go together because bitmaps and sized-chunks are only present as im's dependencies. Also corrects the wasm Apply dispatch comment, which still described a fixed three-operand pop with codegen-supplied padding. Both are gone: the arity table is shared by codegen, the VM and wasmgen, and locals above the arity now hold whatever the previous Apply left there.
|
Both review comments addressed, and both CI failures fixed at the root, in P1 — non-finite arms: confirmed, disclosed and pinned; the decision is now #1022Correct, and reproduced rather than reasoned about. Fixture: a flow-less stock holding
Your mechanism is exact, including the part that makes the guards useless here: On the remedy, though — "keep such arms materialized unless finiteness is guaranteed at runtime" is not implementable. Non-finiteness is a runtime property, so declining to omit anything that could go non-finite means declining to omit at all, which is the entire optimization. A cheap runtime sentinel does not close it either: So: disclosed and pinned, and the semantics decision filed as #1022 rather than settled inside a performance PR. Worth surfacing, because it cuts against the direction your comment and a previous attempt both leaned: P3 — stale
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8b3af8765
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # DISABLE_WASM_OPT=1, so a toolchain bump would otherwise reach a release | ||
| # without either blob having been optimized once. | ||
| - 'rust-toolchain.toml' | ||
| - '.github/workflows/wasm-opt.yml' |
There was a problem hiding this comment.
When a PR changes scripts/install-binaryen.sh—for example, bumping the pinned version or altering an asset name—neither path list triggers this workflow, even though this job consumes that script and the only other consumer, ts-release.yml, runs only for tags or manual dispatch. A broken download or incompatible default can therefore merge without executing the installer and first surface during an npm release; add the script to both the push and pull-request path lists.
Useful? React with 👍 / 👎.
| // guarantee moved from the data into the `apply_*` tests in | ||
| // `lower_tests.rs`, which execute every builtin against the VM. They are | ||
| // the enforcement -- extend them when adding one. | ||
| let arity = func.arity(); |
There was a problem hiding this comment.
Although the dispatch comment was corrected, fresh evidence remains in EmitCtx::apply_locals at lines 157–158 and the emit_apply rustdoc at lines 2462–2464: both still claim that Apply always receives three padded operands. That directly contradicts this arity-based implementation, where unused locals remain stale, and preserves the same misleading maintenance contract the earlier review identified. Update those remaining comments to describe func.arity() and the partially initialized locals.
AGENTS.md reference: AGENTS.md:L119-L119
Useful? React with 👍 / 👎.
| cp "$WASM_SRC" "core/$out_name" | ||
| cp "$WASM_SRC" "core/$out_name.raw" |
There was a problem hiding this comment.
When an existing cache is stamped opt and a changed Rust build enters this block, these copies replace the staged artifact with raw WASM before optimization succeeds. If wasm-opt fails or the build is interrupted before line 85, the old opt stamp remains; on the next run have_mode == want_mode and the newly copied .raw matches WASM_SRC, so the cache skips optimization and silently treats the raw blob as optimized. Remove the old stamp before restaging, or stage the blob and stamp transactionally so a failed optimization cannot leave a valid-looking cache entry.
AGENTS.md reference: AGENTS.md:L119-L119
Useful? React with 👍 / 👎.
| .filter(|(name, _)| name.as_str().starts_with("$\u{205A}ltm\u{205A}")) | ||
| .map(|(_, &off)| off) | ||
| .collect(); |
There was a problem hiding this comment.
Results.offsets contains only one entry per LTM synthetic variable here: calc_flattened_offsets_incremental records (entry.offset, entry.size), but construction of CompiledSimulation discards the size. Consequently an arrayed link or loop score contributes only its first element to this supposed every-slot digest, so a regression affecting any later element leaves all four pinned values unchanged. Enumerate each LTM variable's production layout extent rather than treating every offset-map entry as a one-slot variable.
AGENTS.md reference: AGENTS.md:L98-L98
Useful? React with 👍 / 👎.
| mantissa_digest += mantissa; | ||
| exponent_digest += exponent; |
There was a problem hiding this comment.
Even for the offsets that are sampled, these unweighted sums are permutation-invariant: if two LTM slots exchange their maxima, nonzero_slots and finite_slots stay unchanged and the mantissa and exponent sums also remain exactly unchanged, contrary to the rustdoc's claim that such a swap moves the digest. An offset/remapping regression can therefore attach correct values to the wrong links while this gate remains green. Mix each stable slot identity into the digest, or compare a sorted keyed collection instead of summing an anonymous multiset.
AGENTS.md reference: AGENTS.md:L98-L98
Useful? React with 👍 / 👎.
…imed three Three comments still described `Apply` as always receiving three operands with codegen supplying padding. That contract ended when builtins gained a real arity; the prose preserved the maintenance hazard the implementation had removed, telling a maintainer that unread `apply_locals` are freshly zeroed when they now hold whatever the previous `Apply` left there. - `EmitCtx::apply_locals` said the opcode "always pops exactly three operands (codegen pads)". Three is the WIDEST a builtin needs, not the number every `Apply` populates, so the locals are partially initialized in general. - `emit_apply`'s rustdoc said "the three operands are on the wasm stack". It is `BuiltinId::arity()` of them, with no padding. - `Opcode::stack_effect` still carried "Builtins always take 3 args (actual + padding)" immediately above the line replacing it. That one is mine: the edit that introduced the arity comment left the sentence it contradicted in place, so the stale claim read first. Both wasmgen sites now say what a maintainer adding a builtin needs: the arity table is shared by codegen, the VM and this backend, so the operand count is decided in one place; and the padding was accidentally load-bearing, since it guaranteed unread locals were `0.0`. Removing it moved that guarantee out of the data and into the `apply_*` parity tests in `lower_tests.rs`, which are now the only thing stopping an arm from reading past its arity -- the same pointer the obligation note at the pops already carries. Found by sweeping the class rather than the named sites: the third is in `bytecode.rs`, which a search scoped to `wasmgen/` would not have reached. Not changed, having checked: `vector.rs`'s several claims that the `Apply` scratch f64s are "free" are about availability to clobber, not about their contents. Those emitters write each local before reading it, so they never depended on the padding's zeroes and are unaffected.
Writing the stamp last protects a FIRST build -- an abort leaves no stamp, so
the next run redoes the work. It does not protect an UPDATE, because a valid
stamp from the previous build is still on disk, and that is the case the
original reasoning missed.
Reproduced before fixing, on a real failing wasm-opt (a shim on PATH, so
`command -v` still succeeds and the invocation aborts under `set -e`):
1. good optimized build blob 6829333 raw 8327392 mode opt
2. Rust changes, wasm-opt dies after the copies
blob 8327384 raw 8327384 mode opt <-- stale
3. next run, real wasm-opt EARLY-OUT, no wasm-opt, exit 0
blob 8327384 raw 8327384 mode opt
Step 3 exits 0 with the raw cargo output staged and stamped `opt`. That is
worse than the bug the stamp was added for: it is a green build whose
artifact is wrong, and it also defeats verify-deploy-build.sh's
REQUIRE_WASM_OPT check, which reads the stamp. Post-fix the same sequence
leaves no stamp at step 2 and re-optimizes at step 3 (blob 6829390).
The fix is `rm -f core/$out.mode` before restaging, which makes the whole
window self-invalidating: no stamp means indeterminate, and indeterminate
means rebuild. Paired with the existing write-last, the stamp now exists only
while it is true of what is on disk.
The `.raw` window asked about in review is already safe, and the comment now
says why rather than leaving it to be re-derived: the blob is copied BEFORE
`.raw`, so a failure between the two leaves `.raw` holding the previous
output and the next `cmp` rebuilds. Reversed, it would leave a `.raw`
describing the new source beside a blob built from the old one, which `cmp`
cannot detect because it only ever compares `.raw` against cargo.
verify-deploy-build.sh's check no longer trusts the stamp alone. It now also
requires the blob to differ from its `.raw`, which is independent evidence
that wasm-opt transformed something -- the same thing the CI lane asserts. A
guard that can only be as correct as the thing it guards is not a guard, and
this one demonstrably was: the forged stale-stamp state passed it before and
fails it now. Mutation-tested both ways.
`scripts/install-binaryen.sh` was not in either path list, so a version bump,
a changed release-asset name or a dead URL did not run the one workflow that
executes it. Its only other consumer is ts-release.yml, which runs on `ts-v*`
tags and manual dispatch -- so a break would first surface DURING an npm
release. Same shape as the rust-toolchain.toml gap fixed earlier, and the
same fix.
I checked the lane's other inputs rather than assuming, and deliberately did
NOT add the rest. The test is whether a change to a file can break something
only THIS lane would catch:
- scripts/install-binaryen.sh yes -- nothing else on a PR runs it
- rust-toolchain.toml yes -- already listed, for the same reason
- src/engine/build.sh yes -- already covered by src/engine/**
- package.json, pnpm-lock.yaml, pnpm-workspace.yaml
no -- ci.yaml's frontend job has NO path
filter, so it runs `pnpm install`, `pnpm
build` and `pnpm test` on every PR and reds
first. Listing them here would fire a
~4-minute wasm lane on every dependency
bump for no signal this lane alone provides.
The push and pull_request lists are maintained by hand and read as one
filter, so a path added to only one silently means "runs on merge but not on
the PR" -- a gap that looks like coverage, and the same shape as the one this
commit closes. scripts/lint-project.sh (pre-commit phase 1) now fails if any
workflow's two lists differ, reporting which entries are on which side.
Mutation-tested: dropping the installer from the pull_request list alone reds
it, naming that entry.
…entity Two defects in the gate that carries this branch's primary evidence, both of them the claim outrunning the code. **Coverage.** `Results::offsets` is `HashMap<Ident, usize>` -- one entry per VARIABLE, with no extent: `calc_flattened_offsets_incremental` computes a size and `CompiledSimulation` drops it. Reading one slot per entry therefore sampled only the FIRST element of every arrayed score, while the rustdoc called itself a digest over every LTM slot. Measured: 7,000 of **20,892** LTM slots, across 1,088 arrayed variables out of 7,153. Extents now come from each variable's own declared dimensions resolved through the project's dimension context, which is the same derivation `db::ltm_value_gate_tests` already used. The gap was not theoretical. Widening it raises `nonzero_slots` from 1,369 to **3,141**, so 1,772 slots carrying real scores were invisible; and the positive control -- forcing the predicate true -- now zeroes **614** real scores where the old walk saw 82. The gate was seeing an eighth of the damage it was built to detect, which made it weaker than the throwaway probe it was meant to replace. **Permutation-invariance.** The magnitude sums and both counts are unchanged when two slots exchange their maxima, so an offset or remapping regression that attached correct values to the wrong links would pass -- and the rustdoc claimed such a swap moved the digest. `slot_digests` adds an FNV-1a over the canonically ordered `(name, element, mantissa, exponent)` stream, binding each contribution to the slot it came from. The sums are kept because they are interpretable: a drop in one localises a regression faster than a hash does. The property is demonstrated rather than asserted. `permuting_two_slots_moves_only_the_identity_digest` constructs the swap and pins BOTH halves -- that the sums really are unchanged, so the blindness is not a strawman, and that the identity digest really moves. It covers a swap between variables and a swap between elements of one arrayed variable, the likelier remapping bug. It is a fast default-suite test rather than part of the `#[ignore]`d run, since the property belongs to the digest function and needs no model. Both digests stay stable across runs and across allocator or layout changes, because the order and the inputs derive from names and relatively-quantized values rather than from addresses -- preserving the relative-tolerance property the mantissa split exists for. `CLEARN_LTM_SLOTS` and `CLEARN_LTM_UNKNOWN_EXTENT` are pinned too, so a change that silently narrows what is examined fails here instead of passing quietly. **Value-neutrality re-established at the wider coverage, and this is the part that matters for the PR.** With the omission disabled, every pinned number is identical -- now over all 20,892 slots, where the previous run could only speak for the 7,000 it sampled. The controls are otherwise unchanged: the five lag-alignment coverage rows behave exactly as their per-clause mutation table says, with the original-`PREVIOUS` revert redding one row and the nested-descent revert redding two. New pins: 20,892 slots / 0 unknown extent / 3,141 non-zero / 20,892 finite / mantissa 798,101,758,590 / exponent 2,254 / identity 11438420344658315382.
|
All five addressed in Digest coverage — confirmed, and the gap was doing real damage
Widening it to every element:
So 1,772 slots carrying real scores were invisible, and the gate was detecting an eighth of the damage its own positive control inflicts. It also now pins a count of LTM-prefixed result slots whose extent the metadata does not describe at The consequence for the PR is a strengthening, not a retraction: value-neutrality re-run at the new coverage is identical in every pinned number with the omission disabled. The previous result could only speak for the 7,000-slot sample; the central claim is now supported roughly three times as well. Permutation-invariance — confirmed, and demonstrated rather than assertedFixed by adding an FNV-1a over the canonically ordered
The stale mode stamp — correct, and the guard shared its failure modeReproduced with a
Step 3 exits 0 with the raw output staged and stamped Beyond the report: The Installer in the path filters — correctAdded to both lists. Every other input was checked against "can a change to this file break something only this lane would catch", and nothing else qualified — Also added a lint rule (pre-commit phase 1) failing if any workflow's Remaining fixed-arity docs — correct, and there were three
Full pre-commit gate green. @codex review |
Review summaryReviewed the 8,708-line diff across LTM behaviour, VM/bytecode fusions, compile-pipeline salsa changes, and CI/scripts. Delegating to four focused agents plus direct reading of the load-bearing files. Verdict: no correctness defects found. The PR is unusually well-instrumented for a change of this size — every risky decision is either type-checked ( Notes worth surfacing (all non-blocking)
What was checked and OK
Overall correctness verdict: correct. |
`permuting_two_slots_moves_only_the_identity_digest` asserted that two slots exchanging their maxima moves the identity digest, and claimed that as evidence the fold binds each contribution to its slot. It is not: FNV-1a is an ORDERED fold, so exchanging the two maxima reorders the value stream and moves the hash whether or not `name` and `elem` are in it. Measured -- deleting both identity bytes from the fold left the test green. So the test passed for a different reason than it claimed, which is the same defect this gate has now been fixed for three times, this time in the test rather than in the code under it. The two properties are separate and both are now rows: * a VALUE SWAP -- maxima exchanged between slots, canonical order fixed. Moves an ordered fold with or without identity, so it constrains the ordering, not the identity. * a REBINDING -- the same maxima, in the same order, attached to a different slot identity: a renamed variable, or the same value at a different element index. Only the identity bytes catch this, and it is the closer analogue of the offset/remapping regression the term was added for. Each is mutation-tested against the fold it constrains: removing `name`/`elem` now reds the rebinding row (it previously red nothing), while the swap row stays green, which is exactly the discrimination that was missing. The C-LEARN digest itself is unchanged -- its real caller sorts by `(name, element)`, so a swap does move it. What was wrong was the evidence, not the pin: all seven pinned numbers are identical and the run still passes.
|
Correction to my previous comment, plus two numbers I got wrong. Pushed as The permutation test I described did not test what I said it tested. I wrote that it "constructs the swap and pins both halves — that the plain sums are blind to it, and that the identity digest is not". The first half was true; the second was not tested. Mutating the identity bytes out of the fold left the test green, because FNV-1a is an ordered fold: exchanging two slots' maxima reorders the value stream and moves the hash whether or not Now split into two rows constraining different things:
The rebinding row is also the closer analogue of the regression at issue: an offset or remapping change attaches correct values to the wrong links, which leaves both the multiset and its order intact — so the swap case would not have caught it either. The C-LEARN pin itself was never wrong: its real caller sorts by Two numbers corrected. The positive control zeroes 614 real scores at full coverage, not 82 — 82 was the pre-widening figure and I quoted it in the "before" column while describing post-widening behaviour. And my estimate that C-LEARN had ~24,908 LTM slots was built on a wrong subtrahend: I used 5,215 as the non-LTM slot count when it is 9,132 (5,215 is a variable count, not a slot count). The measured accounting closes exactly — 30,024 slots/step, 21,011 described by LTM metadata, 119 never saved, 20,892 covered, Worth stating plainly, since it is the third time this gate has been wrong about itself — quantization, then coverage and permutation-invariance, now the test asserting a property it did not exercise. The first two were in the artifact; this one was in the evidence, which is the harder kind to notice and the reason the property is now split across two rows rather than argued. Full pre-commit gate green. 75 commits. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3029f138a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| bash "$REPO_ROOT/scripts/verify-deploy-build.sh" | ||
| # REQUIRE_WASM_OPT=1: this is a deploy, so the WASM must be wasm-opt'd. CI runs | ||
| # the same script after a deliberately unoptimized build and does not set it. | ||
| REQUIRE_WASM_OPT=1 bash "$REPO_ROOT/scripts/verify-deploy-build.sh" |
There was a problem hiding this comment.
When a developer uses the preferred pnpm deploy:web:staged path without wasm-opt, build.sh stages a raw blob and this new REQUIRE_WASM_OPT=1 check aborts the deploy. However, docs/dev/deploy.md still states that a deploy works without wasm-opt and merely ships a larger bundle, so the documented prerequisite now contradicts this path; distinguish the root fallback from the staged deploy's hard requirement.
AGENTS.md reference: AGENTS.md:L119-L119
Useful? React with 👍 / 👎.
| PATHS_OUTPUT=$(mktemp) | ||
| if ! python3 - > "$PATHS_OUTPUT" <<'PYEOF'; then | ||
| import glob, sys | ||
| import yaml |
There was a problem hiding this comment.
On a clean repository setup, Python has no yaml module—the repo neither declares nor installs PyYAML—so this import raises ModuleNotFoundError. The surrounding failure branch reads only redirected stdout and increments ERRORS once per line, while the traceback goes to stderr; consequently bash scripts/lint-project.sh prints the traceback followed by Project lint check passed. and exits 0, so the new workflow-path consistency rule does not run in pre-commit or CI. Use the repository's declared YAML tooling or make any Python invocation failure increment ERRORS.
Useful? React with 👍 / 👎.
The rule added to catch silently-diverging workflow path filters was itself silently not running. It imported PyYAML, which this repo neither declares nor installs, and the surrounding branch counted only stdout lines as errors -- so on a machine without it the traceback went to stderr, no error was counted, and `lint-project.sh` printed "Project lint check passed" and exited 0. Reproduced by shadowing the module: traceback, then a passing lint. It happened to work here because PyYAML is a system package on this machine, which is the whole hazard: an undeclared import makes a check's coverage a property of the developer's machine. check-workflow-paths.py parses the subset of YAML it needs with the standard library. It is strict in the direction that matters: a shape it does not model -- an inline `paths:` value, an unparsed line or a mapping key nested inside a paths list -- raises, so a workflow it cannot read fails loudly rather than reporting no difference. That property is tested, not asserted; the first version of the parser silently ignored a stray key inside a paths list while its docstring claimed otherwise. The bash side gains run_line_check, which treats a check that FAILS TO RUN as an error in its own right. That fixes the same latent defect in rule 3: if check-copyright.py crashed, its traceback went to stderr, its stdout was empty, and the lint passed. A rule that has silently stopped running looks exactly like a rule that found nothing.
|
The PyYAML finding is correct on both halves and is fixed. Reproduced before fixing, by shadowing the module: traceback to stderr, then It worked here only because PyYAML is a system package on this machine — which is the hazard in one line: an undeclared import makes a check's coverage a property of the developer's machine.
That strictness is tested rather than asserted, and it needed to be — the first version of the parser silently ignored a stray key inside a Four properties now verified: a clean tree passes; real drift is caught (dropping one entry from the Your second point — that the failure branch only reads stdout — was the more valuable half, and it generalised. Rule 3 had the identical shape: if Not taking the P3 on Full pre-commit gate green. |
1 participant
A performance pass over
simlin-engine: compilation and simulation, with and without LTM, on the two hero models. Every change is behaviour-preserving except one, which is called out below.Results
Interleaved A/B against the merge base, both binaries built and run in the same session. Wall-clock on a contended box is corroboration only; retired instructions are the load-bearing channel (see the methodology note added to
docs/design/engine-performance.md).Retired instructions for the C-LEARN LTM run: −41.1%. LTM compile-stage peak live bytes: 438.8 → 353.2 MiB (−19.5%).
Interactive edit latency — a single-equation edit on C-LEARN, paired over 40 variables:
The median was already fine; the tail was the problem. A cheap edit is now 40.6M instructions, dominated by salsa's own
maybe_changed_afterverification plus re-lexing the one edited equation.Developer loop:
cargo test --workspace−25% on a 4-core CI shape (31.0 → 23.2 s), and ~170 s off every engine-touching pre-commit run.What changed
Compile. Six changes, all artifact-identical: a provably-identity
canonicalizeremoved from a per-dimension predicate;variable_dimensionsderived without a second parse; two previously-untracked functions (compile_implicit_var_fragment, the cycle gate's fragment probe) made salsa queries; FxHash on the topological-sort probe maps; and the engine's own·separator answered without a Unicode case-table lookup.VM. Superinstructions, all created by
ByteCode::fuse_three_addresson the Vm's private execution copy unless noted:SetCond;If[;AssignCurr](100% adjacent by construction — codegen is their sole producer and emits them together), leaf stores and a fusible module-input leaf,LoadPrevConst,ApplyTerConst,SubVarPrev,BinStackPrev. Two reach codegen and therefore wasmgen: real builtin arity instead of paddingApplyto three operands, and a lookup's constant element offset resolved at compile time.LTM. The direct fragment compile is memoized — 5,985 of 7,125 LTM variables previously took an uncached path and were compiled twice per diagnostics pass, three times in MCP
edit_model. Equation ASTs areArc-shared between the shaped memo and the emission loop. And link-score arms whose ceteris-paribus partial is provablyPREVIOUS(target)are omitted rather than materialized: 4,335 arms on C-LEARN, −19.2% of the flow program.The one behaviour change
Arms are omitted only when the partial is provably
PREVIOUS(target)and lag-aligned — every read exactly one step back. Lag alignment is enforced, not assumed: an originalPREVIOUSin the target's own equation, or a synthesizedPREVIOUSnested inside another, both break it and both are rejected. Each has its own gate row, mutation-tested per clause (reverting either check fails exactly one row and neither covers the other). On both hero models no arm was ever misaligned, which is why every earlier measurement was clean and why enforcing this costs zero arms — but that is a property of those models, not of the change.Given that, the transformation is bit-exact except for the sign of zero. Omitted arms lower to
AssignCurr(off, Const(0.0))=+0.0, where a materialized arm ends in... * SIGN(Δx)and can produce−0.0. Whole-slab differential over 7,560,873 slot-steps: 51,358 differ, in exactly one bit-pattern pair (−0.0 → +0.0), numerically zero. Nothing in the engine branches on sign-of-zero —vm.rspinseval_op2(Eq, 0.0, -0.0) == 1.0and!is_truthy(-0.0),float.rspinsapprox_eq(0.0, -0.0), and relative loop scores sum|score|. One cosmetic surface: asimlin-clicolumn that printed-0now prints0.Why the obvious cheaper version is wrong, since it is what a reviewer will reach for: testing "the link's source stayed frozen" (
live_ref == None) is a different question and is unsound — it changes 187 result slots across 35 link-score variables, 151 of them by ≥ 1.0, with a worst case of 8,086.97 → 0. Those arms are not structurally zero; they are scoring the clock, because TIME is excluded from the ceteris-paribus freeze. See #1016. The predicate here asks the positive question instead, and is stable under fixing that defect.Evidence, and its boundary
--no-verify.clearn_residual_exactness,simulates_clearn,oracle_clearn,clearn_ltm_var_count_guardrailgreen throughout.cargo test -p simlin-engine --release --test integration -- --ignored clearn_ltm_slot_maxima_digest(name, element, mantissa, exponent)stream, and two separate tests constrain two separate properties: a value swap (maxima exchanged, order fixed) pins that the fold is ordered, and a rebinding (same maxima, same order, different name or element index) pins that the identity bytes are actually read. The second exists because the first does not imply it — an ordered fold moves under a swap whether or not identity is in the stream, and a rebinding is the closer analogue of the offset-remapping regression this is meant to catch, since it leaves both the multiset and its order intact.#[ignore]dsimulates_delayfixed*failures are pre-existing (engine: DELAY FIXED ring-buffer semantics blocks 4 simulation tests #346), verified by reverting to base.New tests, because three gaps were found
clearn_residual_exactnessnever enables LTM, and the var-count guardrail moves on count and width, neither of which arm omission touches. Added: a sub-second default-suite gate with one arm per known failure mechanism, plus an#[ignore]d C-LEARN slot-maxima digest. The gate includes a positive control — forcing the predicate true drops 82 real scores to zero and reds the digest — so a passing run cannot mean a blind instrument.STACK_CAPACITYis 64 and the deepest stack in the corpus is 8, so a wrongstack_effecthas 5x headroom to hide in and neither a passing suite nor a matching fingerprint reaches it. Added a corpus audit asserting fusion never raises peak depth and neither stream underflows.Measured and declined
Recorded so they are not re-proposed:
opt-level: +144 s per engine-touching CI build, and it would push the Rust pre-commit pipeline past the TypeScript one — the hook ismax(A,B,C)with 24 s of slack — making the developer's hook slower while saving time on a pipeline that is not the critical path. A loss on both sides.opt-level=1emits a 61% larger module andwasm-opt's cost scales with input size, soopt-level=zis both smaller and faster.dt/initial_time/final_timeprefix is established once byrun_initials.ABS/SIGN. A helper read and a fused opcode are both one dispatch.Documentation
docs/design/engine-performance.mdgains a measurement-methodology section, and it corrects a conflation that had been doing damage: the recorded "~4% noise floor" is a cycles floor and does not bind retired instructions, whose measured sd across six independent builds is 0.026%. That conflation had already carried a verdict — the #711 lazy-IFNO-GO cites "below the ~4% layout-noise measurement floor" for a 1.5% instruction share, which is ~58 sigma. The verdict stands for other reasons; only its stated reason is corrected, here and on the issue.Evidence for the floors is a null control: the same binary as both sides of an interleaved A/B reports −0.003% on instructions and −1.540% on cycles — a "win" from measuring nothing.
Costs
Vm::newis slower — 5.67 → 8.59 ms on C-LEARN+LTM — because the fusion pass scans more windows over a larger program. It is 3 ms against a 943 ms run.An open question this surfaced but did not answer
1,369 of 7,000 C-LEARN LTM slots are ever non-zero — 81% are identically zero across the whole run in discovery mode. Uninvestigated. It is either uninteresting (many causal edges genuinely score zero) or the next large lever, and leaving it unstated guarantees nobody finds out which.
Discovered and filed rather than fixed here
#1016 (should a ceteris-paribus partial freeze TIME? — worth ~25 further points of the LTM flow program, but it moves ~41% of C-LEARN's link scores and needs dominance-ranking validation), #1017 (a repeated dimension enumerates 34,904 circuits that are all declined), #1018 (dimension narrowing matches raw spellings against display names). #977 was rewritten around this work and its stale 322-arm row corrected to 0 with the measurement attached. #715 and #915 gained corpus facts that make them concrete.
What review caught
Two
/code-reviewpasses at high effort over the whole diff, nine findings, all resolved. The second pass verified the risky engine changes independently — every fused opcode's stack effect against the sequence it replaces, operand pop order including the non-commutative cases, jump guards andpc_map1:1-ness, builtin arity across the VM, codegen and wasm arm-for-arm, and that both lag-alignment checks are load-bearing — and found no correctness defect in them.Every finding was verified against the code before being acted on. Four came back different from how they were reported, and in three of those the difference changed the fix:
assert!(checked > 100)into a per-modelassert!(checked > 0), closing a guard that could itself pass vacuously.jqdependency on the primary build path: removed rather than accommodated, sincepython3is already required earlier on the same path.The two that were straightforwardly right were the two that mattered most:
variable_dimensionsdiverged on valid models, not only on ones that already fail to compile — two shapes went from 1 slot to N. A 483-model scan found zero shipped models affected, so artifact-identity held by corpus luck rather than by construction. Gating on the mechanism (an equation that lexes to zero tokens, which is the parser's own early return) rather than on the two shapes found immediately turned up a third — comment-only equations — that the natural fix would have answered wrongly.PREVIOUSas step-invariant, so an arm whose target equation already contained one could be dropped despite scoring 0.985. A second shape was found that the review missed — and its numbers had been sitting in the tree for weeks, documented as an open semantics question while also being unread evidence that such an arm is not zero. Both shapes now have gate rows, mutation-tested per clause. The fix costs zero arms and let an earlier workaround be deleted.A third round found five more, four of them the same pattern and two of them in the digest that is this branch's primary evidence: it sampled one element per arrayed variable (7,000 slots of 20,892) while documenting itself as covering every slot, and its unweighted sums were permutation-invariant, so values attached to the wrong slots would not have moved it. Widening it showed the gap was not theoretical — 1,772 slots carrying real scores were invisible, and the gate was detecting an eighth of the damage its own positive control inflicts (82 of 614). Re-running value-neutrality at the full coverage: every pinned number identical with the omission disabled, so the central claim is now supported roughly three times as well as when it was first made. The others: a stale "optimized" stamp could survive a failed optimization and make the next build treat a raw blob as optimized — and the guard meant to catch that read the same stamp, so the backstop shared the failure mode of the thing it backed up; the installer the optimized-WASM lane depends on was not in its own path filters; and two more copies of the stale fixed-arity comment, one of them in the file that defines the opcode, sitting immediately above the line that replaced it.
One pattern ran through all of it, worth naming because it is the round's most reliable defect source: every one of these was prose asserting a property the code did not have — a noise floor applied to the wrong channel, "bit-exact" without lag alignment, "only unparseable equations", "nine significant digits" on a digest that quantized absolutely. Prose is the one artifact nothing executes. The durable fix, applied repeatedly here, is to make the claim executable or make its violation a compile error.
The named next round
Nothing here is started, and each carries its measurements so it does not begin from zero:
Fixes #1019