diff --git a/src/simlin-engine/CLAUDE.md b/src/simlin-engine/CLAUDE.md index 4a244b6a2..78491bc77 100644 --- a/src/simlin-engine/CLAUDE.md +++ b/src/simlin-engine/CLAUDE.md @@ -178,7 +178,7 @@ The unit subsystem is partial-result throughout: a single bad declaration or one - `tests.rs` - integration-style tests spanning all submodules; preserved as a single file to avoid splitting shared test fixtures. - **`src/ltm_finding.rs`** - Strongest-path loop discovery algorithm (Eberlein & Schoenberg 2020). Post-processes simulation results containing link score synthetic variables to find the most important loops at each timestep. `discover_loops_with_graph(results, causal_graph, stocks, ltm_vars, dims, expansion, sub_model_output_ports, budget)` is the primary entry point: `ltm_vars` and `dims` enable A2A link score expansion (per-element edges from `parse_link_offsets`); when empty, all link scores are treated as scalar. A Bare A2A link score is dimensioned over the TARGET's dims, so `expand_a2a_link_offsets` subscripts the TARGET node per element but PROJECTS the SOURCE node onto `from`'s OWN declared dims -- bare for a scalar feeder (`scale → growth`, GH #790), the same-element diagonal for an equal-dim feeder, the broadcast/partial-collapse form for a lower-dim feeder, and the positionally-mapped diagonal for a `State→Region` pair (GH #527) -- by reusing the element graph's own `db::expand_same_element` rule, so the discovery search graph's node names match `model_element_causal_edges` node-for-node (GH #754; before this, subscripting BOTH endpoints with the score's full tuple minted phantom from-nodes like `scale[a]`/`boost[r,a]`/`x[s]` that named no real node, so every loop through such a feeder dangled and was silently undiscoverable). The from-var declared dims + dimension-mapping context ride in a `LinkExpansionContext` (`declared_dims` + `dim_ctx`) that `analyze_model` builds via the public `analysis::build_link_expansion_context` (the SAME `variable_dimensions` / `project_dimensions_context` queries the element graph reads); the db-less `discover_loops(&Results, &Project)` convenience path passes `LinkExpansionContext::default()` (no A2A expansion runs there). Element-mapped (non-positional) pairs never reach the projection: `db::ltm::link_score_dimensions` declines to retarget them (no Bare A2A score; the GH #758 loud skip fires instead) under the GH #756 positional-only gate. `SearchGraph` provides DFS-based strongest-path traversal from stock nodes. The post-simulation score recompute (path → `FoundLoop`) applies the **per-exit-port pathway selection** (`recompute_module_input_edge_series`, GH #698): a loop edge `x → m` into a module is recomputed by max-abs-selecting over the sub-model's `m·$⁚ltm⁚path⁚{entry}⁚{idx}` pathway scores that end at the exit port the loop reads (recovered from the next link `m → y`), instead of reading the module *composite* (which max-abs-selects across ALL ports and so can pick a wrong-signed port for a multi-output module -- flipping the loop polarity vs exhaustive). The pathway indices come from the module's sub-graph via the same `enumerate_pathways_to_outputs_with_truncation` over the same sorted output-port set the sub-model emitted against, so they match index-for-index (the discovery-mode equivalent of exhaustive's `compute_module_link_overrides`). That port set is NOT re-derived parent-scoped (which would shift indices when another project model reads an extra output port -- GH #698 / PR #705 r3353097150): `discover_loops_with_graph` takes a `SubModelOutputPorts` map (sub-model canonical name -> emitted sorted port set) that `analyze_model` builds from the SAME emission decision via `analysis::build_sub_model_output_ports` -> `db::ltm::sub_model_output_ports` (identity-by-construction); the db-less `discover_loops(&Results, &Project)` convenience path reconstructs the same project-wide-union + stdlib-`output` semantics in `project_sub_model_output_ports`. Falls back to the base composite offset for single-output / pathless / indeterminate-port / sub-model-absent-from-map edges. Because the discovery graph is element-level, the recompute `strip_subscript`s `link.from`/`link.to`/`next.from`/`next.to` before its name matches (arrayed loop nodes carry `[elem]`; mirrors the exhaustive twin's stripping -- PR #705 r3353758167); this is currently latent parity defense, since an arrayed loop through a multi-output module is not yet discoverable end-to-end (a scalar module output feeding an arrayed reader scores a constant-0 link, dropping the loop -- GH #716). Returns a `DiscoveryResult` whose `FoundLoop`s carry per-timestep link/loop/pathway scores, ranked **competitive-first** (`rank_and_filter`): loops sharing their cycle partition with at least one other discovered loop come first, ordered by mean partition-relative importance; loops trivially ALONE in their partition (relative score exactly +/-1 by construction -- zero information, e.g. C-LEARN's isolated gas-uptake decay loops) sort after all competing loops and are dropped first under the `MAX_LOOPS` cap. Each `FoundLoop` carries a result-scoped dense `partition` index into `DiscoveryResult::partitions` (`DiscoveredPartition { stocks, loop_count }`, first-appearance order; NOT stable across runs/edits -- key on the stock set for durable identity), threaded through `analysis::ModelAnalysis::partitions` / `LoopSummary::partition`, the FFI `SimlinDiscoveredPartition` / `SimlinDiscoveredLoop.partition`, and pysimlin `Analysis.partitions` / `Loop.partition`. Also hosts the link-set synthetic-node collapse: `trim_synthetic_aggs_from_loop_links` collapses `$⁚ltm⁚agg⁚{n}` nodes out of a single loop's link *cycle*, and the public `collapse_synthetic_links(Vec)` generalizes that to ALL synthetic/macro/module-internal nodes (`ltm::is_synthetic_node_name`) over an arbitrary link *set* -- each chain `X -> $⁚…internal… -> Y` collapses to one composite edge `X -> Y` with product polarity and the per-timestep max-magnitude path score (the composite link score, LTM ref 6.3/6.4); a purely-internal cycle/source is dropped. `CollapsibleLink { from, to, polarity, score: Option> }` is the abstract shape, so structural-only callers (`score = None`) and LTM-run callers share one impl. libsimlin's `simlin_analyze_get_links(.., include_internal=false)` collapses the `get_links()` set through this; `include_internal=true` returns the raw graph. The `#[cfg(test)]` tests live in the sibling `src/ltm_finding_tests.rs` (mounted via `#[cfg(test)] #[path]`, split out for the per-file line cap). - **`src/ltm_agg.rs`** - Aggregate-node enumeration for LTM. `enumerate_agg_nodes` (salsa-tracked) walks every variable's `Expr2` AST left-to-right depth-first and identifies each maximal array-reducer subexpression (`SUM`/`MEAN`/`MIN`/`MAX`/`STDDEV`/`RANK`/`SIZE`); AST-identical subexpressions (keyed by canonical printed equation text) dedupe to one `AggNode`. Two kinds: **synthetic** (`is_synthetic == true`, the reducer is a sub-expression of a larger equation -- a `$⁚ltm⁚agg⁚{n}` aux is minted) and **variable-backed** (`is_synthetic == false`, the reducer is the entire dt-equation of a scalar/A2A variable like `total_population = SUM(pop[*])` -- the variable itself is the agg; EXCEPT a whole-RHS reducer whose shape the variable-backed machinery cannot express -- a MAPPED iterated axis (GH #534: the name-based link-score path cannot remap, so its `Wildcard` partial would silently stub to 0) or NON-ALIGNED result dims (GH #764 / shape-expressiveness T4: a broadcast over extra owner dims `out[D1,D3] = SUM(matrix[D1,*])` or permuted axes `out[D2,D1] = SUM(cube[D1,D2,*])`, where a per-`(row, slot)` slot cannot name a complete owner element) -- which mints a synthetic agg instead (`variable_backed_shape_is_expressible`, the ONE minting condition), riding the two-half emitters + the GH #528 projection). Each `AggNode` carries a `sources: Vec` -- one entry per source variable, SORTED by canonical name and deduped (salsa cache-equality + emission-order determinism; T2 of the shape-expressiveness design), each with its OWN `read_slice: Vec` (one `AxisRead ∈ {Pinned(elem), Iterated{dim, source_dim}, Reduced{subset}}` per THAT source's axes -- which rows of it the reducer actually reads; a scalar feeder like `scale` in `SUM(pop[*] * scale)` carries an empty slice; `Iterated` carries the (target, source) canonical dim pair, equal for the literal case; `Reduced.subset` is `None` for the full extent or the proper-subdimension element subset for a `SUM(arr[*:Sub])` StarRange, GH #766, decided per axis by `classify_axis_access` -- the single per-axis classifier of the shape-expressiveness design) -- and a `result_dims` (the `Iterated` axes' TARGET dims, datamodel-cased -- empty for a whole-extent or pinned-slice reduce). Under the I1 acceptance (`accept_source_slices`, T5 of the shape-expressiveness design / GH #767) every arrayed CO-SOURCE (`Reduced`-bearing slice) carries the identical *canonical* slice (`AggNode::canonical_read_slice` -- the first `Reduced`-bearing source slice, falling back to the first non-empty for the degenerate no-co-source agg), while an ITERATED-DIM PROJECTION FEEDER -- a source whose slice is all-`Iterated` over exactly the canonical slice's iterated target dims, in order, unmapped (`frac[D1]` in `SUM(matrix[D1,*] * frac[D1])`, per-result-slot constant) -- is accepted with ITS OWN slice (`AggNode::source_is_projection_feeder` is the discriminator); per-source consumers (`emit_agg_routed_edges`, `emit_source_to_agg_link_scores`) read `AggNode::source_read_slice(from)`, and name-keyed consumers use `AggNode::reads_var`. A feeder's link-score half is the per-`(row, slot)` CHANGED-LAST equation (`ltm_augment::generate_iterated_feeder_to_agg_equation`, emitted via `link_scores::iterated_feeder_row_scores` from both the synthetic source-half and `try_cross_dimensional_link_scores`' variable-backed feeder branch): the reducer text pinned to the slot with only the feeder frozen -- the arrayed generalization of the GH #737 scalar-feeder convention, exactly complementary per slot to the co-source rows' changed-first numerators for a bilinear body; the co-source rows' changed-first body partial pins the mismatched-arity feeder dep BY DIM NAME (`pin_body_to_row`'s GH #767 extension) so `PREVIOUS(frac[d1·r])` is held frozen at the row instead of bailing to the delta-ratio fallback. `compute_read_slice` decides hoistability: a whole-extent reduce (`SUM(pop[*])` ⇒ all-`Reduced`), a sliced one (`SUM(pop[NYC,*])` ⇒ `[Pinned(nyc), Reduced]`, `SUM(matrix[D1,*])` over an A2A-`D1` body ⇒ `[Iterated(d1,d1), Reduced]` → an arrayed agg over `D1`), a mixed one (`SUM(matrix3d[D1,NYC,*])` over an A2A-`D1` body ⇒ `[Iterated(d1,d1), Pinned(nyc), Reduced]`), or a positionally-MAPPED sliced one (GH #534: `SUM(matrix[State,*])` over `matrix[Region,D2]` with a positional `State→Region` mapping ⇒ `[Iterated{state, region}, Reduced]`, `result_dims = [State]` -- the agg is arrayed over the TARGET's iterated dim and the three `Iterated`-axis consumers remap each source row to the slot of its positionally-corresponding target element via `iterated_axis_slot_elements`, the preimage inversion of `mapped_element_correspondence`, so the positional-only/element-map gate is inherited) is hoisted; the carve-outs are (a) a reducer over a *dynamic index* (`SUM(pop[idx,*])`, `idx` non-literal ⇒ not statically describable ⇒ not hoisted, reference stays on the conservative path -- `db::ltm_ir` reclassifies it as `DynamicIndex`), (b) an ELEMENT-mapped sliced reducer the correspondence declines (execution resolves positionally and ignores the map, GH #756; a POSITIONAL mapping is accepted in either declaration direction since GH #757) ⇒ `compute_read_slice` returns `None`, conservative -- (c) a multi-source slice combination outside the I1 acceptance -- co-sources with differing slices, one variable read with two different slices (I3b), or a no-`Reduced` source that is not the pure iterated projection (a Pinned-axis mix like `SUM(matrix[D1,*] * w[D1, c1])`, a dim-subset/permuted feeder, or any mapped Iterated axis in a feeder combination) -- `combined_read_slice`/`accept_source_slices` return `None`, (d) a StarRange naming a NON-subdimension of its axis (a mid-edit inconsistency; declined rather than silently widened to the full extent), and (e) `RANK` (GH #771: array-valued, so `reducer_is_hoistable` requires `reducer_collapses_to_scalar` and RANK references stay `Direct` -- a bare arg classifies `Bare`, scored by the GH #742 arrayed-capture path; loops through the rank ORDERING are a documented residual). A multi-source reducer whose arrayed args *agree* (`SUM(a[*] + b[*])`, `a`, `b` over the same dim) mints one agg with one `AggSource` per variable, each carrying the shared canonical slice. `AggNodesResult` exposes `aggs` (first-encounter order), `agg_for_key` (by canonical reducer text), and `aggs_in_var` (which aggs occur in a variable's equation, so the element-graph reroute can ask "which agg of `to` reads `from`?"). A variable-backed reduce with a NON-TRIVIAL statically-describable slice is gated by `variable_backed_reduce_agg` (GH #752, generalized by GH #765 / T3 of the shape-expressiveness design) -- the SAME gate `model_element_causal_edges`' dispatch, `build_element_level_loops`' per-circuit routing, and `try_cross_dimensional_link_scores`' row derivation consume, so edges, loop routing, and scores always cover the identical read rows (all three derive from `read_slice_rows`, invariant I4). Accepted: an ALIGNED partial reduce (`row_sum[D1] = SUM(matrix[D1,*])`, `result_dims` equal to the variable's own dims in order -- Pinned-mixed `outf[D1] = MEAN(cube[D1,x,*])` and subset `out[D1] = MEAN(matrix[D1,*:Sub])` slices included: the divisor is the true read count and unread rows get neither edges nor scores) gets read-slice element edges straight onto the variable's element nodes and per-circuit scalar loops whose both-subscripted `matrix[d1,d2]→row_sum[d1]` links resolve the per-`(row, slot)` scores; a scalar-result Pinned/subset slice on a SCALAR owner (`total = SUM(pop[nyc,*])`, `total = SUM(arr[*:Sub])`) routes the read rows into the bare `to` node with matching per-read-row scores; and the ARRAYED-owner scalar-result Pinned/subset BROADCAST slice (`share[Region] = SUM(pop[nyc,*])`, no `Iterated` axis -- GH #777) fans each read row across the FULL target element set (`emit_agg_routed_edges`' broadcast arm emits `pop[nyc,d2] → share[e]` for every `e`; `emit_broadcast_reduce_link_scores` emits the matching per-(read-row, full-target-element) scalar scores `pop[nyc,d2]→share[e]`, the section-3 `PerElement` rule applied to a variable-backed reducer owner), with loop circuits routed to the per-circuit scalar path via `is_broadcast_reduce_edge` -- the read rows are independent of `to`'s dims, so the RELATED-dim (`share[Region]`) and DISJOINT-dim (`share[D9]`) spellings emit identically. Declined: a pure full-extent variable-backed agg keeps the normal reference walker's edges (the true reads for that shape, inert skip). The GH #764 broadcast/permuted result shapes never reach this gate since T4 -- they mint synthetic aggs at enumeration -- so the gate's Iterated-arm alignment check is defense-in-depth. `scalar_feeder_of_variable_backed_agg` (GH #790) is the scalar sibling of `source_is_projection_feeder`: it composes `variable_backed_reduce_agg` with an empty-slice + genuine-`Reduced`-canonical check to recognize a SCALAR FEEDER of a whole-RHS variable-backed reduce (`scale` in `growth[D1] = SUM(matrix[D1,*] * scale)`), so `try_scalar_to_arrayed_link_scores` can route it to the single Bare A2A changed-last feeder score instead of the uncompilable per-target-element partials. `is_synthetic_agg_name` / `synthetic_agg_name` are the `$⁚ltm⁚agg⁚{n}` name helpers. -- **`src/ltm_augment.rs`** - Equation generators for LTM synthetic variables: `generate_link_score_equation_for_link` (ceteris-paribus link scores; takes `RefShape` and source dimension elements to drive per-shape PREVIOUS wrapping), `generate_loop_score_variables` (emits one `loop_score` per loop as a dimension-shaped `datamodel::Equation`: `Scalar` for scalar loops, `ApplyToAll` for dimensioned loops whose links resolve through Bare A2A names, and per-slot `Equation::Arrayed` for dimensioned loops backed by per-element circuits via `Loop.slot_links` -- GH #653; relative loop scores are computed post-simulation in `ltm_post.rs`), `build_partial_equation_shaped` (AST-based PREVIOUS wrapping that holds matching-shape references live and wraps everything else, via `wrap_non_matching_in_previous` and `classify_expr0_subscript_shape`; arrayed-per-element-equation (`Ast::Arrayed`) targets get one partial per element assembled into an `Equation::Arrayed`), `link_score_var_name` (synthetic name helper: Bare gets the canonical `{from}\u{2192}{to}` form, FixedIndex prepends `[elem]` to from; the obsolete per-shape `\u{205A}wildcard`/`\u{205A}dynamic` Wildcard/DynamicIndex suffixes were retired -- those shapes now collapse onto the Bare name, since *every statically-describable* inlined reducer (whole-extent or sliced) is hoisted into a `$⁚ltm⁚agg⁚{n}` node and only a `DynamicIndex` reference -- `arr[i+1]`, a range, or the not-hoistable dynamic-index reducer carve-out `SUM(pop[idx,*])` -- a whole-RHS variable-backed reducer's `Wildcard` argument, or a de-hoisted array-valued reducer's `Wildcard` arg (`RANK(pop[*], 1)`, GH #771) reach this function), `quote_ident` (identifier quoting for equations). Array support: `classify_reducer` (walks target Expr2 AST to identify reducing builtins -- Linear for SUM/MEAN, Nonlinear for MIN/MAX/STDDEV/RANK, Constant for SIZE -- a thin reader of `ltm_agg::reducer_kind`), `generate_element_to_scalar_equation` (per-element link score equations for arrayed-to-scalar edges, used by both the variable-backed-reducer path and the `source[d] → $⁚ltm⁚agg⁚{n}` half) which dispatches on `ReducerKind` -- `generate_linear_partial` (SUM/MEAN algebraic shortcut), `generate_nonlinear_partial` (MIN/MAX nested binary calls; STDDEV the unrolled population-variance `sqrt` ceteris-paribus partial -- divisor `N`, matching `vm.rs::Opcode::ArrayStddev`, with the mean string-inlined; RANK the documented delta-ratio stand-in pinned by `test_generate_rank_keeps_delta_ratio` -- an order statistic, non-differentiable and unreachable via a real model RHS), `generate_scalar_to_element_equation` (per-element link score for the `$⁚ltm⁚agg⁚{n} → target[e]` half; takes a `source_ref_override: Option<&str>` so a multi-slot arrayed agg's `Δsource` denominator carries the projected `agg[]` subscript instead of the bare agg name, which wouldn't compile as a scalar), `substitute_reducers_in_expr0` (textually replaces a recognized reducer subexpression in an `Expr0` with its agg name, for the `$⁚ltm⁚agg⁚{n} → target` link score), `resolve_link_score_name_for_loop` (picks the Bare-or-FixedIndex link-score name a loop-score reference should target). Module link score formulas (black-box delta-ratio and composite-ref) are inlined directly into `module_link_score_equation` in `db.rs` (called by the per-shape `link_score_equation_text_shaped`). The partial-equation builders (`build_partial_equation_shaped`/`_with_live_ref`, `subscript_idents_at_element`) and every link-score equation generator return `Result<_, PartialEquationError>`: a parse failure (genuine `Err`, or an empty `Ok(None)` equation) has no AST to PREVIOUS-wrap, so emitting the unwrapped input would silently produce a non-ceteris-paribus "partial" identical to the full equation (link score magnitude constant |Δz/Δz| = 1) -- a hidden attribution error that compiles cleanly (GH #311). The db-bearing callers (`link_score_equation_text_shaped` and the `src/db/ltm/link_scores.rs` emitters) convert the error into a `Warning` (`emit_ltm_partial_equation_warning`, naming the variable + offending equation text) and skip the variable -- distinct from `model_ltm_fragment_diagnostics`, which only catches *compile* failures. The failure is effectively unreachable in production (the text is always a `print_eqn` re-print; an empty equation is rejected as an `EmptyEquation` Error upstream), so this is defense-in-depth. The `#[cfg(test)]` tests live in the sibling `src/ltm_augment_tests.rs` (split out for the per-file line cap). +- **`src/ltm_augment.rs`** - Equation generators for LTM synthetic variables: `generate_link_score_equation_for_link` (ceteris-paribus link scores; takes `RefShape` and source dimension elements to drive per-shape PREVIOUS wrapping), `generate_loop_score_variables` (emits one `loop_score` per loop as a dimension-shaped `datamodel::Equation`: `Scalar` for scalar loops, `ApplyToAll` for dimensioned loops whose links resolve through Bare A2A names, and per-slot `Equation::Arrayed` for dimensioned loops backed by per-element circuits via `Loop.slot_links` -- GH #653; relative loop scores are computed post-simulation in `ltm_post.rs`), `build_partial_equation_shaped` (the `#[cfg(test)]` TEXT entry point for the ceteris-paribus wrap; arrayed-per-element-equation (`Ast::Arrayed`) targets get one partial per element assembled into an `Equation::Arrayed`). **Production never parses a target equation**: `wrap_changed_first_ast` takes an `Expr0` lowered straight from the target's `Expr2` by `patch::expr2_to_expr0` (which is what `expr2_to_string` prints, so the former print->reparse was a parse of our own output), and every per-occurrence decision -- access shape, the GH #526 other-dep verdict, the literal-element index guard, and the `PerElement` row pinning -- is a lookup into the `db::ltm_ir` occurrence IR by the structural child-index path the wrap tracks, which equals the occurrence's `SiteId` BY CONSTRUCTION now that both walk the same tree. A subscripted source reference the IR did NOT record -- reachable under a `LOOKUP` TABLE argument, which the walker skips as static data ("not a causal edge", which is right about ATTRIBUTION) -- still has to COMPILE, so the pin-only descent discharges it by NAME (`pin_dimension_name_indices`: an index naming one of the TARGET's iterated dimensions becomes the source element this target element reads on that axis, the same structural substitution `pin_bare_source_ref` performs for a bare `Var`). That is a lowering-completeness rule, not a second classifier: it consults no occurrence, infers no shape, and never makes the reference live-selectable -- it asks the SHARED row derivation `per_element_row_for_target` (hence `DimensionsContext::mapped_element_correspondence`) which element an axis reads, so the identity axis and a positionally-MAPPED one (`effect[State, old]` over an `effect[Region, Age]` source, either declaration direction) are one arm and it accepts exactly the mapped pairs `ltm_agg::classify_axis_access` accepts. An index the source's axis DECLARES as an element is resolved BEFORE that dimension-name reading, matching `compiler::subscript`'s `normalize_subscripts3` ("First check if it's a named dimension element (takes priority)") -- the two readings collide when a dimension declares an element whose name is also a dimension the target iterates, and there the element must win or the pin spells a row the simulation never reads. `classify_axis_access` still has the opposite order (GH #986: a precision gap today, since its dimension-first reading finds no mapping and declines). An index that already selects a FIXED element is left verbatim, needing no pin and reading nothing at the current step: a numeric literal, arithmetic over numeric literals (`index_expr_selects_a_fixed_element`, deliberately the narrowest sound predicate -- `compiler::invariance` is the engine's run-invariance derivation but consumes lowered `Expr` and answers a wider question), or an `@N` position index, which `compiler::context` resolves to a concrete element offset in scalar context (spelling `@N` out here would be a second implementation of position syntax). A 0-arity BUILTIN index is where that widening deliberately stops: `TIME` and `PI` are indistinguishable inside `Expr0::App` without a fourth copy of the builtin classification, so the arm stays conservatively loud. What the SHARED derivation declines -- an unmapped or element-mapped pair (GH #756), a transposition, a dimension the target does not iterate -- is declined LOUDLY (`WrapOutcome::missing_occurrence` -> warned skip) rather than emitted with its dimension-name subscript intact, which would not resolve in a scalar fragment. The whole two-verdict space (`Unspellable`, a compilability verdict loud everywhere; `RuntimeRead`, a ceteris-paribus verdict loud only in a bare table argument) is ENUMERATED cell by cell in `ltm_augment_pin_tests.rs`'s two verdict enumerations rather than sampled. There is ONE access-shape classifier family, on `Expr2`; the Expr0 mirror is test-support only and lives in `ltm_augment_wrap_test_support.rs` (kept because three wrap unit tests -- unparseable text, empty text, and the Fig. 2 Q4 `SUM(w[from]) + from` shape the engine REJECTS as a model -- cannot be db-backed fixtures), with `ltm_classifier_agreement_tests.rs` proving it matches the IR field for field (`SiteId` path, `shape`, `axes`, `in_reducer`) corpus-wide, `link_score_var_name` (synthetic name helper: Bare gets the canonical `{from}\u{2192}{to}` form, FixedIndex prepends `[elem]` to from; the obsolete per-shape `\u{205A}wildcard`/`\u{205A}dynamic` Wildcard/DynamicIndex suffixes were retired -- those shapes now collapse onto the Bare name, since *every statically-describable* inlined reducer (whole-extent or sliced) is hoisted into a `$⁚ltm⁚agg⁚{n}` node and only a `DynamicIndex` reference -- `arr[i+1]`, a range, or the not-hoistable dynamic-index reducer carve-out `SUM(pop[idx,*])` -- a whole-RHS variable-backed reducer's `Wildcard` argument, or a de-hoisted array-valued reducer's `Wildcard` arg (`RANK(pop[*], 1)`, GH #771) reach this function), `quote_ident` (identifier quoting for equations). Array support: `classify_reducer` (walks target Expr2 AST to identify reducing builtins -- Linear for SUM/MEAN, Nonlinear for MIN/MAX/STDDEV/RANK, Constant for SIZE -- a thin reader of `ltm_agg::reducer_kind`; it also hands back the reducer's array-argument AST as `ClassifiedReducer::body`, lowered from `Expr2` rather than printed, so the body-aware row partials never re-parse it), `generate_element_to_scalar_equation` (per-element link score equations for arrayed-to-scalar edges, used by both the variable-backed-reducer path and the `source[d] → $⁚ltm⁚agg⁚{n}` half) which dispatches on `ReducerKind` -- `generate_linear_partial` (SUM/MEAN algebraic shortcut), `generate_nonlinear_partial` (MIN/MAX nested binary calls; STDDEV the unrolled population-variance `sqrt` ceteris-paribus partial -- divisor `N`, matching `vm.rs::Opcode::ArrayStddev`, with the mean string-inlined; RANK the documented delta-ratio stand-in pinned by `test_generate_rank_keeps_delta_ratio` -- an order statistic, non-differentiable and unreachable via a real model RHS), `generate_scalar_to_element_equation` (per-element link score for the `$⁚ltm⁚agg⁚{n} → target[e]` half; takes a `source_ref_override: Option<&str>` so a multi-slot arrayed agg's `Δsource` denominator carries the projected `agg[]` subscript instead of the bare agg name, which wouldn't compile as a scalar), `substitute_reducers_in_expr0` (textually replaces a recognized reducer subexpression in an `Expr0` with its agg name, for the `$⁚ltm⁚agg⁚{n} → target` link score), `resolve_link_score_name_for_loop` (picks the Bare-or-FixedIndex link-score name a loop-score reference should target). Module link score formulas (black-box delta-ratio and composite-ref) are inlined directly into `module_link_score_equation` in `db.rs` (called by the per-shape `link_score_equation_text_shaped`). The partial-equation builders (`build_partial_equation_shaped`/`_with_live_ref`, `subscript_idents_at_element`) and every link-score equation generator return `Result<_, PartialEquationError>`: a parse failure (genuine `Err`, or an empty `Ok(None)` equation) has no AST to PREVIOUS-wrap, so emitting the unwrapped input would silently produce a non-ceteris-paribus "partial" identical to the full equation (link score magnitude constant |Δz/Δz| = 1) -- a hidden attribution error that compiles cleanly (GH #311). The db-bearing callers (`link_score_equation_text_shaped` and the `src/db/ltm/link_scores.rs` emitters) convert the error into a `Warning` (`emit_ltm_partial_equation_warning`, naming the variable + offending equation text) and skip the variable -- distinct from `model_ltm_fragment_diagnostics`, which only catches *compile* failures. The failure is effectively unreachable in production (the text is always a `print_eqn` re-print; an empty equation is rejected as an `EmptyEquation` Error upstream), so this is defense-in-depth. The `#[cfg(test)]` tests live in the sibling `src/ltm_augment_tests.rs` (split out for the per-file line cap). Four more siblings are `#[path]`-mounted into `ltm_augment` purely for that cap, so every caller still names their items `crate::ltm_augment::*`: **`ltm_augment_occurrence.rs`** (the wrap's read side of the occurrence IR -- `SlotOccurrences` groups a target's stream by slot ONCE and is the only way to obtain an `OccurrenceLookup`, so the borrow forces callers to hoist it out of their per-element loop), **`ltm_augment_post_transform.rs`** (the concrete-form lowerings: the agg-name substitution, and the `PerElement` row pinning the wrap calls AS IT DESCENDS -- the wrap is the only place that knows both the occurrence and whether it is about to FREEZE the reference, which is what picks the bare row for the live occurrence over the qualified row for every other one), **`ltm_augment_with_lookup.rs`** (the GH #910 implicit-WITH-LOOKUP rules), and **`ltm_augment_wrap_test_support.rs`** (the `#[cfg(test)]` occurrence reconstruction + the Expr0 classifier mirror described above). - **`src/ltm_augment_with_lookup.rs`** - The implicit-WITH-LOOKUP rules for LTM (GH #910), re-exported from `ltm_augment` so every `crate::ltm_augment::*` path is unchanged. A `v = WITH LOOKUP(input, table)` variable is lowered by the compiler to `LOOKUP(v, input)` (`compiler::apply_implicit_with_lookup`), so a link-score partial that RE-EVALUATES such a target's equation is in gf-INPUT units while the guard form's `PREVIOUS(target)` anchor is in gf-OUTPUT units. `is_implicit_with_lookup` carries the coverage doc: every partial is either a **full re-evaluation** (class 1 -- must be wrapped in the gf application) or a **delta-ratio stand-in** (class 2 -- the RANK arm and the nested-arithmetic arm, already in output units, must NEVER be wrapped or a gf output is fed back through the gf). `WithLookupSlotRefs` resolves the target's table reference ONCE per target (`NoGf` / `Shared` / `PerElement`), so a per-element-gf target costs one row-major `SubscriptIterator` walk rather than one per element; an arrayed target's shared table is pinned as `to[1,...]` because a bare arrayed reference resolves each iterated element's own table offset, which the VM reads as NaN past `table_count`. `compose_with_lookup_polarity` (in `src/ltm/polarity.rs`) is the polarity twin, mirroring `apply_implicit_with_lookup`'s placeholder-Positive and zero-point-table rules. The polarity tests live in `src/ltm/with_lookup_tests.rs`, a child module of `ltm::tests`. - **`src/ltm_post.rs`** - Post-simulation relative loop *and link* score computation. `compute_rel_loop_scores(results, loop_partitions)` normalizes each loop's `loop_score` series against the sum of absolute scores within its cycle partition, using SAFEDIV-0 semantics (zero denominator -> zero result). Called after simulation rather than emitted as synthetic equations to avoid O(P^2) equation-text growth on models with dense partitions. `loop_partitions` is an `IndexMap` (re-exported `engine::indexmap`). `compute_rel_loop_scores*` walk its **emission** order rather than re-sorting the loop ids: the partition-sum denominator accumulates `|loop_score|` in that order, and emission order keeps the IEEE-754 (non-associative) sum bit-for-bit identical to the pre-#461 compile-time emitter (GH #468). Emission order is itself deterministic across salsa cache invalidations / processes because `assign_loop_ids` orders loops by a content-derived key (`ltm::graph::loop_id_sort_key`), so it never flaps even though `IndexMap`'s `PartialEq` (salsa cache equality) is order-insensitive. `compute_rel_link_scores(links, step_count)` is the link-level analogue (GH #652): raw link scores divide by the change in the *target*, so they are not comparable across targets and ranking by raw magnitude surfaces numerically-degenerate links (near-constant targets blow up the score). It groups the input `RelLinkInput { to, score }` links by their `to` target and normalizes each link's score by the per-target, per-timestep sum of `|score|` over all that target's *scored* inputs -- a **signed** value in `[-1, 1]` (sign kept like `compute_rel_loop_scores`), with the same `denom_summand` NaN-exclusion / Inf-retention / SAFEDIV-0 semantics. Denominator scope is the scored inputs only: complete in discovery mode (every causal edge scored) but covering just the in-loop subset in exhaustive mode (documented caveat on the fn). libsimlin's `analyze_links_core` calls it over the final (post-synthetic-collapse) link set so the per-target denominator matches the links the caller receives. - **LTM open work**: known LTM bugs and improvements are tracked on GitHub under the `ltm` label; issue #488 is the pinned epic that organises them by area (core algorithm, discovery/post-sim, augmentation, module/array umbrellas). Each open `ltm`-labelled issue carries file:line references and a suggested fix, so a new session can pick a bite-sized piece without re-investigating the subsystem. diff --git a/src/simlin-engine/src/db/ltm/link_scores.rs b/src/simlin-engine/src/db/ltm/link_scores.rs index aa063f706..c1cac98ef 100644 --- a/src/simlin-engine/src/db/ltm/link_scores.rs +++ b/src/simlin-engine/src/db/ltm/link_scores.rs @@ -503,7 +503,7 @@ pub(super) fn try_cross_dimensional_link_scores( // (`SUM(pop[*] * (1 - weight[*]))` w.r.t. `weight` has the // sign-flipping coefficient `-pop[e]`). let (arrayed_dep_dims, model_deps) = - reducer_body_ctx_parts(db, source_vars, project, &classified.body_text); + reducer_body_ctx_parts(db, source_vars, project, &classified.body); let row_dim_names: Vec = from_dims.iter().map(|d| d.name().to_string()).collect(); // The live source's accepted slice, when `to` IS a variable-backed agg // reading `from`: lets the body partial resolve a mismatched-arity @@ -522,7 +522,7 @@ pub(super) fn try_cross_dimensional_link_scores( .find(|a| !a.is_synthetic && a.name == to && a.reads_var(from)) .map(|a| a.source_read_slice(from)); let body_ctx = crate::ltm_augment::ReducerBodyCtx { - body_text: &classified.body_text, + body: &classified.body, live_source: from, arrayed_dep_dims: &arrayed_dep_dims, model_deps: &model_deps, @@ -902,10 +902,10 @@ fn emit_broadcast_reduce_link_scores( // changed-first partial must account for. Mirrors // `try_cross_dimensional_link_scores`' setup. let (arrayed_dep_dims, model_deps) = - reducer_body_ctx_parts(db, source_vars, project, &classified.body_text); + reducer_body_ctx_parts(db, source_vars, project, &classified.body); let row_dim_names: Vec = from_dims.iter().map(|d| d.name().to_string()).collect(); let body_ctx = crate::ltm_augment::ReducerBodyCtx { - body_text: &classified.body_text, + body: &classified.body, live_source: from, arrayed_dep_dims: &arrayed_dep_dims, model_deps: &model_deps, @@ -1247,16 +1247,16 @@ pub(super) fn try_scalar_to_arrayed_link_scores( // the whole stream, which was quadratic in the element count. let slot_occurrences = crate::ltm_augment::SlotOccurrences::new(to_occurrences); - // Build one `LtmSyntheticVar` for `element` from its equation text and - // that text's dependency set. The element name is the only part of the + // Build one `LtmSyntheticVar` for `element` from its equation AST and that + // equation's dependency set. The element name is the only part of the // generated equation/name that varies between elements. // Returns `None` (after surfacing a `Warning`) when the per-element - // ceteris-paribus partial fails to parse (GH #311) -- that element's link + // ceteris-paribus partial cannot be built (GH #311) -- that element's link // score is skipped rather than emitted with a silently non-ceteris-paribus - // equation. The `elem_text.is_empty()` zero is a legitimate no-slot value, - // not a parse failure. + // equation. A `None` `elem_eqn` is the legitimate no-slot case (a target + // hole), scored 0, and is distinct from a failure. let build_var = |element: &str, - elem_text: &str, + elem_eqn: Option<&crate::ast::Expr0>, elem_deps: &HashSet>, deps_to_sub: &HashSet>| -> Option { @@ -1275,11 +1275,9 @@ pub(super) fn try_scalar_to_arrayed_link_scores( emit_ltm_partial_equation_warning(db, model, &name, &err); return None; } - let equation = if elem_text.is_empty() { - "0".to_string() - } else { - // GH #910: `elem_text` is the target's RAW element equation, but an - // implicit WITH-LOOKUP target's compiled value is `gf(elem_text)`. + let equation = if let Some(elem_eqn) = elem_eqn { + // GH #910: `elem_eqn` is the target's RAW element equation, but an + // implicit WITH-LOOKUP target's compiled value is `gf(elem_eqn)`. // Wrap the per-element partial in THIS element's own table (the // per-element-equation case) or the shared one, so the numerator // and the `Δto[e]` denominator live on the same scale. A gf-less @@ -1294,7 +1292,7 @@ pub(super) fn try_scalar_to_arrayed_link_scores( // Equation text uses the qualified `dim·element` form (direct // LoadPrev, no helper auxes); the NAME below keeps the bare form. &crate::ltm_augment::qualify_element_csv(element, &to_dims), - elem_text, + elem_eqn, // A true scalar source has no hoisted reducers, so the wrap // holds `from` live as an ident and the post-transform // substitution is a no-op. @@ -1316,6 +1314,10 @@ pub(super) fn try_scalar_to_arrayed_link_scores( return None; } } + } else { + // A target hole: no per-element slot and no default. Zero is the + // right link-score value (no sensitivity). + "0".to_string() }; Some(LtmSyntheticVar { name, @@ -1327,7 +1329,7 @@ pub(super) fn try_scalar_to_arrayed_link_scores( }; // GH #780: `build_var` returns `None` only on a true per-element - // `PartialEquationError` (the empty-slot case returns `Some("0")`). One + // `PartialEquationError` (the no-slot case returns `Some("0")`). One // doomed element dooms the whole edge -- a loop hop through it resolves // (`loop_link_score_ref`) to that element's name, which was never // emitted, so the loop would otherwise stub to a warned constant 0. @@ -1342,11 +1344,11 @@ pub(super) fn try_scalar_to_arrayed_link_scores( // dependency set, and the subset to element-pin are all // element-invariant -- compute them once, outside the loop. Ast::ApplyToAll(_, expr) => { - let elem_text = crate::patch::expr2_to_string(expr); + let elem_eqn = crate::patch::expr2_to_expr0(expr); let elem_deps = crate::variable::identifier_set(ast, target_ast_dims, None); let deps_to_sub = deps_to_subscript(&elem_deps); for element in &elements { - match build_var(element, &elem_text, &elem_deps, &deps_to_sub) { + match build_var(element, Some(&elem_eqn), &elem_deps, &deps_to_sub) { Some(v) => cross_vars.push(v), None => { unscoreable_edges.insert((from.to_string(), to.to_string())); @@ -1362,24 +1364,25 @@ pub(super) fn try_scalar_to_arrayed_link_scores( for element in &elements { let canonical_elem = crate::common::CanonicalElementName::from_raw(element); let slot = per_elem.get(&canonical_elem).or(default_expr.as_ref()); - let (elem_text, elem_deps): (String, HashSet>) = match slot { - Some(expr) => ( - crate::patch::expr2_to_string(expr), - crate::variable::identifier_set( - &Ast::Scalar(expr.clone()), - target_ast_dims, - None, + let (elem_eqn, elem_deps): (Option, HashSet>) = + match slot { + Some(expr) => ( + Some(crate::patch::expr2_to_expr0(expr)), + crate::variable::identifier_set( + &Ast::Scalar(expr.clone()), + target_ast_dims, + None, + ), ), - ), - // No slot and no default: the target has a hole at - // this element. A zero equation is the right - // link-score value (no sensitivity), matching the - // historical placeholder behaviour for un-derivable - // partials. - None => (String::new(), HashSet::new()), - }; + // No slot and no default: the target has a hole at + // this element. A zero equation is the right + // link-score value (no sensitivity), matching the + // historical placeholder behaviour for un-derivable + // partials. + None => (None, HashSet::new()), + }; let deps_to_sub = deps_to_subscript(&elem_deps); - match build_var(element, &elem_text, &elem_deps, &deps_to_sub) { + match build_var(element, elem_eqn.as_ref(), &elem_deps, &deps_to_sub) { Some(v) => cross_vars.push(v), None => { unscoreable_edges.insert((from.to_string(), to.to_string())); @@ -2357,7 +2360,11 @@ fn emit_per_element_link_scores( /// One target element's equation parts: (body text, full dep set, the /// arrayed deps to element-pin). - type ElemEqnParts = (String, HashSet>, HashSet>); + type ElemEqnParts = ( + crate::ast::Expr0, + HashSet>, + HashSet>, + ); let Some(from_sv) = source_vars.get(from) else { return false; @@ -2443,10 +2450,10 @@ fn emit_per_element_link_scores( // Per target element: the body text + dep sets (shared for A2A, // per-slot for Arrayed -- mirroring `try_scalar_to_arrayed_link_scores`). let a2a_parts: Option = if let Ast::ApplyToAll(_, expr) = ast { - let text = crate::patch::expr2_to_string(expr); + let eqn = crate::patch::expr2_to_expr0(expr); let deps = crate::variable::identifier_set(ast, target_ast_dims, None); let to_sub = deps_to_subscript(&deps); - Some((text, deps, to_sub)) + Some((eqn, deps, to_sub)) } else { None }; @@ -2487,14 +2494,14 @@ fn emit_per_element_link_scores( let slot = per_elem.get(&canonical_elem).or(default_expr.as_ref()); match slot { Some(expr) => { - let text = crate::patch::expr2_to_string(expr); + let eqn = crate::patch::expr2_to_expr0(expr); let deps = crate::variable::identifier_set( &Ast::Scalar(expr.clone()), target_ast_dims, None, ); let to_sub = deps_to_subscript(&deps); - Some((text, deps, to_sub)) + Some((eqn, deps, to_sub)) } // No slot, no default: a hole -- this element has no // equation, so the site cannot occur in it; skip. @@ -2503,7 +2510,7 @@ fn emit_per_element_link_scores( } _ => None, }; - let Some((body_text, deps, to_sub)) = a2a_parts.as_ref().or(slot_parts.as_ref()) else { + let Some((body_eqn, deps, to_sub)) = a2a_parts.as_ref().or(slot_parts.as_ref()) else { continue; }; @@ -2532,7 +2539,9 @@ fn emit_per_element_link_scores( db, model, &name, - &crate::ltm_augment::PartialEquationError::new(body_text), + &crate::ltm_augment::PartialEquationError::new(&crate::ast::print_eqn( + body_eqn, + )), ); } return true; @@ -2551,7 +2560,7 @@ fn emit_per_element_link_scores( axes, &row_parts, &qualified_element, - body_text, + body_eqn, deps, to_sub, from_dims, @@ -2594,11 +2603,11 @@ fn reducer_body_ctx_parts( db: &dyn Db, source_vars: &HashMap, project: SourceProject, - body_text: &str, + body: &crate::ast::Expr0, ) -> (HashMap, HashSet) { let mut arrayed_dep_dims: HashMap = HashMap::new(); let mut model_deps: HashSet = HashSet::new(); - for ident in crate::ltm_augment::expr_reference_idents(body_text) { + for ident in crate::ltm_augment::expr_reference_idents(body) { if let Some(sv) = source_vars.get(&ident) { model_deps.insert(ident.clone()); let dims = variable_dimensions(db, *sv, project); @@ -2878,10 +2887,10 @@ pub(super) fn emit_source_to_agg_link_scores( // Linear arm build the true changed-first row partial instead of // asserting ∂agg/∂from[e] = 1. let (arrayed_dep_dims, model_deps) = - reducer_body_ctx_parts(db, source_vars, project, &classified.body_text); + reducer_body_ctx_parts(db, source_vars, project, &classified.body); let row_dim_names: Vec = from_dims.iter().map(|d| d.name().to_string()).collect(); let body_ctx = crate::ltm_augment::ReducerBodyCtx { - body_text: &classified.body_text, + body: &classified.body, live_source: from, arrayed_dep_dims: &arrayed_dep_dims, model_deps: &model_deps, @@ -3455,7 +3464,7 @@ pub(super) fn emit_agg_to_target_link_scores( match crate::ltm_augment::generate_agg_to_scalar_target_equation( &agg.name, to, - &crate::patch::expr2_to_string(expr), + &crate::patch::expr2_to_expr0(expr), &reducer_subst, &all_deps, Some(project_dimensions_context(db, project)), @@ -3485,11 +3494,12 @@ pub(super) fn emit_agg_to_target_link_scores( // One shared body; emit one per-target-element scalar var. Track A // stage 1: the generator runs the wrap on this OWN A2A body and // substitutes reducers -> agg names AFTER the wrap, so we thread the - // un-substituted text + `reducer_subst`. An own-text parse failure - // (unreachable -- the text is a `print_eqn` of a compiled AST) then - // surfaces on the first element's generator `Err` and dooms the edge - // once, via the same GH #661 warning path the per-element loop uses. - let to_own_text = crate::patch::expr2_to_string(expr); + // un-substituted AST + `reducer_subst`. The own-equation parse that + // used to be able to fail here is gone (the generation half lowers + // the `Expr2` straight to `Expr0`); a doomed wrap still surfaces on + // the first element's generator `Err` and dooms the edge once, via + // the same GH #661 warning path the per-element loop uses. + let to_own_eqn = crate::patch::expr2_to_expr0(expr); if to_dims.is_empty() { return; } @@ -3520,7 +3530,7 @@ pub(super) fn emit_agg_to_target_link_scores( to, // Qualified for equation text; the name keeps the bare form. &crate::ltm_augment::qualify_element_csv(element, &to_dims), - &to_own_text, + &to_own_eqn, &reducer_subst, &all_deps, &deps_to_subscript, @@ -3599,7 +3609,7 @@ pub(super) fn emit_agg_to_target_link_scores( to, // Qualified for equation text; the name keeps the bare form. &crate::ltm_augment::qualify_element_csv(element, &to_dims), - &crate::patch::expr2_to_string(slot_expr), + &crate::patch::expr2_to_expr0(slot_expr), &reducer_subst, &slot_deps, &deps_to_subscript, diff --git a/src/simlin-engine/src/db/ltm_char_tests.rs b/src/simlin-engine/src/db/ltm_char_tests.rs index 875fd633f..f4f117405 100644 --- a/src/simlin-engine/src/db/ltm_char_tests.rs +++ b/src/simlin-engine/src/db/ltm_char_tests.rs @@ -1433,8 +1433,8 @@ fn reducer_index_nested_freeze_preserves_loud_failure_not_silent_compile() { // only inside `PREVIOUS(g)` -- it is already lagged. The changed-first partial // holds `from` live and must LEAVE `PREVIOUS(g)` untouched // (`from + PREVIOUS(g)`), NOT re-wrap it to `PREVIOUS(PREVIOUS(g))` (a t-2 -// read). This pins the `already_lagged` selection semantics the stage-2 switch -// must reproduce via `occ.already_lagged`: it suppresses the wrap/freeze of an +// read). This pins the already-lagged selection semantics the wrap reproduces by +// recognizing a `PREVIOUS`/`INIT` node structurally: it suppresses the wrap of an // already-lagged occurrence but not its live selection. // --------------------------------------------------------------------------- diff --git a/src/simlin-engine/src/db/ltm_ir.rs b/src/simlin-engine/src/db/ltm_ir.rs index 6012e1ebb..73fe2a247 100644 --- a/src/simlin-engine/src/db/ltm_ir.rs +++ b/src/simlin-engine/src/db/ltm_ir.rs @@ -399,15 +399,12 @@ impl WalkAccum<'_> { /// `missing_occurrence` guard), whereas absence in the per-edge view /// silently misclassifies the edge as `Bare`. Suppression therefore applies /// to this view only. - #[allow(clippy::too_many_arguments)] fn push_occurrence( &mut self, reference: OccurrenceRef, shape: RefShape, axes: Vec, - target_element: Option<&str>, reducer_keys: &[String], - already_lagged: bool, index_nested: bool, ) { // Inside an unaddressable builtin child, record NO occurrence: its path @@ -421,10 +418,7 @@ impl WalkAccum<'_> { reference, shape, axes, - target_element: target_element.map(|s| s.to_string()), in_reducer: !reducer_keys.is_empty(), - reducer_keys: reducer_keys.to_vec(), - already_lagged, index_nested, }); } @@ -484,7 +478,6 @@ fn collect_all_reference_sites_and_occurrences( None, &mut reducer_keys, false, - false, &mut acc, ); acc.path.pop(); @@ -506,7 +499,6 @@ fn collect_all_reference_sites_and_occurrences( Some(k.as_str()), &mut reducer_keys, false, - false, &mut acc, ); acc.path.pop(); @@ -522,7 +514,6 @@ fn collect_all_reference_sites_and_occurrences( None, &mut reducer_keys, false, - false, &mut acc, ); acc.path.pop(); @@ -650,16 +641,6 @@ pub(crate) fn site_child_index(n: usize) -> Option { u16::try_from(n).ok().filter(|&i| i != UNADDRESSABLE_CHILD) } -/// `true` iff `builtin` is `PREVIOUS(...)` / `INIT(...)`: everything inside is -/// already lagged (read at t-1) or frozen (read at t=0). Used to set the -/// `already_lagged` occurrence marker so the transform does not re-wrap it. -fn builtin_is_previous_or_init(builtin: &crate::builtins::BuiltinFn) -> bool { - matches!( - builtin, - crate::builtins::BuiltinFn::Previous(_, _) | crate::builtins::BuiltinFn::Init(_) - ) -} - /// Recursive helper for [`collect_all_reference_sites_and_occurrences`]: /// left-to-right DFS over an `Expr2` tree, pushing one [`ReferenceSite`] per /// model-variable reference (bucketed by source name) AND one @@ -668,8 +649,7 @@ fn builtin_is_previous_or_init(builtin: &crate::builtins::BuiltinFn) -> bo /// `in_reducer` becomes `true` (via `reducer_keys` non-empty) once we descend /// into a builtin that can route through an aggregate node and stays sticky (a /// reducer nested in another reducer's arg is still inside *a* reducer); `SIZE` -/// does not route through an agg, so it never sets the flag. `already_lagged` -/// becomes sticky-true inside a `PREVIOUS`/`INIT` call; `index_nested` +/// does not route through an agg, so it never sets the flag. `index_nested` /// becomes sticky-true once we descend into a subscript index expression. #[allow(clippy::too_many_arguments)] fn walk_all_in_expr( @@ -678,7 +658,6 @@ fn walk_all_in_expr( lookup_dims: &mut impl FnMut(&str) -> Vec, target_element: Option<&str>, reducer_keys: &mut Vec, - already_lagged: bool, index_nested: bool, acc: &mut WalkAccum, ) { @@ -694,9 +673,7 @@ fn walk_all_in_expr( OccurrenceRef::Variable(ident.as_str().to_string()), RefShape::Bare, Vec::new(), - target_element, reducer_keys, - already_lagged, index_nested, ); } else if let Some((module, port)) = module_output_parts(ident.as_str(), ctx) { @@ -708,9 +685,7 @@ fn walk_all_in_expr( }, RefShape::Bare, Vec::new(), - target_element, reducer_keys, - already_lagged, index_nested, ); } @@ -742,9 +717,7 @@ fn walk_all_in_expr( OccurrenceRef::Variable(ident.as_str().to_string()), shape, axes, - target_element, reducer_keys, - already_lagged, index_nested, ); } else if let Some((module, port)) = module_output_parts(ident.as_str(), ctx) { @@ -779,9 +752,7 @@ fn walk_all_in_expr( }, RefShape::Bare, axes, - target_element, reducer_keys, - already_lagged, index_nested, ); } @@ -814,7 +785,6 @@ fn walk_all_in_expr( lookup_dims, target_element, reducer_keys, - already_lagged, true, acc, ), @@ -826,7 +796,6 @@ fn walk_all_in_expr( lookup_dims, target_element, reducer_keys, - already_lagged, true, acc, ); @@ -838,7 +807,6 @@ fn walk_all_in_expr( lookup_dims, target_element, reducer_keys, - already_lagged, true, acc, ); @@ -856,8 +824,6 @@ fn walk_all_in_expr( if pushed_reducer_key { reducer_keys.push(crate::patch::expr2_to_string(expr)); } - // Contents of a PREVIOUS/INIT call are already lagged/frozen. - let child_lagged = already_lagged || builtin_is_previous_or_init(builtin); // Builtin arity is the one child count not bounded by the AST shape // (`MEAN(a, b, ...)` is variadic), so it is the only place a `SiteId` // element can run out of range. `site_child_index` returns `None` @@ -884,9 +850,7 @@ fn walk_all_in_expr( OccurrenceRef::Variable(id.to_string()), RefShape::Bare, Vec::new(), - target_element, reducer_keys, - child_lagged, index_nested, ); } else if let Some((module, port)) = module_output_parts(id, ctx) { @@ -898,9 +862,7 @@ fn walk_all_in_expr( }, RefShape::Bare, Vec::new(), - target_element, reducer_keys, - child_lagged, index_nested, ); } @@ -911,7 +873,6 @@ fn walk_all_in_expr( lookup_dims, target_element, reducer_keys, - child_lagged, index_nested, acc, ), @@ -939,7 +900,6 @@ fn walk_all_in_expr( lookup_dims, target_element, reducer_keys, - already_lagged, index_nested, acc, ); @@ -953,7 +913,6 @@ fn walk_all_in_expr( lookup_dims, target_element, reducer_keys, - already_lagged, index_nested, acc, ); @@ -965,7 +924,6 @@ fn walk_all_in_expr( lookup_dims, target_element, reducer_keys, - already_lagged, index_nested, acc, ); @@ -980,7 +938,6 @@ fn walk_all_in_expr( lookup_dims, target_element, reducer_keys, - already_lagged, index_nested, acc, ); @@ -1269,41 +1226,27 @@ pub(crate) fn derive_other_dep_verdict( OtherDepVerdict::Collapse } -/// How a per-occurrence reference routes, the occurrence-faithful counterpart -/// of [`SiteRouting`]. Unlike `SiteRouting` (which the edge consumers split -/// into one entry per routed agg), a single syntactic occurrence keeps ONE -/// record carrying ALL the synthetic aggs it routes through (GH #793 nested -/// reducers). The RAW walker shape is preserved on the occurrence (the -/// not-hoisted in-reducer `Wildcard`->`DynamicIndex` reclassification the -/// `ClassifiedSite` builder performs is a per-edge-consumer artifact that -/// discards the reducer-enclosure bit; the occurrence keeps `Wildcard` + -/// `in_reducer` instead). -#[derive(Debug, Clone, PartialEq, Eq, salsa::Update)] -pub(crate) enum OccurrenceRouting { - /// Consumers use the occurrence's own `shape` / `axes`. - Direct, - /// The occurrence's read is carried by one or more synthetic aggregate - /// nodes (its enclosing hoisted reducer(s)). - ThroughAgg { aggs: Vec }, -} - /// One classified reference occurrence over a target equation, the finer /// substrate both LTM consumers project from (edge emission is a per-edge /// dedup of these; the transform selects a live SET by shape and names the -/// first non-index-nested occurrence as the normalizer). Every field maps to a -/// spec §3 requirement, but they are not all consumed by the same code today: -/// the ceteris-paribus wrap reads `site_id` (by path), `reference`, `shape`, -/// `axes`, and `index_nested`; `db::module_link_score_equation` reads -/// `reference` for the document-order module-output pick. `target_element`, -/// `routing`, `in_reducer`, `reducer_keys`, and `already_lagged` are classified -/// facts the wrap does not *need* to read -- it reaches the same decisions -/// structurally (it never descends into a `PREVIOUS`/`INIT` call, so -/// `already_lagged` content is untouched by construction; the reducer-freeze arm -/// asks `subtree_has_live_shape` rather than reading `in_reducer`). They are -/// carried because they are the same walk's output and the remaining GH #965 -/// generation-half work consumes them, and each is pinned at the IR level -/// (`db::ltm_ir_tests`) so a walker regression is caught even with no production -/// reader. Do not add a field with neither a production consumer nor an IR pin. +/// first non-index-nested occurrence as the normalizer). +/// +/// Every field has a reader. The ceteris-paribus wrap reads `site_id` (by path), +/// `reference`, `shape`, `axes`, and `index_nested`; +/// `db::module_link_score_equation` reads `reference` for the document-order +/// module-output pick; the corpus gate reads `in_reducer` to scope its +/// per-occurrence comparison. **Do not add a field without one.** +/// +/// Four fields once rode along here -- `target_element`, `routing`, +/// `reducer_keys`, `already_lagged` -- carried on the theory that the GH #965 +/// generation half would consume them. It consumed none, so they are gone. +/// `routing` was the instructive case: it duplicated the GH #793 +/// enclosing-reducer narrowing the `ClassifiedSite` loop already performs, i.e. a +/// second implementation of one rule inside the IR built to end second +/// implementations. `target_element` and `already_lagged` duplicated facts their +/// consumers already hold (the generators are handed the target element; the wrap +/// sees a `PREVIOUS`/`INIT` node directly, and must in any case account for the +/// freezes it inserts ITSELF, which no field of an occurrence can describe). #[derive(Debug, Clone, PartialEq, Eq, salsa::Update)] pub(crate) struct OccurrenceSite { /// Stable, deterministic occurrence identity within `to`'s equation. @@ -1315,26 +1258,22 @@ pub(crate) struct OccurrenceSite { /// `Wildcard`->`DynamicIndex` reclassification), so the two views agree. pub shape: RefShape, /// Per-index access (the extended `AxisRead` vocabulary). Empty for a bare - /// `Var` / module output. + /// `Var` / module output. Read by the wrap for the other-dep verdict, the + /// literal-element index guard, and the `PerElement` row pinning. pub axes: Vec, - /// `Some(elem)` when the occurrence sits in an `Ast::Arrayed` per-element - /// slot (canonical element / comma-tuple), else `None`. - pub target_element: Option, - /// How the occurrence routes (direct, or through its hoisted reducer aggs). - pub routing: OccurrenceRouting, /// `true` iff the occurrence sits syntactically inside an aggregate-routed - /// reducer (`SUM`/`MEAN`/`MIN`/`MAX`/`STDDEV`/`RANK`). Surfaced explicitly - /// -- the transform's whole-reducer freeze (#517) and bare-feeder decline - /// (#779) need "inside a scalar reducer" even when routing is `Direct`. + /// reducer (`SUM`/`MEAN`/`MIN`/`MAX`/`STDDEV`/`RANK`). Consumed by the corpus + /// gate, which scopes its per-occurrence shape comparison to non-reducer + /// references and asserts both streams agree on reducer context. + /// + /// NOT yet consumed by the transform's GH #779 bare-reducer-feeder decline, + /// which re-derives "inside a scalar reducer" with its own walk: the two + /// decisions use two different reducer SETS + /// (`ltm_agg::reducer_collapses_to_scalar` includes `SIZE` and excludes + /// `RANK`; `builtin_routes_through_agg`, which sets this bit, does the + /// opposite), so consuming it there would flip a bare source inside + /// `RANK(...)` from scored to loudly declined. Tracked as GH #982. pub in_reducer: bool, - /// Canonical printed text of every enclosing hoistable reducer, outermost - /// to innermost (empty when `!in_reducer`). - pub reducer_keys: Vec, - /// `true` iff the occurrence sits inside a `PREVIOUS(...)` / `INIT(...)` - /// call -- already lagged/frozen. The transform must NOT re-wrap it (a - /// double lag reads t-2), though it stays live-selectable (Q3). The edge - /// emitter ignores this. - pub already_lagged: bool, /// `true` iff the occurrence is reachable ONLY through another reference's /// subscript index (`other_arr[from]`). Such an occurrence is excluded /// from live selection, from the normalizer, and from the changed-last @@ -1343,18 +1282,15 @@ pub(crate) struct OccurrenceSite { pub index_nested: bool, } -/// Walker output for one occurrence, before the (agg-dependent) routing is -/// resolved -- the occurrence analogue of [`ReferenceSite`]. `model_ltm_reference_sites` -/// finalizes it into an [`OccurrenceSite`] by attaching [`OccurrenceRouting`]. +/// Walker output for one occurrence -- the occurrence analogue of +/// [`ReferenceSite`]. `model_ltm_reference_sites` moves it into an +/// [`OccurrenceSite`] field for field. struct RawOccurrence { site_id: SiteId, reference: OccurrenceRef, shape: RefShape, axes: Vec, - target_element: Option, in_reducer: bool, - reducer_keys: Vec, - already_lagged: bool, index_nested: bool, } @@ -1483,56 +1419,21 @@ pub(crate) fn model_ltm_reference_sites( }) .unwrap_or_default(); - // The synthetic aggs a `from` reference routes through, narrowed to - // the aggs minted for one of the reference's *enclosing* reducers -- - // the exact per-occurrence `ThroughAgg` decision the `ClassifiedSite` - // loop below makes per raw site (GH #793), reused for the occurrence - // stream's `OccurrenceRouting`. - let matching_aggs_for = |from_name: &str, reducer_keys: &[String]| -> Vec { - synthetic_aggs_in_to - .iter() - .copied() - .filter(|&i| agg_nodes.aggs[i].reads_var(from_name)) - .filter(|&i| { - reducer_keys - .iter() - .any(|k| k == &agg_nodes.aggs[i].equation_text) - }) - .map(AggRef) - .collect() - }; - - // Finalize the per-occurrence view: attach routing (a single occurrence - // keeps ONE record carrying all its matching aggs, unlike the per-edge - // `ClassifiedSite` which duplicates one entry per agg). The RAW walker - // shape is preserved -- the not-hoisted in-reducer `Wildcard`->`DynamicIndex` - // reclassification below is a per-edge-consumer artifact. - let mut occ_sites: Vec = Vec::with_capacity(raw_occurrences.len()); - for occ in raw_occurrences { - let routing = match &occ.reference { - OccurrenceRef::Variable(from) if occ.in_reducer => { - let aggs = matching_aggs_for(from, &occ.reducer_keys); - if aggs.is_empty() { - OccurrenceRouting::Direct - } else { - OccurrenceRouting::ThroughAgg { aggs } - } - } - _ => OccurrenceRouting::Direct, - }; - occ_sites.push(OccurrenceSite { + // Finalize the per-occurrence view. The RAW walker shape is preserved -- + // the not-hoisted in-reducer `Wildcard`->`DynamicIndex` reclassification + // below is a per-edge-consumer artifact, and the occurrence keeps + // `in_reducer` instead. + let occ_sites: Vec = raw_occurrences + .into_iter() + .map(|occ| OccurrenceSite { site_id: occ.site_id, reference: occ.reference, shape: occ.shape, axes: occ.axes, - target_element: occ.target_element, - routing, in_reducer: occ.in_reducer, - reducer_keys: occ.reducer_keys, - already_lagged: occ.already_lagged, index_nested: occ.index_nested, - }); - } + }) + .collect(); if !occ_sites.is_empty() { occurrences.insert(to_name_str.to_string(), occ_sites); } diff --git a/src/simlin-engine/src/db/ltm_ir_tests.rs b/src/simlin-engine/src/db/ltm_ir_tests.rs index f2baf8a23..91490c0d2 100644 --- a/src/simlin-engine/src/db/ltm_ir_tests.rs +++ b/src/simlin-engine/src/db/ltm_ir_tests.rs @@ -900,7 +900,7 @@ mod model_ltm_reference_sites_tests { mod occurrence_ir_tests { use super::*; use crate::datamodel; - use crate::db::ltm_ir::{OccurrenceAxis, OccurrenceRef, OccurrenceRouting, OccurrenceSite}; + use crate::db::ltm_ir::{OccurrenceAxis, OccurrenceRef, OccurrenceSite}; use crate::testutils::{x_aux, x_flow, x_model, x_module, x_stock}; /// Sync `datamodel`, run `model_ltm_reference_sites`, and return the @@ -976,12 +976,12 @@ mod occurrence_ir_tests { } #[test] - fn reducer_enclosure_surfaced_even_when_routing_is_direct() { + fn reducer_enclosure_surfaced_on_an_unhoisted_reducer() { // `z = SUM(pop[idx, *])` with `idx` a scalar: the reducer is NOT hoisted - // (dynamic index), so the occurrence routes `Direct` -- yet it must - // still surface `in_reducer` + `reducer_keys` (the #517/#779 - // "inside a scalar reducer" fact the per-edge `ClassifiedSite` folds - // away by reclassifying Wildcard->DynamicIndex and dropping the bit). + // (dynamic index), yet the occurrence must still surface `in_reducer` + // (the #517/#779 "inside a scalar reducer" fact the per-edge + // `ClassifiedSite` folds away by reclassifying Wildcard->DynamicIndex and + // dropping the bit). let project = TestProject::new("main") .named_dimension("D1", &["a", "b"]) .named_dimension("D2", &["x", "y"]) @@ -998,12 +998,6 @@ mod occurrence_ir_tests { "the occurrence keeps the RAW walker shape, unlike ClassifiedSite" ); assert!(o.in_reducer, "the SUM argument sits inside a reducer"); - assert_eq!(o.reducer_keys.len(), 1, "one enclosing reducer"); - assert_eq!( - o.routing, - OccurrenceRouting::Direct, - "the dynamic-index reducer is not hoisted" - ); // The per-edge view folds the enclosure away: it reclassifies to // DynamicIndex, discarding the reducer-enclosure bit. let sites = ir.sites.get(&("pop".to_string(), "z".to_string())).unwrap(); @@ -1015,53 +1009,38 @@ mod occurrence_ir_tests { } #[test] - fn hoisted_reducer_occurrence_routes_through_agg() { - // `share[Region] = pop / SUM(pop[*])`: the bare numerator is Direct/Bare; - // the SUM's wildcard arg is hoisted, so its occurrence routes ThroughAgg - // while KEEPING the raw Wildcard shape. + fn hoisted_reducer_occurrence_keeps_the_raw_wildcard_shape() { + // `share[Region] = pop / SUM(pop[*])`: the bare numerator is Bare; the + // SUM's wildcard arg is hoisted, and its occurrence KEEPS the raw + // `Wildcard` shape where the per-edge view reclassifies. Routing lives + // exclusively on the per-EDGE `ClassifiedSite` now (the occurrence copy + // duplicated the same GH #793 narrowing), so `ThroughAgg` is asserted + // there. let project = TestProject::new("main") .named_dimension("Region", &["nyc", "boston"]) .array_aux("pop[Region]", "100") .array_aux("share[Region]", "pop / SUM(pop[*])"); - with_ir_and_occ(&project, "share", |_ir, occs| { + with_ir_and_occ(&project, "share", |ir, occs| { let pop = var_occs(occs, "pop"); assert_eq!(pop.len(), 2, "occs: {occs:?}"); let bare = pop.iter().find(|o| !o.in_reducer).expect("bare numerator"); - assert_eq!(bare.routing, OccurrenceRouting::Direct); assert_eq!(bare.shape, RefShape::Bare); let reduced = pop.iter().find(|o| o.in_reducer).expect("SUM argument"); - assert!( - matches!(reduced.routing, OccurrenceRouting::ThroughAgg { .. }), - "a hoisted reducer occurrence routes through its synthetic agg" - ); assert_eq!( reduced.shape, RefShape::Wildcard, "the raw walker shape is preserved on the occurrence" ); - }); - } - - #[test] - fn already_lagged_marks_previous_and_init_contents() { - // `to = from + PREVIOUS(g) + INIT(h)`: `from` is live/unlagged; the - // occurrences inside PREVIOUS and INIT are `already_lagged` (so the - // transform will not re-wrap and double-lag them). - let project = TestProject::new("main") - .scalar_aux("from", "1") - .scalar_aux("g", "2") - .scalar_aux("h", "3") - .scalar_aux("to", "from + PREVIOUS(g) + INIT(h)"); - with_ir_and_occ(&project, "to", |_ir, occs| { - let get = |name: &str| { - var_occs(occs, name) - .first() - .copied() - .unwrap_or_else(|| panic!("no occurrence for {name}: {occs:?}")) - }; - assert!(!get("from").already_lagged, "the bare `from` is not lagged"); - assert!(get("g").already_lagged, "g sits inside PREVIOUS"); - assert!(get("h").already_lagged, "h sits inside INIT"); + let sites = ir + .sites + .get(&("pop".to_string(), "share".to_string())) + .expect("the pop -> share edge"); + assert!( + sites + .iter() + .any(|s| matches!(s.routing, SiteRouting::ThroughAgg { .. })), + "the hoisted reducer read routes through its synthetic agg: {sites:?}" + ); }); } @@ -1377,9 +1356,8 @@ mod occurrence_ir_tests { .iter() .find(|o| matches!(&o.reference, OccurrenceRef::ModuleOutput { .. })) .expect("the module-output occurrence"); - assert_eq!( - module_occ.routing, - OccurrenceRouting::Direct, + assert!( + !module_occ.in_reducer, "the composite read is not inside a reducer" ); } @@ -1762,10 +1740,8 @@ fn suppressed_occurrences_still_record_edge_sites() { super::OccurrenceRef::Variable("pop".to_string()), RefShape::FixedIndex(vec!["nyc".to_string()]), Vec::new(), - None, &[], false, - false, ); } diff --git a/src/simlin-engine/src/ltm_augment.rs b/src/simlin-engine/src/ltm_augment.rs index 4f81cdfd5..e8cedbc7a 100644 --- a/src/simlin-engine/src/ltm_augment.rs +++ b/src/simlin-engine/src/ltm_augment.rs @@ -57,24 +57,25 @@ mod post_transform; pub(crate) use post_transform::per_element_row_for_target; #[cfg(test)] pub(crate) use post_transform::substitute_reducers_in_equation; -use post_transform::{ - PerElementRefCtx, qualify_axis_element, rewrite_per_element_source_refs, - substitute_reducers_in_expr0, -}; +use post_transform::{PerElementRefCtx, qualify_axis_element, substitute_reducers_in_expr0}; /// Context for recognizing GH #511 iterated-dimension source references in /// the partial-equation builder: the live source's declared dimension names -/// (canonical, in declaration order; same length as `source_dim_elements`), +/// (canonical, in declaration order; same length as `source_dim_elements`) and /// the target equation's iterated dimension names (canonical, in the order -/// they appear on `Ast::ApplyToAll`/`Ast::Arrayed`), and a `DimensionsContext` -/// for the AC3.5 mapped-dimension case. `build_partial_equation_shaped` is -/// passed `None` by callers whose live source is a scalar (or an aggregate +/// they appear on `Ast::ApplyToAll`/`Ast::Arrayed`). `build_partial_equation_shaped` +/// is passed `None` by callers whose live source is a scalar (or an aggregate /// node) -- those have no source subscripts, so iterated-dim recognition /// never applies. +/// +/// It used to carry a `DimensionsContext` too, for the AC3.5 mapped-dimension +/// case. That was a duplicate of [`WrapCtx::dims_ctx`] -- production threaded the +/// same context into both -- kept alive solely by the Expr0 per-axis classifiers. +/// With those gone from production the field went with them; the remaining +/// `#[cfg(test)]` classifiers take the one `dims_ctx` explicitly. pub(crate) struct IteratedDimCtx<'a> { pub source_dim_names: &'a [String], pub target_iterated_dims: &'a [String], - pub dim_ctx: Option<&'a crate::dimensions::DimensionsContext>, /// Declared dimensions of the target's NON-LIVE array deps, keyed by /// canonical dep name (GH #526). Threaded by the db-bearing per-shape /// link-score path so the other-dep verdict ([`other_dep_verdict`], via the @@ -87,425 +88,6 @@ pub(crate) struct IteratedDimCtx<'a> { pub dep_dims: Option<&'a HashMap>>, } -/// Recognize an *iterated-dimension* `Expr0` subscript on the *live source* -/// -- one whose indices are exactly the target equation's iterated -/// dimensions, in the position matching the live source's declared -/// dimension order -- the Expr0-AST sibling of -/// `db::ltm_ir::classify_iterated_dim_shape` (GH #511). -/// -/// `live_source[d_0, d_1, ...]` is the iterated-dim case iff: -/// 1. it has exactly one index per source dimension (`indices.len() == -/// ctx.source_dim_names.len()`), and -/// 2. every index `d_i` is a bare `Var` naming a dimension that is one of -/// the target equation's iterated dimensions, *and* -/// 3. for each `i`, `d_i` is either the same name as the source's `i`-th -/// declared dimension `ctx.source_dim_names[i]`, or (when a -/// `DimensionsContext` is available) a dimension that *maps to* it (the -/// AC3.5 mapped-dimension case). -/// -/// When it matches, `wrap_non_matching_in_previous` collapses the subscript -/// to a bare `Var(live_source)` before the live/PREVIOUS dispatch -- it then -/// becomes the live ref (`live_shape == Bare`) or (when `live_shape != Bare`, -/// which shouldn't happen for an edge the IR classified `Bare`) a -/// `PREVIOUS(Var(live_source))` (a `Var` arg, which codegen accepts -- vs -/// the `PREVIOUS(Subscript(...))` the pre-fix code produced, which trips the -/// codegen assertion). The model equation itself is untouched -- only the -/// LTM partial's `Expr0` is normalized -- so simulation still evaluates -/// `live_source[d_i]` correctly: in this slot, `live_source[d_i]` and a bare -/// `live_source` reference inside an apply-to-all-over-the-target's-dims -/// equation pick the same element (the bare ref broadcasts/iterates that -/// dimension). -/// -/// `#[cfg(test)]`: production reads the shape straight off the occurrence IR -/// (an iterated-dim subscript is classified `Bare` there); this Expr0 sibling -/// survives only for the wrap unit tests' occurrence builder and the alignment -/// gate. -#[cfg(test)] -fn is_live_source_iterated_dim_subscript( - indices: &[IndexExpr0], - source_dim_elements: &[Vec], - ctx: Option<&IteratedDimCtx<'_>>, -) -> bool { - let Some(ctx) = ctx else { return false }; - if indices.is_empty() || indices.len() != ctx.source_dim_names.len() { - return false; - } - for (i, idx) in indices.iter().enumerate() { - let d = match idx { - IndexExpr0::Expr(Expr0::Var(name, _)) => canonicalize(name.as_str()).into_owned(), - _ => return false, - }; - if !ctx.target_iterated_dims.iter().any(|t| t == &d) { - return false; - } - if !expr0_iterated_axis_lines_up(&d, i, source_dim_elements, ctx) { - return false; - } - } - true -} - -/// Does iterated-dimension index `d` (canonical) line up with the live -/// source's `i`-th axis -- by name, or through a usable positional-mapping -/// remap? The mapped arm consults the SAME -/// [`crate::ltm_agg::iterated_axis_slot_elements`] / -/// `mapped_element_correspondence` gate the Expr2 classifier -/// (`ltm_agg::classify_axis_access`) uses -- BOTH declaration directions -/// (GH #757), positional mappings only (GH #756) -- so the partial -/// builder's live-shape match and the reference-site IR agree by -/// construction. (No mapping context ⇒ no mapped recognition; the by-name -/// check still applies.) -fn expr0_iterated_axis_lines_up( - d: &str, - i: usize, - source_dim_elements: &[Vec], - ctx: &IteratedDimCtx<'_>, -) -> bool { - let src_name = &ctx.source_dim_names[i]; - if d == src_name.as_str() { - return true; - } - let Some(dim_ctx) = ctx.dim_ctx else { - return false; - }; - let Some(elems) = source_dim_elements.get(i) else { - return false; - }; - crate::ltm_agg::iterated_axis_slot_elements(d, src_name, elems, dim_ctx).is_some() -} - -/// The per-axis [`crate::ltm_agg::AxisRead`] vector of a subscript whose -/// every index is either an iterated-dimension name lined up with the -/// source's axis at that position or a literal element of that axis -/// (position-STRICT, matching the Expr2 side's per-axis -/// `resolve_literal_axis_index`) -- the Expr0 sibling of the -/// `classify_axis_access`-derived classification -/// `db::ltm_ir::classify_iterated_dim_shape` performs, minus the `Reduced` -/// arm (a wildcard/StarRange index returns `None`; direct references never -/// collapse an axis, and the `Wildcard` precedence check already ran). -/// `None` when any index is neither -- the caller falls through to the -/// legacy literal pass / `DynamicIndex`. -/// -/// Unlike the other Expr0 classifiers (now `#[cfg(test)]` since the wrap reads -/// the occurrence IR), this and [`expr0_iterated_axis_lines_up`] survive in -/// PRODUCTION behind a documented stage-3 seam. They run in the post-transform -/// row-pinning lowering ([`post_transform::rewrite_per_element_source_refs`]), -/// which re-classifies the ALREADY-WRAPPED `Expr0` -- a tree the wrap mutated by -/// inserting `PREVIOUS(...)` nodes, so its child-index structure no longer -/// matches the target's ORIGINAL `Expr2` and a `SiteId` computed on that -/// original AST cannot address it. The occurrence IR therefore cannot drive the -/// lowering; retiring these two requires stage 3 (delete the print->reparse -/// round trip so the lowering shares the IR's classification directly). See the -/// `ltm_augment_post_transform` module doc. -/// -/// Wrap-time occurrence TAGGING (attaching each occurrence's classification to -/// the node as the wrap builds it, so the post-transform pass reads a tag -/// instead of re-classifying) was rejected as the seam bridge: the tags the -/// lowering needs sit on source references INSIDE the `PREVIOUS(...)`/`INIT(...)` -/// subtrees the wrap froze, i.e. content the wrap descends past but never -/// re-enters, so a tag-only descent would have to walk exactly the pre-existing -/// PREVIOUS/INIT nodes the single-sourced wrap deliberately does not visit -- -/// resurrecting a second traversal of the same tree. The alternative, a -/// `Loc`-keyed side map from node location to occurrence, is itself a parallel -/// map keyed on a coordinate the wrap rewrites (a spliced `PREVIOUS` node -/// carries no original `Loc`), so it reintroduces exactly the drift surface this -/// track exists to remove. Stage 3's print->reparse deletion, by contrast, -/// makes the lowering read the ONE occurrence IR directly with no second map. -fn classify_expr0_per_element_axes( - indices: &[IndexExpr0], - source_dim_elements: &[Vec], - ctx: &IteratedDimCtx<'_>, -) -> Option> { - use crate::ltm_agg::AxisRead; - if indices.is_empty() - || indices.len() != ctx.source_dim_names.len() - || indices.len() != source_dim_elements.len() - { - return None; - } - let mut axes = Vec::with_capacity(indices.len()); - for (i, idx) in indices.iter().enumerate() { - if let IndexExpr0::Expr(Expr0::Var(name, _)) = idx { - let d = canonicalize(name.as_str()).into_owned(); - if ctx.target_iterated_dims.iter().any(|t| t == &d) { - if !expr0_iterated_axis_lines_up(&d, i, source_dim_elements, ctx) { - return None; - } - axes.push(AxisRead::Iterated { - dim: d, - source_dim: ctx.source_dim_names[i].clone(), - }); - continue; - } - } - // Position-strict literal resolution: the Expr2 classifier resolves - // each index against ITS axis only, so the any-dimension fallback - // `resolve_literal_element_index` carries (for the legacy - // FixedIndex match) must not apply here -- a cross-axis literal - // would build a `Pinned` the IR never minted, breaking the - // live-shape equality match. - let candidate = match idx { - IndexExpr0::Expr(Expr0::Var(name, _)) => canonicalize(name.as_str()).into_owned(), - IndexExpr0::Expr(Expr0::Const(s, _, _)) => s.parse::().ok()?.to_string(), - _ => return None, - }; - if !source_dim_elements[i].iter().any(|e| e == &candidate) { - return None; - } - axes.push(AxisRead::Pinned(candidate)); - } - Some(axes) -} - -/// Build the per-axis [`OccurrenceAxis`] classification of an iterated-dimension -/// subscript on a *non-live-source* dependency (e.g. `pop[Region,Age]` in -/// `growth[Region,Age] = row_sum[Region] * c * pop[Region,Age]`), the way -/// `db::ltm_ir::classify_occurrence_axes` classifies it on the `Expr2` AST. -/// -/// This is the Expr0 sibling that lets the `#[cfg(test)]` occurrence builder -/// ([`build_wrap_test_occurrences`]) reconstruct an occurrence's `axes` for the -/// wrap unit tests. PRODUCTION reads the axes straight off the occurrence IR -/// ([`OccurrenceLookup`]) and feeds them to the single-sourced -/// [`derive_other_dep_verdict`] via [`other_dep_verdict`] -- there is no Expr0 -/// re-derivation of the verdict on the live path, so the two families cannot -/// drift. -/// -/// Each index that names (or positionally maps to) the dep's declared axis at -/// that position is `Iterated`, a target-iterated name that does NOT line up is -/// `MismatchedIterated` (the GH #526 case a bare collapse must not silently -/// freeze), an over-dep-arity target-iterated name is `Iterated{d,d}` (the -/// verdict's arity guard dominates it), and any non-target-iterated-Var index is -/// `Dynamic` (which makes `derive_other_dep_verdict` return `NotIterated`, -/// matching the pre-flip recognizer's short-circuit). -#[cfg(test)] -fn other_dep_occurrence_axes( - dep: &Ident, - indices: &[IndexExpr0], - ctx: Option<&IteratedDimCtx<'_>>, -) -> Vec { - let Some(ctx) = ctx else { - return Vec::new(); - }; - let dep_dims = ctx.dep_dims.and_then(|m| m.get(dep.as_str())); - indices - .iter() - .enumerate() - .map(|(i, idx)| { - let IndexExpr0::Expr(Expr0::Var(name, _)) = idx else { - return OccurrenceAxis::Dynamic; - }; - let d = canonicalize(name.as_str()).into_owned(); - if !ctx.target_iterated_dims.iter().any(|t| t == &d) { - return OccurrenceAxis::Dynamic; - } - match dep_dims.and_then(|dd| dd.get(i)) { - Some(dep_dim) if other_dep_axis_lines_up(&d, dep_dim, ctx) => { - OccurrenceAxis::Iterated { - dim: d, - source_dim: canonicalize(dep_dim.name()).into_owned(), - } - } - Some(_) => OccurrenceAxis::MismatchedIterated { dim: d }, - None => OccurrenceAxis::Iterated { - dim: d.clone(), - source_dim: d, - }, - } - }) - .collect() -} - -/// Does iterated-dimension index `d` (canonical) line up with a non-live -/// dep's declared axis `dep_dim` -- by name, or through a usable -/// positional-mapping remap? The dep-side sibling of -/// [`expr0_iterated_axis_lines_up`], consulting the SAME -/// [`crate::ltm_agg::iterated_axis_slot_elements`] / -/// `mapped_element_correspondence` gate (both declaration directions, -/// positional mappings only) so the live-source and other-dep recognizers -/// can never disagree about which mapped pairs are usable. -#[cfg(test)] -fn other_dep_axis_lines_up( - d: &str, - dep_dim: &crate::dimensions::Dimension, - ctx: &IteratedDimCtx<'_>, -) -> bool { - let dep_dim_name = canonicalize(dep_dim.name()); - if d == dep_dim_name.as_ref() { - return true; - } - let Some(dim_ctx) = ctx.dim_ctx else { - return false; - }; - let elems = dimension_element_names(dep_dim); - crate::ltm_agg::iterated_axis_slot_elements(d, dep_dim_name.as_ref(), &elems, dim_ctx).is_some() -} - -/// Whether a single subscript index is a "literal element" reference -- -/// i.e., a `Var` naming a known dimension element or an integer literal. -/// These are dimension references at runtime, not variable references, -/// and must not be PREVIOUS-wrapped even when their textual form -/// collides with a user-variable name. -/// -/// `position` is the index's 0-based position in the subscript; literal -/// `Var` names are matched against the dimension at that position first -/// and then against any dimension as a fallback (mirroring -/// `classify_expr0_subscript_shape`'s match rules). -/// -/// `#[cfg(test)]`: see [`resolve_literal_element_index`]. -#[cfg(test)] -fn is_literal_element_index( - idx: &IndexExpr0, - position: usize, - source_dim_elements: &[Vec], -) -> bool { - resolve_literal_element_index(idx, position, source_dim_elements).is_some() -} - -/// Resolve a single subscript index to a literal element name, mirroring -/// `db::ltm_ir::resolve_literal_index` (the Expr2 sibling) so both -/// classifiers agree on what counts as a "literal element". -/// -/// `#[cfg(test)]`: this Expr0 classifier no longer drives production. The -/// ceteris-paribus wrap consumes the occurrence IR ([`OccurrenceLookup`]) -- the -/// same `db::ltm_ir` classification the edge emitter uses -- so there is ONE -/// classifier family and the historical Expr0/Expr2 drift (which silently zeroed -/// a link score when the wrap re-derived a different shape than the emitter, e.g. -/// GH #759 / GH #913 / the `pop[01]` canonicalization) is structurally -/// impossible. This sibling survives only for the wrap unit tests' occurrence -/// builder ([`build_wrap_test_occurrences`]) and the alignment gate -/// (`ltm_classifier_agreement_tests`), which proves the reparsed-Expr0 walk and -/// the IR's `Expr2` walk stay path- and shape-isomorphic corpus-wide. -/// -/// Element names appear as `Var` nodes; integer literals appear as -/// `Const` nodes whose text is the integer. Either form is validated -/// by membership in `source_dim_elements`. For an indexed dim of size -/// N, `dimension_element_names` produces `["1", "2", ..., "N"]`, so a -/// `Const("999", ...)` over an indexed dim of size 5 won't match and -/// falls through to `None`. Matching prefers the dim at the index's -/// position, falling back to any dim if not found there. -#[cfg(test)] -fn resolve_literal_element_index( - idx: &IndexExpr0, - position: usize, - source_dim_elements: &[Vec], -) -> Option { - let candidate = match idx { - IndexExpr0::Expr(Expr0::Var(name, _)) => canonicalize(name.as_str()).into_owned(), - IndexExpr0::Expr(Expr0::Const(s, _, _)) => { - // Integer literals (only) could be element references for - // indexed dims. Canonicalize via parse-then-format so - // non-canonical forms like `pop[01]` reduce to `"1"` and - // match `dimension_element_names`'s `"1".."N"` output. The - // Expr2 sibling (`db::ltm_ir::resolve_literal_index`) - // does the same; without canonicalization here we'd - // disagree on `01` (Expr2 -> FixedIndex(["1"]), - // Expr0 -> DynamicIndex), the live-shape match would - // fail, and the partial would silently zero. - let n = s.parse::().ok()?; - n.to_string() - } - _ => return None, - }; - let matches_position = position < source_dim_elements.len() - && source_dim_elements[position] - .iter() - .any(|e| e == &candidate); - let matches_any = !matches_position - && source_dim_elements - .iter() - .any(|dim| dim.iter().any(|e| e == &candidate)); - if matches_position || matches_any { - Some(candidate) - } else { - None - } -} - -/// Classify an `Expr0` subscript's shape based on its indices. -/// -/// Mirrors `db::ltm_ir::resolve_literal_index`'s classification logic but at -/// the `Expr0` (parsed-AST) level. `#[cfg(test)]`: the ceteris-paribus wrap no -/// longer re-derives shape on `Expr0` (it reads the occurrence IR); this sibling -/// survives only for the wrap unit tests' occurrence builder and the alignment -/// gate -- see [`resolve_literal_element_index`]. Each input string in -/// `source_dim_elements` is the canonical lowercase element name for the -/// corresponding source dimension, in source-declared order. -/// -/// Rules: -/// - any `IndexExpr0::Wildcard` → `RefShape::Wildcard` -/// - an iterated-dimension subscript on the live source (all axes iterated) -/// → `RefShape::Bare`; a mixed iterated+literal one → `RefShape::PerElement` -/// - all indices are literal element names that match the source's -/// declared elements (or parseable integer literals for indexed -/// dimensions) → `RefShape::FixedIndex(canonical_names)` -/// - otherwise (StarRange, DimPosition, Range, non-literal Expr, or a -/// literal that doesn't match) → `RefShape::DynamicIndex` -/// -/// The literal pass tries each index against the dimension at that position -/// first, then falls back to scanning all dimensions. This keeps the -/// classifier robust when callers pass dimensions in source-declared -/// order but the subscript indices may not align 1:1 with dimension -/// positions in unusual cases. Defensive `DynamicIndex` for unknown -/// names ensures the worst case is over-conservative wrapping rather -/// than incorrectly matching the live shape. -#[cfg(test)] -fn classify_expr0_subscript_shape( - indices: &[IndexExpr0], - source_dim_elements: &[Vec], - iter_ctx: Option<&IteratedDimCtx<'_>>, -) -> RefShape { - if indices - .iter() - .any(|idx| matches!(idx, IndexExpr0::Wildcard(_))) - { - return RefShape::Wildcard; - } - // GH #511: an iterated-dimension subscript on the live source - // (`row_sum[Region]` inside an apply-to-all-over-`Region x Age` equation) - // reads the same source element -- it is `Bare`, mirroring - // `db::ltm_ir::classify_iterated_dim_shape`. Checked before the - // literal-element pass because a dimension name (`Region`) is not a - // literal element, so it would otherwise fall to `DynamicIndex`. - if is_live_source_iterated_dim_subscript(indices, source_dim_elements, iter_ctx) { - return RefShape::Bare; - } - // GH #525 (T6): a mixed iterated+literal subscript (`pop[Region, young]` - // inside an A2A-over-`Region` equation) is `PerElement`, mirroring - // `classify_iterated_dim_shape`'s `classify_axis_access`-derived mixed - // arm -- the partial builder's live-shape match must agree with the - // reference-site IR (the documented sync requirement). All-`Pinned` - // falls through to the literal pass (`FixedIndex`), and all-`Iterated` - // is the `Bare` case above, so this arm fires only for a genuine mix. - if let Some(ctx) = iter_ctx - && let Some(axes) = classify_expr0_per_element_axes(indices, source_dim_elements, ctx) - { - let n_iterated = axes - .iter() - .filter(|a| matches!(a, crate::ltm_agg::AxisRead::Iterated { .. })) - .count(); - if n_iterated > 0 && n_iterated < axes.len() { - return RefShape::PerElement { axes }; - } - } - let mut elems = Vec::with_capacity(indices.len()); - for (i, idx) in indices.iter().enumerate() { - // Use the same resolver as `is_literal_element_index` so this - // classifier and the Expr2 sibling - // (`db::ltm_ir::resolve_literal_index`) agree on what counts - // as a literal element. Integer literals are validated against - // `source_dim_elements` (which contains `["1", ..., "size"]` - // for indexed dims), so out-of-range integers like `pop[999]` - // over a size-5 indexed dim fall through to `DynamicIndex` -- - // matching what the edge emitter sees and avoiding the - // shape-mismatch that would zero out the partial. - match resolve_literal_element_index(idx, i, source_dim_elements) { - Some(elem) => elems.push(elem), - None => return RefShape::DynamicIndex, - } - } - RefShape::FixedIndex(elems) -} - /// Does `name` (case-insensitively) name an array-reducing builtin in the /// form that collapses an array dimension? `SUM`/`STDDEV`/`SIZE`/`RANK` /// reduce at any arity (`RANK(arr, n)`, etc.); `MEAN`/`MIN`/`MAX` reduce an @@ -600,6 +182,12 @@ struct WrapOutcome { /// `build_partial_equation_shaped_with_live_ref` returns /// `UnfreezablePartial`), and the db-bearing emitters turn that into a /// skip-and-warn. + /// + /// The pin-only descents set it for a second, non-desync reason: a source + /// subscript the IR records NOTHING for (a `LOOKUP` table argument) that + /// `post_transform::pin_dimension_name_indices` could not lower by name + /// either. Same contract, same reason -- an un-pinned dimension-name + /// subscript in a scalar fragment is the same silent zero. missing_occurrence: bool, } @@ -639,6 +227,21 @@ struct WrapCtx<'a> { /// occurrence's structural path (tracked as the wrap descends), not a /// re-derivation on the reparsed `Expr0`. occ: &'a OccurrenceLookup<'a>, + /// The `PerElement` row-pinning context (GH #525, T6), `Some` only for + /// [`generate_per_element_link_equation`]. + /// + /// When set, the wrap ALSO lowers each live-source reference to its concrete + /// per-element subscript as it goes. That used to be a separate pass over + /// the WRAPPED tree, which had to re-derive each occurrence's per-axis + /// access with an Expr0 classifier because a `SiteId` computed on the + /// original AST cannot address a tree the wrap has inserted `PREVIOUS` nodes + /// into. Folding it into the wrap deletes that classifier: here the + /// occurrence is still reachable by path, and -- decisively -- the wrap is + /// the only place that knows whether it is about to FREEZE the reference, + /// which is what selects the bare-row spelling for the live occurrence and + /// the qualified-row spelling for every other one. See + /// [`post_transform::pin_source_subscript_indices`]. + pin: Option<&'a PerElementRefCtx<'a>>, } /// Append child index `i` to `path`, yielding the child node's structural path. @@ -756,6 +359,7 @@ fn wrap_non_matching_in_previous( ctx: &WrapCtx<'_>, out: &mut WrapOutcome, path: &[u16], + frozen: bool, ) -> Expr0 { // `dims_ctx` is consumed only by `wrap_index_non_matching_in_previous` (via // `ctx`); `source_dim_elements` / `iter_ctx` are no longer read here (the @@ -786,6 +390,18 @@ fn wrap_non_matching_in_previous( Expr0::Var(ref ident, loc) => { let canonical = Ident::new(ident.as_str()); if &canonical == live_source { + // `PerElement` row pinning: a BARE reference to the source (the + // mixed `Bare`+`PerElement` edge's other site) reads the target + // element's projection onto the source's own axes. Pin it FIRST, + // then take the ordinary live/wrap decision on the result -- a + // `PerElement` live shape never matches a bare reference, so the + // pinned subscript is what gets frozen, and a `PREVIOUS` of a + // qualified element subscript is the direct LoadPrev the scalar + // fragment needs. + let expr = match ctx.pin.and_then(post_transform::pin_bare_source_ref) { + Some(indices) => Expr0::Subscript(ident.clone(), indices, loc), + None => expr, + }; // The bare-Var occurrence matches `Bare`. Any other live // shape (FixedIndex / Wildcard / DynamicIndex) doesn't // match a bare reference, so we wrap. @@ -841,7 +457,7 @@ fn wrap_non_matching_in_previous( // (`generate_per_element_link_equation`) emits a SCALAR equation // per (row, target element), so a live-source occurrence is frozen // at `PREVIOUS` and then row-pinned AFTER the wrap - // (`rewrite_per_element_source_refs`) -- collapsing to bare here + // (`post_transform::pin_source_subscript_indices`) -- collapsing to bare here // would discard the subscript that pin needs, leaving an over-arity // `PREVIOUS(pop)` for a positionally-mapped occurrence that fails to // compile and silently zeroes the score. Skip the collapse so the @@ -852,7 +468,13 @@ fn wrap_non_matching_in_previous( if !matches!(live_shape, RefShape::PerElement { .. }) && node_shape == Some(&RefShape::Bare) { - return wrap_non_matching_in_previous(Expr0::Var(ident, loc), ctx, out, path); + return wrap_non_matching_in_previous( + Expr0::Var(ident, loc), + ctx, + out, + path, + frozen, + ); } } else if other_deps.contains(&canonical) && !matches!(live_shape, RefShape::PerElement { .. }) @@ -874,6 +496,7 @@ fn wrap_non_matching_in_previous( ctx, out, path, + frozen, ); } OtherDepVerdict::Mismatch => { @@ -909,24 +532,54 @@ fn wrap_non_matching_in_previous( // Without the per-index split, DynamicIndex live refs would skip // wrapping inner deps and the partial would no longer be // ceteris-paribus. - let occ_axes = node_occ.map(|o| o.axes.as_slice()).unwrap_or(&[]); - let indices: Vec = indices - .into_iter() - .enumerate() - .map(|(i, idx)| { - if matches!(occ_axes.get(i), Some(OccurrenceAxis::Pinned(_))) { - idx - } else { + // + // `PerElement` row pinning takes over entirely: the occurrence's + // axes are all `Iterated`/`Pinned` by construction (that IS the + // shape), so there is no dynamic index to recurse into, and the + // indices become this `(site, element)` instantiation's row -- + // spelled BARE here because `frozen` is false, i.e. the wrap is + // leaving this occurrence live. That bare-vs-qualified choice is + // the one thing only the wrap can decide, and it is why the + // pinning lives here rather than in a pass over the wrapped tree. + let indices: Vec = match ctx.pin { + Some(pin_ctx) => post_transform::pin_source_subscript_indices( + indices, + node_occ, + pin_ctx, + !frozen, + |i, idx| { wrap_index_non_matching_in_previous( idx, ctx, out, - false, + true, &child_path(path, i), + frozen, ) - } - }) - .collect(); + }, + ), + None => { + let occ_axes = node_occ.map(|o| o.axes.as_slice()).unwrap_or(&[]); + indices + .into_iter() + .enumerate() + .map(|(i, idx)| { + if matches!(occ_axes.get(i), Some(OccurrenceAxis::Pinned(_))) { + idx + } else { + wrap_index_non_matching_in_previous( + idx, + ctx, + out, + false, + &child_path(path, i), + frozen, + ) + } + }) + .collect() + } + }; let subscript = Expr0::Subscript(ident, indices, loc); if out.live_ref.is_none() { out.live_ref = Some(subscript.clone()); @@ -936,56 +589,57 @@ fn wrap_non_matching_in_previous( // Non-live reference: recurse into indices so any nested // user-variable references get wrapped, then build the new // subscript. If the outer ident is itself a dep, wrap the - // whole thing. - // - // EXCEPTION (Track A stage 1, `PerElement` path): for a FROZEN - // occurrence of the LIVE SOURCE ITSELF, suppress only the wrap's - // generic element-index QUALIFICATION -- NOT the whole index pass. - // The post-transform row-pinning lowering - // (`rewrite_per_element_source_refs`) re-derives and qualifies every - // source-axis index from the source's KNOWN declared dims (one - // consistent, ambiguity-free owner), so letting the wrap's - // `qualify_element_index` touch a literal element first would - // HALF-qualify an AMBIGUOUS one (a name declared by several dims, - // like C-LEARN's region elements: in `pop[boston, old]`, `old` -> - // `age·old` but the ambiguous `boston` stays bare because - // `dimension_uniquely_containing_element` declines it). The - // lowering's per-axis classifier then cannot classify the resulting - // half-qualified subscript (a `dim·elem` index no longer matches the - // bare source-element list), so the ambiguous index is never - // re-qualified -- diverging from HEAD's `region·boston` (whose - // pre-wrap rewrite qualified via the source axis it knew). Leaving - // literal indices bare keeps the subscript classifiable so the - // lowering qualifies all of them from `from_dims`. + // whole thing -- decided FIRST, because a wrapped subscript's + // indices are inside a frozen subtree and the `PerElement` pinning + // spells those qualified. + let will_wrap = &canonical == live_source || other_deps.contains(&canonical); + let child_frozen = frozen || will_wrap; + // `PerElement` row pinning for a FROZEN occurrence of the LIVE + // SOURCE ITSELF: rewrite every describable axis to ITS OWN row for + // this target element, QUALIFIED (`region·boston`), so the freeze + // compiles to a direct LoadPrev. Qualification has to come from the + // SOURCE's declared dims, which is why the wrap's own generic + // `qualify_element_index` is suppressed here (`skip` below): that + // helper only qualifies a name exactly one PROJECT dimension + // declares, so on an AMBIGUOUS element (a name several dims declare, + // like C-LEARN's regions) it would leave `pop[boston, age·old]` + // half-qualified. The pinning knows the owner axis for every index + // and qualifies all of them consistently. // - // A genuinely-DYNAMIC index (a non-element, non-dim-name expression - // like `idx` in `pop[Region, idx]`) is still recursed and wrapped: - // it is real ceteris-paribus content, and HEAD's rewrite-then-wrap - // composition wrapped it too (its pre-transform rewrite pinned only - // the iterated axis, leaving `idx` for the wrap's index pass to lag). - // Suppressing the ENTIRE index pass -- as a blanket "keep the indices - // pristine" would -- silently drops that `PREVIOUS(idx)` lag, changing - // both the emitted text and the compiled score series. So the skip is - // scoped to qualification only; the lowering, which leaves an - // unresolvable dynamic index untouched, then preserves the - // `PREVIOUS(idx)` the wrap produced. + // A genuinely-DYNAMIC index (`idx` in `pop[Region, idx]`) is not + // describable per axis, so the pinning hands it back to the wrap's + // index pass and it still gets its `PREVIOUS(idx)` lag. Suppressing + // the whole index pass instead would silently drop that lag, changing + // both the emitted text and the compiled score series. let skip_index_qualification = &canonical == live_source && matches!(live_shape, RefShape::PerElement { .. }); - let indices: Vec = indices - .into_iter() - .enumerate() - .map(|(i, idx)| { - wrap_index_non_matching_in_previous( - idx, - ctx, - out, - skip_index_qualification, - &child_path(path, i), - ) - }) - .collect(); + let recurse_index = |i: usize, idx: IndexExpr0, out: &mut WrapOutcome| { + wrap_index_non_matching_in_previous( + idx, + ctx, + out, + skip_index_qualification, + &child_path(path, i), + child_frozen, + ) + }; + let indices: Vec = match ctx.pin.filter(|_| &canonical == live_source) { + Some(pin_ctx) => post_transform::pin_source_subscript_indices( + indices, + node_occ, + pin_ctx, + // Frozen: never the live reference, so never the bare row. + false, + |i, idx| recurse_index(i, idx, out), + ), + None => indices + .into_iter() + .enumerate() + .map(|(i, idx)| recurse_index(i, idx, out)) + .collect(), + }; let subscript = Expr0::Subscript(ident, indices, loc); - if &canonical == live_source || other_deps.contains(&canonical) { + if will_wrap { Expr0::App( UntypedBuiltinFn("PREVIOUS".to_string(), vec![subscript]), loc, @@ -1005,7 +659,28 @@ fn wrap_non_matching_in_previous( // SAMPLE-IF-TRUE-heavy models like C-LEARN this was the dominant // helper source). Leave the whole call untouched. if name.eq_ignore_ascii_case("previous") || name.eq_ignore_ascii_case("init") { - return Expr0::App(UntypedBuiltinFn(name, args), loc); + // The wrap adds nothing inside, but the `PerElement` row pinning + // still has to reach the source references in there: an already- + // lagged read is still a read of a concrete element, and leaving + // its dimension-name subscript in a scalar fragment either fails + // to compile or reads the wrong element. The pin-only descent + // does exactly that lowering with the same path cursor and the + // same occurrence IR, and wraps nothing. + let call = Expr0::App(UntypedBuiltinFn(name, args), loc); + return match ctx.pin { + Some(pin_ctx) => post_transform::pin_only_source_refs( + call, + pin_ctx, + ctx.occ, + path, + &mut out.missing_occurrence, + // This call lags (or freezes) everything inside it, + // subscript index reads included, so a runtime table + // index in there is already ceteris-paribus. + true, + ), + None => call, + }; } // A LOOKUP call's first argument names a graphical-function table // (a lookup-only variable, or the WITH-LOOKUP self-reference); it @@ -1029,12 +704,32 @@ fn wrap_non_matching_in_previous( .enumerate() .map(|(i, a)| { if i == 0 { - a + // Static table data, not a causal value read: held + // verbatim by the wrap, but still row-pinned (the + // pin-only descent) so a `PerElement` lowering does + // not leave a dimension-name subscript behind. + match ctx.pin { + Some(pin_ctx) => post_transform::pin_only_source_refs( + a, + pin_ctx, + ctx.occ, + &child_path(path, i), + &mut out.missing_occurrence, + // The wrap holds the table arg VERBATIM, so + // it inherits the enclosing freeze and + // nothing more: a runtime index in a bare + // table argument stays live (declined), the + // same argument inside a frozen subtree is + // already lagged (kept). + frozen, + ), + None => a, + } } else { if !child_is_addressable(i) { out.missing_occurrence = true; } - wrap_non_matching_in_previous(a, ctx, out, &child_path(path, i)) + wrap_non_matching_in_previous(a, ctx, out, &child_path(path, i), frozen) } }) .collect(); @@ -1071,6 +766,25 @@ fn wrap_non_matching_in_previous( .subtree_has_live_shape(path, live_source, live_shape) { let reducer = Expr0::App(UntypedBuiltinFn(name, args), loc); + // Frozen WHOLE, so the wrap never descends -- but the reducer can + // still hold source references the `PerElement` lowering must pin + // (an index-nested occurrence is excluded from + // `subtree_has_live_shape`, so a reducer whose ONLY matching-shape + // occurrence sits in a subscript index freezes whole with that + // occurrence inside it). Pin-only descent, always qualified: it is + // inside a freeze, so nothing in here is the live reference. + let reducer = match ctx.pin { + Some(pin_ctx) => post_transform::pin_only_source_refs( + reducer, + pin_ctx, + ctx.occ, + path, + &mut out.missing_occurrence, + // Frozen whole, one line below. + true, + ), + None => reducer, + }; return Expr0::App(UntypedBuiltinFn("PREVIOUS".to_string(), vec![reducer]), loc); } let args = args @@ -1083,7 +797,7 @@ fn wrap_non_matching_in_previous( if !child_is_addressable(i) { out.missing_occurrence = true; } - wrap_non_matching_in_previous(a, ctx, out, &child_path(path, i)) + wrap_non_matching_in_previous(a, ctx, out, &child_path(path, i), frozen) }) .collect(); Expr0::App(UntypedBuiltinFn(name, args), loc) @@ -1095,6 +809,7 @@ fn wrap_non_matching_in_previous( ctx, out, &child_path(path, 0), + frozen, )), loc, ), @@ -1105,12 +820,14 @@ fn wrap_non_matching_in_previous( ctx, out, &child_path(path, 0), + frozen, )), Box::new(wrap_non_matching_in_previous( *rhs, ctx, out, &child_path(path, 1), + frozen, )), loc, ), @@ -1120,18 +837,21 @@ fn wrap_non_matching_in_previous( ctx, out, &child_path(path, 0), + frozen, )), Box::new(wrap_non_matching_in_previous( *then_expr, ctx, out, &child_path(path, 1), + frozen, )), Box::new(wrap_non_matching_in_previous( *else_expr, ctx, out, &child_path(path, 2), + frozen, )), loc, ), @@ -1188,6 +908,7 @@ fn wrap_index_non_matching_in_previous( out: &mut WrapOutcome, skip_element_qualification: bool, path: &[u16], + frozen: bool, ) -> IndexExpr0 { // Only `dims_ctx` (element/dimension recognition) and `iter_ctx` (the // iterated-dim-name guard) are read directly here; the full wrap context @@ -1267,12 +988,16 @@ fn wrap_index_non_matching_in_previous( // before descending); a `Range`'s two operands are children 0 and 1 of // that, mirroring `walk_all_in_expr`. let result = match index { - IndexExpr0::Expr(e) => { - IndexExpr0::Expr(wrap_non_matching_in_previous(e, ctx, &mut idx_out, path)) - } + IndexExpr0::Expr(e) => IndexExpr0::Expr(wrap_non_matching_in_previous( + e, + ctx, + &mut idx_out, + path, + frozen, + )), IndexExpr0::Range(l, r, loc) => IndexExpr0::Range( - wrap_non_matching_in_previous(l, ctx, &mut idx_out, &child_path(path, 0)), - wrap_non_matching_in_previous(r, ctx, &mut idx_out, &child_path(path, 1)), + wrap_non_matching_in_previous(l, ctx, &mut idx_out, &child_path(path, 0), frozen), + wrap_non_matching_in_previous(r, ctx, &mut idx_out, &child_path(path, 1), frozen), loc, ), other => other, @@ -1461,22 +1186,30 @@ pub(crate) fn build_partial_equation_shaped_with_live_ref( iter_ctx: Option<&IteratedDimCtx<'_>>, dims_ctx: Option<&crate::dimensions::DimensionsContext>, ) -> Result<(String, Option), PartialEquationError> { - // Reconstruct the occurrence IR the production wrap consumes from the raw - // equation text (the production callers get it from `model_ltm_reference_sites`; - // this text-level test entry rebuilds an equivalent stream on the reparsed - // Expr0, using the `#[cfg(test)]` Expr0 classifiers the alignment gate proves - // stay in step with the IR). Slot-0 body, matching `wrap_changed_first_ast`. + // This is a genuine TEXT entry point (the unit tests spell the target + // equation as a string), so the parse happens here, once, rather than inside + // the transform: production hands `wrap_changed_first_ast` an `Expr0` lowered + // straight from the target's `Expr2`. + let Ok(Some(ast)) = Expr0::new(equation_text, LexerType::Equation) else { + return Err(PartialEquationError::new(equation_text)); + }; + // Reconstruct the occurrence IR the production wrap consumes (the production + // callers get it from `model_ltm_reference_sites`; this text-level test entry + // rebuilds an equivalent stream on the parsed Expr0, using the `#[cfg(test)]` + // Expr0 classifiers the alignment gate proves stay in step with the IR). + // Slot-0 body, matching `wrap_changed_first_ast`. let occurrences = build_wrap_test_occurrences( - equation_text, + &ast, live_source, deps, source_dim_elements, iter_ctx, + dims_ctx, ); let slot_occurrences = SlotOccurrences::new(&occurrences); let occ = slot_occurrences.for_slot(0); let (transformed, out) = wrap_changed_first_ast( - equation_text, + &ast, deps, live_source, live_shape, @@ -1484,7 +1217,8 @@ pub(crate) fn build_partial_equation_shaped_with_live_ref( iter_ctx, dims_ctx, &occ, - )?; + None, + ); if out.other_dep_mismatch || out.missing_occurrence { return Err(PartialEquationError::unfreezable(equation_text)); } @@ -1495,30 +1229,46 @@ pub(crate) fn build_partial_equation_shaped_with_live_ref( #[path = "ltm_augment_wrap_test_support.rs"] mod wrap_test_support; #[cfg(test)] -pub(crate) use wrap_test_support::{build_wrap_test_occurrences, test_occurrences_for_var}; +pub(crate) use wrap_test_support::{ + build_wrap_test_occurrences, classify_expr0_subscript_shape, is_literal_element_index, + live_source_occurrence_axis, resolve_literal_element_index, test_occurrences_for_var, +}; /// The shared changed-first transform: filter `deps` down to the -/// other-deps set, parse `equation_text`, and PREVIOUS-wrap via +/// other-deps set and PREVIOUS-wrap `target_expr` via /// [`wrap_non_matching_in_previous`] -- returning the transformed AST (not /// printed text) plus the [`WrapOutcome`] out-channels (the captured live /// reference and the GH #526 mismatch doom flag). The single /// implementation behind both `build_partial_equation_shaped_with_live_ref` /// (which prints it) and [`shaped_guard_form_text`] (which doom-checks the -/// AST before printing), so the two can never drift on dep filtering, -/// parse-failure handling, or the wrap itself. -/// -/// A parse failure (`Err`) or an empty equation (`Ok(None)`) leaves no AST -/// to PREVIOUS-wrap, so the ceteris-paribus partial cannot be built. -/// Returning the input unchanged would silently produce a non-ceteris- -/// paribus "partial" identical to the full equation (link score magnitude -/// == 1); fail loudly instead so the caller skips the variable and warns. +/// AST before printing), so the two can never drift on dep filtering or the +/// wrap itself. +/// +/// `target_expr` is the target equation's own AST, lowered from its `Expr2` +/// by [`crate::patch::expr2_to_expr0`] -- **not** parsed from printed text. +/// Track A's generation half deleted that round trip: `patch::expr2_to_string` +/// IS `print_eqn(expr2_to_expr0(..))`, so parsing its output back was a parse of +/// our own print of the very tree we already had in this shape. Two things +/// follow, and the second is the load-bearing one: +/// +/// 1. There is no parse to fail here, so this is infallible (the caller's +/// `PartialEquationError::Parse` channel now fires only where a genuine +/// source-format text boundary remains -- see +/// [`scalar_or_a2a_target_expr`]). +/// 2. The tree the wrap walks is structurally IDENTICAL to the `Expr2` tree +/// `db::ltm_ir::walk_all_in_expr` computed the occurrence `SiteId`s on, so +/// the wrap's tracked child-index path equals the occurrence's `SiteId` **by +/// construction** rather than by a corpus-proven print/reparse isomorphism. +/// The property that makes the swap byte-neutral is pinned by +/// `classifier_agreement_tests::assert_lowering_matches_reparse`. +/// /// `live_reducer_text` (Track A stage 1) opts the wrap into holding a /// designated hoisted reducer subexpression LIVE verbatim (matched by /// canonical text) instead of an ident -- see [`WrapCtx::live_reducer_text`]. /// `None` is the ordinary ident-live behavior; every non-agg caller passes it. #[allow(clippy::too_many_arguments)] fn wrap_changed_first_ast( - equation_text: &str, + target_expr: &Expr0, deps: &HashSet>, live_source: &Ident, live_shape: &RefShape, @@ -1526,16 +1276,15 @@ fn wrap_changed_first_ast( iter_ctx: Option<&IteratedDimCtx<'_>>, dims_ctx: Option<&crate::dimensions::DimensionsContext>, occ: &OccurrenceLookup<'_>, -) -> Result<(Expr0, WrapOutcome), PartialEquationError> { + pin: Option<&PerElementRefCtx<'_>>, +) -> (Expr0, WrapOutcome) { let other_deps: HashSet> = deps .iter() .filter(|d| *d != live_source && normalize_module_ref(d) != *live_source) .cloned() .collect(); - let Ok(Some(ast)) = Expr0::new(equation_text, LexerType::Equation) else { - return Err(PartialEquationError::new(equation_text)); - }; + let ast = target_expr.clone(); let ctx = WrapCtx { live_source, @@ -1545,13 +1294,14 @@ fn wrap_changed_first_ast( iter_ctx, dims_ctx, occ, + pin, }; let mut out = WrapOutcome::default(); // The wrap walks the slot's expression from its root; the occurrence // lookup was already rebased to slot-local paths, so the root path is // empty. - let transformed = wrap_non_matching_in_previous(ast, &ctx, &mut out, &[]); - Ok((transformed, out)) + let transformed = wrap_non_matching_in_previous(ast, &ctx, &mut out, &[], false); + (transformed, out) } /// Is `expr` *array-slice-valued* -- does it contain a wildcard/star-range @@ -1940,7 +1690,7 @@ fn wrap_live_shaped_in_previous( /// leaves the partial unwrapped (an ordinary target). #[allow(clippy::too_many_arguments)] // threads the link-score generation context fn shaped_guard_form_text( - equation_text: &str, + target_expr: &Expr0, deps: &HashSet>, from: &Ident, shape: &RefShape, @@ -1958,8 +1708,12 @@ fn shaped_guard_form_text( None => partial, } }; + // The diagnostic text a loud skip names. Printed only on a failure path: + // the transform itself consumes the AST, so the source spelling is needed + // solely to make the warning name the offending equation. + let err_text = || print_eqn(target_expr); let (changed_first, out) = wrap_changed_first_ast( - equation_text, + target_expr, deps, from, shape, @@ -1967,13 +1721,14 @@ fn shaped_guard_form_text( iter_ctx, dims_ctx, occ, - )?; + None, + ); // A walker desync (finding 1): the changed-first partial silently froze the // live reference, and the changed-last dual would read the SAME desynced // occurrence stream, so BOTH conventions are corrupt. Skip loudly rather // than emit either silent zero. if out.missing_occurrence { - return Err(PartialEquationError::unfreezable(equation_text)); + return Err(PartialEquationError::unfreezable(&err_text())); } if !out.other_dep_mismatch && !contains_unfreezable_previous(&changed_first) { let source_ref = source_ref_for_guard( @@ -1990,15 +1745,15 @@ fn shaped_guard_form_text( )); } - // Changed-last fallback: freeze only the live source. Re-parse the - // (already proven parseable) equation text rather than threading the - // pristine AST out of `wrap_changed_first_ast` -- this leg is the rare - // doomed path, and a second cheap parse keeps the shared helper's - // signature identical to `build_partial_equation_shaped_with_live_ref`'s - // needs. - let Ok(Some(ast)) = Expr0::new(equation_text, LexerType::Equation) else { - return Err(PartialEquationError::new(equation_text)); - }; + // Changed-last fallback: freeze only the live source, starting from the + // SAME pristine target AST the changed-first leg wrapped. This used to + // re-parse the equation text here -- justified at the time as a cheap second + // parse on a rare doomed path, which was true of the cost but not of the + // structure: the "cheap re-parse" was reconstructing a tree the caller + // already owned, and it was the only reason this function needed the text at + // all. Taking the AST as the parameter removes both the parse and the + // possibility that the two conventions ever walk different trees. + let ast = target_expr.clone(); // GH #779: decline the BARE-spelled feeder of an un-hoisted multi-source // reducer. When the live source is referenced BARE (unsubscripted) and is @@ -2021,7 +1776,7 @@ fn shaped_guard_form_text( && !source_dim_names.is_empty() && references_bare_source_inside_reducer(&ast, from, false) { - return Err(PartialEquationError::bare_reducer_feeder(equation_text)); + return Err(PartialEquationError::bare_reducer_feeder(&err_text())); } let mut frozen_ref: Option = None; @@ -2029,10 +1784,10 @@ fn shaped_guard_form_text( let Some(frozen) = frozen_ref else { // No matching occurrence: the "frozen" equation would be the // target's own equation, scoring a silent constant 0. - return Err(PartialEquationError::unfreezable(equation_text)); + return Err(PartialEquationError::unfreezable(&err_text())); }; if contains_unfreezable_previous(&changed_last) { - return Err(PartialEquationError::unfreezable(equation_text)); + return Err(PartialEquationError::unfreezable(&err_text())); } let source_ref = source_ref_for_guard( from, @@ -2559,11 +2314,11 @@ fn subscript_idents_in_expr0( /// `$⁚ltm⁚link_score⁚{from}→{to}[{element}]`, mirroring the arrayed->scalar /// `{from}[{elem}]→{to}` convention from `generate_element_to_scalar_equation`. /// -/// `to_elem_eqn_text` is the target's OWN equation text for this element (the -/// hoisted reducers still spelled `SUM(...)`, NOT agg-substituted -- Track A -/// stage 1): the shared A2A body for an `Equation::ApplyToAll` target, or the -/// matching per-element slot's text (or the default slot) for an -/// `Equation::Arrayed` one. `reducer_subst` maps each hoisted reducer's +/// `to_elem_eqn` is the target's OWN equation AST for this element (the hoisted +/// reducers still spelled `SUM(...)`, NOT agg-substituted -- Track A stage 1), +/// lowered straight from its `Expr2` by [`crate::patch::expr2_to_expr0`]: the +/// shared A2A body for an `Equation::ApplyToAll` target, or the matching +/// per-element slot (or the default slot) for an `Equation::Arrayed` one. `reducer_subst` maps each hoisted reducer's /// canonical text to its agg name; it is empty for a true scalar `from`. `to_deps` /// is the full dependency set of that equation (computed with the target's AST /// dimensions so element-name subscripts are not mistaken for variables), plus @@ -2624,7 +2379,7 @@ pub(crate) fn generate_scalar_to_element_equation( from: &str, to: &str, element: &str, - to_elem_eqn_text: &str, + to_elem_eqn: &Expr0, reducer_subst: &HashMap, to_deps: &HashSet>, to_deps_to_subscript: &HashSet>, @@ -2639,8 +2394,8 @@ pub(crate) fn generate_scalar_to_element_equation( let to_q = quote_ident(to); let to_elem = format!("{to_q}[{element}]"); - // Composition inverted (Track A stage 1): `to_elem_eqn_text` is the target - // element's OWN equation (reducers still spelled `SUM(...)`). When `from` + // Composition inverted (Track A stage 1): `to_elem_eqn` is the target + // element's OWN equation AST (reducers still spelled `SUM(...)`). When `from` // is an arrayed/scalar agg, the reducer that became it is held LIVE by the // wrap (matched by text via `reducer_subst`); a true scalar `from` (empty // `reducer_subst`) keeps the ident-live `RefShape::Bare` wrap. Either way @@ -2657,7 +2412,7 @@ pub(crate) fn generate_scalar_to_element_equation( // separate empty-stream path. let live_reducer_text = live_reducer_text_for_agg(reducer_subst, from); let (wrapped, out) = wrap_changed_first_ast( - to_elem_eqn_text, + to_elem_eqn, to_deps, &from_canonical, &RefShape::Bare, @@ -2665,7 +2420,8 @@ pub(crate) fn generate_scalar_to_element_equation( None, dims_ctx, occ, - )?; + None, + ); // Loud degradation over a silent zero (matching `shaped_guard_form_text` / // `build_partial_equation_shaped_with_live_ref`): a walker desync // (`missing_occurrence`) or a GH #526 mismatched other-dep @@ -2674,7 +2430,7 @@ pub(crate) fn generate_scalar_to_element_equation( // `missing_occurrence` cannot fire, and `iter_ctx` is `None`), so this only // affects the scalar-source path. if out.missing_occurrence || out.other_dep_mismatch { - return Err(PartialEquationError::unfreezable(to_elem_eqn_text)); + return Err(PartialEquationError::unfreezable(&print_eqn(to_elem_eqn))); } let partial = print_eqn(&substitute_reducers_in_expr0(wrapped, reducer_subst)); let mut partial = subscript_idents_at_element(&partial, to_deps_to_subscript, element)?; @@ -2708,7 +2464,7 @@ pub(crate) fn generate_scalar_to_element_equation( /// stage 1): the ceteris-paribus wrap runs on the target element's OWN /// equation text with the site's actual `PerElement { site_axes }` shape held /// live, and the row-pinning is a POST-transform lowering -/// ([`rewrite_per_element_source_refs`]) of the wrapped AST. The result: +/// (`post_transform::pin_source_subscript_indices`, run FROM the wrap). The result: /// /// - the live occurrence (held live by the wrap because its shape equals /// `site_axes`) lowered to the concrete row subscript `{from}[{row}]` (a @@ -2739,7 +2495,7 @@ pub(crate) fn generate_per_element_link_equation( site_axes: &[crate::ltm_agg::AxisRead], row_parts_bare: &[String], element_qualified: &str, - to_elem_eqn_text: &str, + to_elem_eqn: &Expr0, to_deps: &HashSet>, to_deps_to_subscript: &HashSet>, from_dims: &[crate::dimensions::Dimension], @@ -2750,40 +2506,38 @@ pub(crate) fn generate_per_element_link_equation( occ: &OccurrenceLookup<'_>, ) -> Result { let from_canonical = Ident::::new(from); - let source_dim_elements: Vec> = - from_dims.iter().map(dimension_element_names).collect(); let source_dim_names: Vec = from_dims.iter().map(|d| d.name().to_string()).collect(); let iter_ctx = IteratedDimCtx { source_dim_names: &source_dim_names, target_iterated_dims, - dim_ctx: Some(dims_ctx), - // The changed-first wrap below runs with this ctx `Some` (the live - // source's `PerElement` shape is recognized against the target's - // iterated dims), but the `PerElement` live shape suppresses the - // GH #526 other-dep collapse entirely (see `wrap_non_matching_in_previous`), - // so the verdict's `dep_dims` are never consulted; none to thread. + // The `PerElement` live shape suppresses the GH #526 other-dep collapse + // entirely (see `wrap_non_matching_in_previous`), so the verdict's + // `dep_dims` are never consulted; none to thread. dep_dims: None, }; let ref_ctx = PerElementRefCtx { from: &from_canonical, site_axes, row_parts_bare, - source_dim_elements: &source_dim_elements, from_dims, target_elem_by_dim, - iter_ctx: &iter_ctx, dim_ctx: dims_ctx, }; - // Invert the composition (Track A stage 1): run the ceteris-paribus wrap - // on the target element's OWN equation, holding the site's ACTUAL - // `PerElement` shape live, and make the row-pinning a POST-transform - // lowering of the already-wrapped AST. Historically this pinned every - // source ref to a concrete row FIRST and then wrapped a synthesized - // `FixedIndex(row)`-shaped *derived* text -- text that no occurrence stream - // describes, so the occurrence IR (which enumerates the target's OWN - // equation) could not drive the wrap. Running the wrap on the own text and - // lowering afterward keeps the byte-identical output while making the wrap - // occurrence-addressable for stage 2. The `PerElement` live shape + // The ceteris-paribus wrap runs on the target element's OWN equation, + // holding the site's ACTUAL `PerElement` shape live, and row-pins each + // source reference AS IT GOES (`ref_ctx`). Two earlier arrangements are + // worth knowing about, because each was wrong in a different way. Pinning + // FIRST and wrapping a synthesized `FixedIndex(row)`-shaped derived text + // (pre-`b7898692`) produced text no occurrence stream describes, so the + // occurrence IR could not drive the wrap at all. Wrapping first and pinning + // AFTER (`b7898692`) fixed that, but the pass then had to re-derive every + // occurrence's per-axis access with an Expr0 classifier, because a `SiteId` + // computed on the original AST cannot address a tree the wrap has inserted + // `PREVIOUS` nodes into. Pinning inside the wrap needs neither: the + // occurrence is reachable by path, and the wrap is the only place that knows + // whether it is about to freeze the reference -- which is exactly what picks + // the bare row for the live occurrence and the qualified row for the rest. + // The `PerElement` live shape // suppresses the GH #526 other-dep collapse (an iterated other-dep like // `w[Age]` keeps its subscript so the post-transform per-element pin can // resolve each dimension-name index -- collapsing to bare would let the @@ -2803,7 +2557,7 @@ pub(crate) fn generate_per_element_link_equation( axes: site_axes.to_vec(), }; let (wrapped, out) = wrap_changed_first_ast( - to_elem_eqn_text, + to_elem_eqn, to_deps, &from_canonical, &live_shape, @@ -2811,17 +2565,12 @@ pub(crate) fn generate_per_element_link_equation( Some(&iter_ctx), Some(dims_ctx), occ, - )?; + Some(&ref_ctx), + ); if out.other_dep_mismatch || out.missing_occurrence { - return Err(PartialEquationError::unfreezable(to_elem_eqn_text)); + return Err(PartialEquationError::unfreezable(&print_eqn(to_elem_eqn))); } - // POST-transform row-pinning lowering: rewrite the wrapped AST's live and - // frozen source occurrences (including those the wrap moved inside - // `PREVIOUS(...)`) to their concrete per-element subscripts -- the live - // occurrence to its bare row (so it re-prints as the historical - // `from[]`), every other occurrence to its own qualified row. - let lowered = rewrite_per_element_source_refs(wrapped, &ref_ctx, false); - let partial = print_eqn(&lowered); + let partial = print_eqn(&wrapped); let mut partial = subscript_idents_at_element(&partial, to_deps_to_subscript, element_qualified)?; if let Some(table_ref) = gf_table_ref { @@ -2843,10 +2592,10 @@ pub(crate) fn generate_per_element_link_equation( /// everything else PREVIOUS. The result is `Equation::Scalar`-shaped text, /// named `$⁚ltm⁚link_score⁚{agg}→{to}`. /// -/// Composition inverted (Track A stage 1): `to_own_eqn_text` is `to`'s OWN -/// equation text (hoisted reducers still spelled `SUM(...)`), and +/// Composition inverted (Track A stage 1): `to_own_eqn` is `to`'s OWN +/// equation AST (hoisted reducers still spelled `SUM(...)`), and /// `reducer_subst` maps each hoisted reducer's canonical text to its agg name. -/// The ceteris-paribus wrap runs on that own text with the reducer that became +/// The ceteris-paribus wrap runs on that own AST with the reducer that became /// `agg_name` held LIVE verbatim (matched by text; every co-reducer freezes /// whole via the GH #517 path), and the agg substitution is a POST-transform /// lowering ([`substitute_reducers_in_expr0`]) of the wrapped AST -- the @@ -2877,7 +2626,7 @@ pub(crate) fn generate_per_element_link_equation( pub(crate) fn generate_agg_to_scalar_target_equation( agg_name: &str, to_name: &str, - to_own_eqn_text: &str, + to_own_eqn: &Expr0, reducer_subst: &HashMap, to_deps: &HashSet>, dims_ctx: Option<&crate::dimensions::DimensionsContext>, @@ -2894,7 +2643,7 @@ pub(crate) fn generate_agg_to_scalar_target_equation( // is a scalar -- referenced bare, no iterated-dim context. let live_reducer_text = live_reducer_text_for_agg(reducer_subst, agg_name); let (wrapped, _out) = wrap_changed_first_ast( - to_own_eqn_text, + to_own_eqn, to_deps, &agg_canonical, &RefShape::Bare, @@ -2902,7 +2651,8 @@ pub(crate) fn generate_agg_to_scalar_target_equation( None, dims_ctx, occ, - )?; + None, + ); let mut partial = print_eqn(&substitute_reducers_in_expr0(wrapped, reducer_subst)); if let Some(table_ref) = gf_table_ref { partial = format!("LOOKUP({table_ref}, {partial})"); @@ -3600,7 +3350,6 @@ fn build_arrayed_link_score_equation( let iter_ctx = IteratedDimCtx { source_dim_names, target_iterated_dims: &target_iterated_dims, - dim_ctx, dep_dims, }; // A subscript like `source[m]` where `m` is an element of *`source`'s* @@ -3628,7 +3377,7 @@ fn build_arrayed_link_score_equation( gf_table_ref: Option<&str>, slot: u16| -> Result { - let elem_eqn_text = crate::patch::expr2_to_string(expr); + let elem_eqn = crate::patch::expr2_to_expr0(expr); // Per-element dependency set: walk *only this slot's* expression // (the union over all elements -- what `identifier_set` on the // whole `Ast::Arrayed` returns -- would over-freeze refs absent @@ -3646,7 +3395,7 @@ fn build_arrayed_link_score_equation( .collect(); let occ = slot_occurrences.for_slot(slot); shaped_guard_form_text( - &elem_eqn_text, + &elem_eqn, &deps_e, from, shape, @@ -3703,31 +3452,39 @@ fn build_arrayed_link_score_equation( )) } -/// Extract the equation text of a Scalar/ApplyToAll target's AST. +/// The ceteris-paribus wrap's input AST for a Scalar/ApplyToAll target. /// /// `Ast::Arrayed` targets are routed through /// [`build_arrayed_link_score_equation`] before this is reached, so the /// `Arrayed` AST arm here is dead in practice. /// -/// The `eqn`-text fallbacks (both the `Ast::Arrayed` arm and the no-AST -/// branch) cover the degenerate case where the target failed to lower -- -/// `ast()` is `None`, or it's an `Ast::Arrayed` we didn't intercept -- -/// but its datamodel `eqn` is still a plain scalar string. Returning that -/// raw text gives the link-score guard form *something* to differentiate, -/// which is strictly more useful than a `"0"` partial; the stock-to-flow -/// path has always done this for the same variable shape. A target with no -/// usable scalar equation at all (a stub, or an arrayed `eqn` we can't -/// flatten here) falls through to `"0"` -- the link score then degrades to -/// the historical placeholder rather than producing a parse error. -fn scalar_or_a2a_target_equation_text(target_var: &Variable) -> String { +/// The lowered case ([`crate::patch::expr2_to_expr0`]) is the normal path and +/// involves no text at all -- that is the print->reparse deletion. The +/// `eqn`-TEXT fallbacks (both the `Ast::Arrayed` arm and the no-AST branch) +/// cover the degenerate case where the target failed to lower -- `ast()` is +/// `None`, or it's an `Ast::Arrayed` we didn't intercept -- but its datamodel +/// `eqn` is still a plain scalar string. Parsing that raw text gives the +/// link-score guard form *something* to differentiate, which is strictly more +/// useful than a `"0"` partial; the stock-to-flow path has always done this for +/// the same variable shape. A target with no usable scalar equation at all (a +/// stub, or an arrayed `eqn` we can't flatten here) falls through to `"0"` -- the +/// link score then degrades to the historical placeholder. +/// +/// This fallback is the ONE remaining parse on this path, and it is the +/// legitimate kind: it reads USER-authored `datamodel` source text that no +/// compiled AST exists for, i.e. exactly the "unavoidable source-format +/// boundary" GH #965 carves out, not a re-parse of engine output. A genuine +/// failure there still surfaces as the loud `PartialEquationError::Parse` the +/// db-bearing caller turns into a warned skip. +fn scalar_or_a2a_target_expr(target_var: &Variable) -> Result { use crate::ast::Ast; - if let Some(ast) = target_var.ast() { - match ast { - Ast::Scalar(expr) | Ast::ApplyToAll(_, expr) => crate::patch::expr2_to_string(expr), - _ => scalar_eqn_text_or_zero(target_var), - } - } else { - scalar_eqn_text_or_zero(target_var) + if let Some(Ast::Scalar(expr) | Ast::ApplyToAll(_, expr)) = target_var.ast() { + return Ok(crate::patch::expr2_to_expr0(expr)); + } + let text = scalar_eqn_text_or_zero(target_var); + match Expr0::new(&text, LexerType::Equation) { + Ok(Some(expr)) => Ok(expr), + _ => Err(PartialEquationError::new(&text)), } } @@ -3784,7 +3541,7 @@ fn scalar_or_a2a_target_deps( } /// The target's datamodel `eqn` text when it is a plain `Equation::Scalar`, -/// else `"0"`. See [`scalar_or_a2a_target_equation_text`] for why this +/// else `"0"`. See [`scalar_or_a2a_target_expr`] for why this /// fallback exists (a variable that failed to lower). fn scalar_eqn_text_or_zero(target_var: &Variable) -> String { match target_var { @@ -3847,7 +3604,7 @@ fn generate_auxiliary_to_auxiliary_equation( // "SMTH1(x, 5)") while the AST holds the post-module-expansion form // (e.g., Var("$⁚s⁚0⁚smth1·output")). Using the AST-derived text // ensures the identifiers in the equation match those in `deps`. - let to_equation = scalar_or_a2a_target_equation_text(to_var); + let to_equation = scalar_or_a2a_target_expr(to_var)?; // Dependencies of the 'to' variable, with the target's and source's // dimension/element names filtered out (GH #759). @@ -3859,7 +3616,6 @@ fn generate_auxiliary_to_auxiliary_equation( let iter_ctx = IteratedDimCtx { source_dim_names, target_iterated_dims: &target_iterated_dims, - dim_ctx, dep_dims, }; // A scalar / `Ast::ApplyToAll` target is a single body -- slot 0 of the @@ -4149,11 +3905,11 @@ fn generate_stock_to_flow_equation( ); } - // Get the flow equation text. Prefer the AST when available because - // it handles both Scalar and ApplyToAll (arrayed) equations, whereas + // The flow's own equation AST. Prefer the lowered AST when available + // because it handles both Scalar and ApplyToAll (arrayed) equations, whereas // the raw `eqn` field only covers Scalar. Without this, arrayed flows // fall through to "0" and produce a zero link score. - let flow_equation = scalar_or_a2a_target_equation_text(flow_var); + let flow_equation = scalar_or_a2a_target_expr(flow_var)?; // Dependencies of the flow variable, with the flow's and stock's // dimension/element names filtered out (GH #759). @@ -4165,7 +3921,6 @@ fn generate_stock_to_flow_equation( let iter_ctx = IteratedDimCtx { source_dim_names, target_iterated_dims: &target_iterated_dims, - dim_ctx, dep_dims, }; // Link score formula from LTM paper: |Δxz/Δz| × sign(Δxz/Δx) @@ -4548,7 +4303,10 @@ pub(crate) fn qualify_element_csv( /// The result of [`classify_reducer`]: which array reducer the target's /// equation applies to the source, plus the two pieces of context the /// per-element link-score generators need to build a correct partial. -#[derive(Debug, Clone, PartialEq, Eq)] +/// `Eq` is deliberately absent: [`Expr0`] carries an `f64` on `Const`, so it is +/// `PartialEq` only. No consumer needs total equality here. +#[cfg_attr(feature = "debug-derive", derive(Debug))] +#[derive(Clone, PartialEq)] pub(crate) struct ClassifiedReducer { pub kind: ReducerKind, /// Uppercase function name (e.g. "SUM", "MIN"). @@ -4559,9 +4317,12 @@ pub(crate) struct ClassifiedReducer { /// coefficient to the source (`SUM(pop[*] * scale)`) -- that is what /// `body_text` is for (GH #744). pub is_bare: bool, - /// Canonical printed text of the reducer's array argument (its "body"), - /// e.g. `pop[*] * (1 - weight[*])` for `SUM(pop[*] * (1 - weight[*]))`. - pub body_text: String, + /// The reducer's array argument (its "body") -- e.g. the AST of + /// `pop[*] * (1 - weight[*])` for `SUM(pop[*] * (1 - weight[*]))`, lowered + /// straight from the target's `Expr2` by [`crate::patch::expr2_to_expr0`]. + /// It used to be that AST's printed TEXT, which every consumer immediately + /// parsed back -- a parse of our own print of a tree we already held. + pub body: Expr0, } /// Examine the target variable's Expr2 AST to find the array-reducing function @@ -4572,7 +4333,7 @@ pub(crate) struct ClassifiedReducer { /// variable (identified by canonical name). Returns the [`ClassifiedReducer`]: /// the `ReducerKind`, the uppercase function name (e.g., "SUM", "MIN"), /// whether the reducer is the top-level expression (`is_bare`), and the -/// reducer argument's canonical text (`body_text`). +/// reducer argument's AST (`body`). /// /// When `is_bare` is false, the reducer is nested inside other arithmetic /// (e.g., `2 * SUM(population[*])`). Callers should fall back to the @@ -4580,7 +4341,7 @@ pub(crate) struct ClassifiedReducer { /// ignores the surrounding arithmetic and produces wrong link scores. /// Arithmetic INSIDE the reducer argument is a separate concern: the linear /// shortcut is exact only for a bare-source body, so callers supply the -/// generators a [`ReducerBodyCtx`] built from `body_text` and the body-aware +/// generators a [`ReducerBodyCtx`] built from `body` and the body-aware /// partial handles non-unit coefficients (GH #744). /// /// Returns `None` if no reducing builtin is found for the given source. @@ -4620,14 +4381,14 @@ fn classify_reducer_in_expr( Expr2::App(builtin, _, _) => { // Check if this builtin is a reducer whose argument references // the source variable. - if let Some((kind, name, body_text)) = + if let Some((kind, name, body)) = classify_builtin_if_references_source(builtin, source_ident) { return Some(ClassifiedReducer { kind, name, is_bare: is_top_level, - body_text, + body, }); } // Even if this particular App node isn't the reducer we want, @@ -4674,7 +4435,7 @@ fn classify_reducer_in_expr( fn classify_builtin_if_references_source( builtin: &crate::builtins::BuiltinFn, source_ident: &str, -) -> Option<(ReducerKind, &'static str, String)> { +) -> Option<(ReducerKind, &'static str, Expr0)> { use crate::builtins::BuiltinFn; let kind = crate::ltm_agg::reducer_kind(builtin)?; @@ -4701,7 +4462,7 @@ fn classify_builtin_if_references_source( if !expr_references_var(array_arg, canonical_source.as_ref()) { return None; } - Some((kind, upper, crate::patch::expr2_to_string(array_arg))) + Some((kind, upper, crate::patch::expr2_to_expr0(array_arg))) } /// Check if an Expr2 references a variable with the given canonical name, @@ -4734,17 +4495,15 @@ fn expr_references_var(expr: &crate::ast::Expr2, canonical_name: &str) -> bool { } } -/// Canonical head identifiers of every `Var`/`Subscript` reference in -/// `equation_text`, recursing into subscript index expressions. Function -/// names are not collected (they are `App` nodes, not `Var`s); subscript -/// *index* identifiers (dimension and element names) ARE collected, so -/// callers must intersect the result with the model-variable map before -/// treating an entry as a variable reference. Returns an empty set when the -/// text does not parse. +/// Canonical head identifiers of every `Var`/`Subscript` reference in `expr`, +/// recursing into subscript index expressions. Function names are not collected +/// (they are `App` nodes, not `Var`s); subscript *index* identifiers (dimension +/// and element names) ARE collected, so callers must intersect the result with +/// the model-variable map before treating an entry as a variable reference. /// /// Used by the link-score emitters to discover which of a reducer body's /// references are arrayed model variables (the [`ReducerBodyCtx`] inputs). -pub(crate) fn expr_reference_idents(equation_text: &str) -> HashSet { +pub(crate) fn expr_reference_idents(expr: &Expr0) -> HashSet { fn walk(expr: &Expr0, out: &mut HashSet) { match expr { Expr0::Const(..) => {} @@ -4784,9 +4543,7 @@ pub(crate) fn expr_reference_idents(equation_text: &str) -> HashSet { } } let mut out = HashSet::new(); - if let Ok(Some(ast)) = Expr0::new(equation_text, LexerType::Equation) { - walk(&ast, &mut out); - } + walk(expr, &mut out); out } @@ -4799,9 +4556,8 @@ pub(crate) fn expr_reference_idents(equation_text: &str) -> HashSet { /// hoisted `$⁚ltm⁚agg⁚{n}`, `try_cross_dimensional_link_scores` for a /// variable-backed whole-RHS reducer); all names are canonical. pub(crate) struct ReducerBodyCtx<'a> { - /// The reducer's array argument, canonical text (from - /// [`ClassifiedReducer::body_text`]). - pub body_text: &'a str, + /// The reducer's array argument AST (from [`ClassifiedReducer::body`]). + pub body: &'a Expr0, /// The live source variable (the row whose partial is being built). pub live_source: &'a str, /// Declared dimension count for every ARRAYED model variable referenced @@ -5253,9 +5009,7 @@ fn pinned_body_row_terms( current_element: &str, all_elements: &[String], ) -> Option { - let Ok(Some(ast)) = Expr0::new(ctx.body_text, LexerType::Equation) else { - return None; - }; + let ast = ctx.body.clone(); let row_parts_of = |elem: &str| -> Vec { elem.split(',').map(|p| p.trim().to_string()).collect() }; let current_parts = row_parts_of(current_element); @@ -5387,9 +5141,7 @@ fn generate_linear_body_partial( n_elements: usize, reducer_name: &str, ) -> Option { - let Ok(Some(ast)) = Expr0::new(ctx.body_text, LexerType::Equation) else { - return None; - }; + let ast = ctx.body.clone(); let row_parts: Vec = current_element .split(',') .map(|p| p.trim().to_string()) diff --git a/src/simlin-engine/src/ltm_augment_pin_tests.rs b/src/simlin-engine/src/ltm_augment_pin_tests.rs new file mode 100644 index 000000000..bf150fe8c --- /dev/null +++ b/src/simlin-engine/src/ltm_augment_pin_tests.rs @@ -0,0 +1,1263 @@ +// Copyright 2026 The Simlin Authors. All rights reserved. +// Use of this source code is governed by the Apache License, +// Version 2.0, that can be found in the LICENSE file. + +//! Tests for the `PerElement` ROW PINNING -- specifically the three pin-only +//! descents the ceteris-paribus wrap runs where it deliberately stops walking: a +//! pre-existing `PREVIOUS`/`INIT` call, the GH #517 whole-frozen reducer, and a +//! `LOOKUP` table argument. +//! +//! Split out of `ltm_augment_tests.rs` for the per-file line cap, and mounted as a +//! child of its `tests` module, so `use super::*` resolves every helper that file +//! already has in scope (`PinFixture`, `make_named_dimension`, `deps_set`, +//! `build_wrap_test_occurrences`, ...). +//! +//! Each descent has exactly ONE test that fails when that descent is deleted -- +//! the probe matrix is in the branch's commit messages. That property is the point +//! of the file: the regression these commits fix existed because two of the three +//! descents were probed and the third was not, and deleting the un-probed one left +//! the entire 5000-test corpus green. +//! +//! The pinning has TWO obligations and the tests are organized around them: +//! a fragment must COMPILE (no dimension-name subscript may survive into a scalar +//! equation), and it must stay CETERIS PARIBUS (no unattributed live read). The +//! table-argument descent is the only one whose enclosing node does not discharge +//! the second by itself, which is where both defects on this branch landed. + +use super::*; + +/// The per-element row pinning must descend into a subscript RANGE's endpoints, +/// not just its plain expression indices. +/// +/// A source reference can hide in a range bound under another variable's +/// subscript (`other[pop[region]:3]`). The IR walker records an occurrence there +/// (`IndexExpr2::Range` pushes children 0 and 1) and the sibling lowering +/// `substitute_reducers_in_expr0` descends both endpoints -- but the pinning +/// once matched only `IndexExpr0::Expr` and passed a `Range` through untouched. +/// The recorded occurrence was then left un-pinned: its dimension-name subscript +/// survived into the scalar per-element equation, which either fails to compile +/// (a `PREVIOUS`-of-dim-name capture helper) or reads the wrong element. +/// +/// Exercised through [`pin_only_source_refs`], the descent the wrap runs under a +/// subtree it froze -- which is where a range bound under a frozen other-dep +/// lands. +#[test] +fn per_element_pin_descends_into_range_endpoints() { + use crate::dimensions::DimensionsContext; + use crate::ltm_agg::AxisRead; + + let region = make_named_dimension("region", &["nyc", "boston"]); + let from_dims = vec![region.clone()]; + let source_dim_elements = vec![vec!["nyc".to_string(), "boston".to_string()]]; + let source_dim_names = vec!["region".to_string()]; + let target_iterated_dims = vec!["region".to_string()]; + let dim_ctx = DimensionsContext::from( + [datamodel::Dimension::named( + "region".to_string(), + vec!["nyc".to_string(), "boston".to_string()], + )] + .as_slice(), + ); + let iter_ctx = IteratedDimCtx { + source_dim_names: &source_dim_names, + target_iterated_dims: &target_iterated_dims, + dep_dims: None, + }; + let from = Ident::::new("pop"); + let site_axes = vec![AxisRead::Iterated { + dim: "region".to_string(), + source_dim: "region".to_string(), + }]; + let row_parts_bare = vec!["boston".to_string()]; + let mut target_elem_by_dim = HashMap::new(); + target_elem_by_dim.insert("region".to_string(), ("boston".to_string(), 1usize)); + + let ctx = super::post_transform::PerElementRefCtx { + from: &from, + site_axes: &site_axes, + row_parts_bare: &row_parts_bare, + from_dims: &from_dims, + target_elem_by_dim: &target_elem_by_dim, + dim_ctx: &dim_ctx, + }; + + // `pop[region]` sits in the LOWER bound of a range index of `other`. + let ast = Expr0::new("other[pop[region]:3]", LexerType::Equation) + .expect("fixture parses") + .expect("fixture is non-empty"); + // The occurrence stream the pinning reads: `pop[region]` is recorded at the + // range's lower-bound child of `other`'s first index. + let deps = deps_set(&["other"]); + let occurrences = build_wrap_test_occurrences( + &ast, + &from, + &deps, + &source_dim_elements, + Some(&iter_ctx), + Some(&dim_ctx), + ); + let slot_occurrences = SlotOccurrences::new(&occurrences); + let occ = slot_occurrences.for_slot(0); + assert!( + !occ.is_empty(), + "the fixture must record the range-bound occurrence, or the pin has \ + nothing to read and this test passes vacuously" + ); + let mut unlowerable = false; + let lowered = + super::post_transform::pin_only_source_refs(ast, &ctx, &occ, &[], &mut unlowerable, true); + assert!( + !unlowerable, + "the range-bound occurrence IS recorded, so the pin must lower it rather than \ + reporting it unlowerable" + ); + let text = print_eqn(&lowered); + + assert!( + !text.contains("pop[region]"), + "the source reference inside the range bound must be pinned, not left \ + with its dimension-name subscript; got: {text}" + ); + assert!( + text.contains("boston"), + "the range-bound occurrence must be pinned to this instantiation's row; \ + got: {text}" + ); +} + +/// The shared `(Region, Age)` context for the `PerElement` pin-only-descent +/// tests: source `pop[Region, Age]` (`region = [nyc, boston]`, +/// `age = [young, old]`), target `growth[Region]` instantiated at +/// `region\u{B7}boston`, emitting site `pop[Region, young]` (one `Iterated` axis, +/// one `Pinned`). +/// +/// The descents differ only in the EQUATION they walk, so the context lives here +/// once. Spelled out per test it is ~60 lines of identical scaffolding, which is +/// exactly where the one interesting line gets lost. +struct PinFixture { + from: Ident, + from_dims: Vec, + source_dim_elements: Vec>, + source_dim_names: Vec, + target_iterated_dims: Vec, + dim_ctx: crate::dimensions::DimensionsContext, + site_axes: Vec, + row_parts_bare: Vec, + target_elem_by_dim: HashMap, + /// The QUALIFIED target element this instantiation emits for -- `region·boston` + /// for [`PinFixture::new`], `state·ma` for [`PinFixture::mapped`]. + target_element: String, +} + +impl PinFixture { + /// `extra_dims` are project dimensions BEYOND the source's own two: a test + /// needs one to reach the "index names another dimension" arm, and the + /// source's `from_dims` deliberately stay `[region, age]` so such an index + /// is genuinely un-resolvable by name. + fn new(extra_dims: Vec) -> Self { + use crate::ltm_agg::AxisRead; + let mut project_dims = vec![ + datamodel::Dimension::named( + "region".to_string(), + vec!["nyc".to_string(), "boston".to_string()], + ), + datamodel::Dimension::named( + "age".to_string(), + vec!["young".to_string(), "old".to_string()], + ), + ]; + project_dims.extend(extra_dims); + let mut target_elem_by_dim = HashMap::new(); + target_elem_by_dim.insert("region".to_string(), ("boston".to_string(), 1usize)); + PinFixture { + from: Ident::::new("pop"), + from_dims: vec![ + make_named_dimension("region", &["nyc", "boston"]), + make_named_dimension("age", &["young", "old"]), + ], + source_dim_elements: vec![ + vec!["nyc".to_string(), "boston".to_string()], + vec!["young".to_string(), "old".to_string()], + ], + source_dim_names: vec!["region".to_string(), "age".to_string()], + target_iterated_dims: vec!["region".to_string()], + dim_ctx: crate::dimensions::DimensionsContext::from(project_dims.as_slice()), + site_axes: vec![ + AxisRead::Iterated { + dim: "region".to_string(), + source_dim: "region".to_string(), + }, + AxisRead::Pinned("young".to_string()), + ], + row_parts_bare: vec!["boston".to_string(), "young".to_string()], + target_elem_by_dim, + target_element: "region\u{B7}boston".to_string(), + } + } + + /// The MAPPED twin of [`PinFixture::new`]: the same source `pop[Region, Age]`, + /// but the target iterates `State` (`[ny, ma]`) instead of `Region`, with a + /// declared `State`/`Region` dimension mapping, instantiated at `state·ma`. + /// Under the POSITIONAL correspondence that reads source row + /// `[boston, young]` -- `ma` is `State`'s second element and `boston` is + /// `Region`'s -- so a `State`-named index of a `Region` axis is spellable even + /// though the two names differ. + /// + /// `declare_on_state` picks the DECLARATION DIRECTION: `true` declares the + /// mapping on `State` toward `Region`, `false` on `Region` toward `State`. + /// `mapped_element_correspondence` honors both (GH #757), so both must pin. + /// A non-empty `element_map` makes the mapping an EXPLICIT element map, which + /// the correspondence declines (GH #756: the executed A2A lowering resolves + /// positionally and ignores the map, so following it would spell a row the + /// simulation never reads). + fn mapped(declare_on_state: bool, element_map: Vec<(String, String)>) -> Self { + use crate::ltm_agg::AxisRead; + let mut state = datamodel::Dimension::named( + "state".to_string(), + vec!["ny".to_string(), "ma".to_string()], + ); + let mut region = datamodel::Dimension::named( + "region".to_string(), + vec!["nyc".to_string(), "boston".to_string()], + ); + let mapping = |target: &str, element_map: Vec<(String, String)>| { + vec![datamodel::DimensionMapping { + target: target.to_string(), + element_map, + }] + }; + if declare_on_state { + state.mappings = mapping("region", element_map); + } else { + region.mappings = mapping( + "state", + element_map + .into_iter() + .map(|(a, b)| (b, a)) + .collect::>(), + ); + } + let project_dims = vec![ + region, + datamodel::Dimension::named( + "age".to_string(), + vec!["young".to_string(), "old".to_string()], + ), + state, + ]; + let mut target_elem_by_dim = HashMap::new(); + target_elem_by_dim.insert("state".to_string(), ("ma".to_string(), 1usize)); + PinFixture { + from: Ident::::new("pop"), + from_dims: vec![ + make_named_dimension("region", &["nyc", "boston"]), + make_named_dimension("age", &["young", "old"]), + ], + source_dim_elements: vec![ + vec!["nyc".to_string(), "boston".to_string()], + vec!["young".to_string(), "old".to_string()], + ], + source_dim_names: vec!["region".to_string(), "age".to_string()], + target_iterated_dims: vec!["state".to_string()], + dim_ctx: crate::dimensions::DimensionsContext::from(project_dims.as_slice()), + site_axes: vec![ + AxisRead::Iterated { + dim: "state".to_string(), + source_dim: "region".to_string(), + }, + AxisRead::Pinned("young".to_string()), + ], + row_parts_bare: vec!["boston".to_string(), "young".to_string()], + target_elem_by_dim, + target_element: "state\u{B7}ma".to_string(), + } + } + + /// The NAME-COLLISION twin of [`PinFixture::new`]: the source's second axis is + /// `Bucket = [region, old]`, so its element `region` is spelled exactly like the + /// `Region` DIMENSION the target iterates. Source `pop[Region, Bucket]`, target + /// `growth[Region]` at `region·boston`, emitting site `pop[Region, old]`. + /// + /// XMILE lets an element name collide with a dimension name, and + /// `compiler::subscript`'s `normalize_subscripts3` resolves the collision by + /// looking the index up in the AXIS's own elements FIRST (`get_element_index`, + /// "takes priority") and only then as a dimension name. So `pop[Region, region]` + /// reads `Bucket`'s `region` element -- not an iteration over `Region` -- and a + /// pin that reads it the other way either drops the edge or spells a row the + /// simulation never reads. + fn colliding_element() -> Self { + use crate::ltm_agg::AxisRead; + let project_dims = vec![ + datamodel::Dimension::named( + "region".to_string(), + vec!["nyc".to_string(), "boston".to_string()], + ), + datamodel::Dimension::named( + "bucket".to_string(), + vec!["region".to_string(), "old".to_string()], + ), + ]; + let mut target_elem_by_dim = HashMap::new(); + target_elem_by_dim.insert("region".to_string(), ("boston".to_string(), 1usize)); + PinFixture { + from: Ident::::new("pop"), + from_dims: vec![ + make_named_dimension("region", &["nyc", "boston"]), + make_named_dimension("bucket", &["region", "old"]), + ], + source_dim_elements: vec![ + vec!["nyc".to_string(), "boston".to_string()], + vec!["region".to_string(), "old".to_string()], + ], + source_dim_names: vec!["region".to_string(), "bucket".to_string()], + target_iterated_dims: vec!["region".to_string()], + dim_ctx: crate::dimensions::DimensionsContext::from(project_dims.as_slice()), + site_axes: vec![ + AxisRead::Iterated { + dim: "region".to_string(), + source_dim: "region".to_string(), + }, + AxisRead::Pinned("old".to_string()), + ], + row_parts_bare: vec!["boston".to_string(), "old".to_string()], + target_elem_by_dim, + target_element: "region\u{B7}boston".to_string(), + } + } + + fn iter_ctx(&self) -> IteratedDimCtx<'_> { + IteratedDimCtx { + source_dim_names: &self.source_dim_names, + target_iterated_dims: &self.target_iterated_dims, + // The `PerElement` live shape suppresses the GH #526 other-dep + // collapse entirely, so the verdict's `dep_dims` are never consulted. + dep_dims: None, + } + } + + /// Parse `eqn` and build the occurrence stream `db::ltm_ir` would record for + /// it -- the builder mirrors the IR, `LOOKUP` table-argument skip included, + /// so the streams these tests read are the production ones. + fn parse( + &self, + eqn: &str, + deps: &[&str], + ) -> ( + Expr0, + HashSet>, + Vec, + ) { + let deps = deps_set(deps); + let ast = Expr0::new(eqn, LexerType::Equation) + .expect("fixture parses") + .expect("fixture is non-empty"); + let occurrences = build_wrap_test_occurrences( + &ast, + &self.from, + &deps, + &self.source_dim_elements, + Some(&self.iter_ctx()), + Some(&self.dim_ctx), + ); + (ast, deps, occurrences) + } + + /// How many occurrences of the SOURCE the stream records -- every test's + /// non-vacuity check, since a stream that recorded nothing would let a + /// do-nothing pin pass. + fn source_occurrences(occurrences: &[crate::db::ltm_ir::OccurrenceSite]) -> usize { + occurrences + .iter() + .filter( + |o| matches!(&o.reference, crate::db::ltm_ir::OccurrenceRef::Variable(v) if v == "pop"), + ) + .count() + } + + fn generate( + &self, + ast: &Expr0, + deps: &HashSet>, + occ: &OccurrenceLookup<'_>, + ) -> Result { + generate_per_element_link_equation( + "pop", + "growth", + &self.site_axes, + &self.row_parts_bare, + &self.target_element, + ast, + deps, + // Nothing to element-pin by name: the source's pinning is the wrap's job. + &HashSet::new(), + &self.from_dims, + &self.target_elem_by_dim, + &self.target_iterated_dims, + &self.dim_ctx, + None, + occ, + ) + } +} + +/// A `PerElement` source occurrence nested inside a WHOLE-FROZEN array reducer +/// must still be row-pinned. +/// +/// `growth[Region] = pop[Region, young] + SUM(other[pop[Region, young], *])` has +/// two occurrences of the emitting site's shape. The first is the live one. The +/// second sits in a subscript INDEX inside the reducer, and +/// `OccurrenceLookup::subtree_has_live_shape` excludes index-nested occurrences +/// (the GH #517 / Fig. 2 Q4 rule), so the reducer carries no live reference and +/// the wrap freezes it WHOLE without descending. +/// +/// The wrap therefore has to pin that occurrence through its pin-only descent. +/// Left un-pinned, `pop[region, young]` -- a DIMENSION-name subscript -- survives +/// into a scalar link-score fragment, where it needs a `PREVIOUS`-of-dim-name +/// capture helper that cannot compile: the fragment is dropped, the variable +/// keeps a layout slot with no bytecode, and the score reads a constant 0. That +/// is the silent-zero class this track exists to delete, and no char golden +/// reaches this shape -- deleting the descent leaves the whole corpus green. +#[test] +fn per_element_pin_reaches_inside_a_whole_frozen_reducer() { + let fx = PinFixture::new(vec![]); + let (ast, deps, occurrences) = fx.parse( + "pop[Region, young] + SUM(other[pop[Region, young], *])", + &["other"], + ); + // Non-vacuity: the reducer-nested occurrence must actually be recorded, and + // recorded as index-nested -- that bit is what makes the reducer freeze whole. + assert!( + occurrences.iter().any(|o| o.index_nested + && matches!(&o.reference, crate::db::ltm_ir::OccurrenceRef::Variable(v) if v == "pop")), + "the fixture must record an index-nested `pop` occurrence, or the reducer \ + would not freeze whole and this test would not exercise the descent" + ); + let slot_occurrences = SlotOccurrences::new(&occurrences); + let text = fx + .generate(&ast, &deps, &slot_occurrences.for_slot(0)) + .expect("the per-element equation is derivable"); + + assert!( + !text.contains("pop[region, young]"), + "the occurrence inside the whole-frozen reducer kept its DIMENSION-name \ + subscript, which cannot compile in a scalar fragment (a silent zero); \ + got: {text}" + ); + assert!( + text.contains("pop[region\u{B7}boston, age\u{B7}young]"), + "the reducer-nested occurrence must be pinned to this instantiation's row, \ + QUALIFIED so the freeze compiles to a direct LoadPrev; got: {text}" + ); + assert!( + text.contains("pop[boston, young]"), + "the LIVE occurrence must keep the bare row spelling; got: {text}" + ); +} + +/// A `PerElement` source occurrence inside a PRE-EXISTING `PREVIOUS`/`INIT` call +/// must still be row-pinned. +/// +/// `growth[Region] = pop[Region, young] + PREVIOUS(pop[Region, young]) + +/// INIT(pop[Region, old])` -- the wrap deliberately does not descend into an +/// already-lagged or already-frozen call (wrapping its contents again would read +/// two steps back and force a nested-PREVIOUS helper chain), so the row pinning +/// has to reach in through the pin-only descent. Left un-pinned, +/// `pop[region, young]` -- a DIMENSION-name subscript -- survives into a scalar +/// link-score fragment, which cannot compile: the fragment is dropped, the +/// variable keeps a layout slot with no bytecode, and the score reads a constant +/// 0. +/// +/// This descent was the ONE of the three that no test and no char golden reached: +/// deleting it left all 5274 lib tests and all 634 integration tests green. Both +/// call names ride the same arm, so the fixture exercises `PREVIOUS` and `INIT` +/// together. +#[test] +fn per_element_pin_reaches_inside_a_pre_existing_previous_or_init() { + let fx = PinFixture::new(vec![]); + let (ast, deps, occurrences) = fx.parse( + "pop[Region, young] + PREVIOUS(pop[Region, young]) + INIT(pop[Region, old])", + &[], + ); + // Non-vacuity: all three references are recorded, so the descent has real + // occurrences to pin rather than silently doing nothing. + assert_eq!( + PinFixture::source_occurrences(&occurrences), + 3, + "the fixture must record all three `pop` occurrences: {occurrences:?}" + ); + let slot_occurrences = SlotOccurrences::new(&occurrences); + let text = fx + .generate(&ast, &deps, &slot_occurrences.for_slot(0)) + .expect("the per-element equation is derivable"); + + assert!( + !text.contains("pop[region, young]") && !text.contains("pop[region, old]"), + "an occurrence inside a pre-existing PREVIOUS/INIT kept its DIMENSION-name \ + subscript, which cannot compile in a scalar fragment (a silent zero); \ + got: {text}" + ); + assert!( + text.contains("previous(pop[region\u{B7}boston, age\u{B7}young])"), + "the PREVIOUS-nested occurrence must be pinned to this instantiation's row, \ + QUALIFIED so the lagged read compiles to a direct LoadPrev; got: {text}" + ); + assert!( + text.contains("init(pop[region\u{B7}boston, age\u{B7}old])"), + "the INIT-nested occurrence rides the same arm and must be pinned to ITS OWN \ + row for this target element; got: {text}" + ); + assert!( + text.contains("pop[boston, young]"), + "the LIVE occurrence must keep the bare row spelling; got: {text}" + ); +} + +/// A `PerElement` source reference inside a `LOOKUP` **table** argument is +/// row-pinned STRUCTURALLY, and the partial is emitted. +/// +/// `db::ltm_ir` records no occurrence under a table argument +/// (`BuiltinContents::LookupTable(_) => {}` -- "a graphical-function table +/// reference is static data, not a causal edge"). That is right about +/// ATTRIBUTION -- the reference is not a causal edge and earns no score -- but the +/// pin it needs is about COMPILABILITY: left un-pinned, `pop[region, old]` keeps +/// its DIMENSION-name subscript, which cannot resolve in a scalar link-score +/// fragment. The fragment is dropped, the variable keeps a layout slot with no +/// bytecode, and the score reads a constant 0. +/// +/// Both facts hold at once, so `pin_dimension_name_indices` discharges the +/// lowering by NAME (the source's own declared dims) without recording an +/// occurrence or classifying a shape -- the same thing `pin_bare_source_ref` does +/// for a bare `Var`. This is the shape that regressed when the row pinning moved +/// into the wrap (`391bc3c1`): the previous pass over the wrapped tree +/// re-classified with the Expr0 classifier, which needs no occurrence, so it +/// pinned the table argument regardless. +/// +/// The interim fix declined the whole partial LOUDLY instead, which threw away +/// more than the un-scoreable reference: this fixture's FIRST term is a perfectly +/// good scoreable site, and declining dropped every per-element score of the +/// `pop → growth` edge with it. So the assertion here is the pinned form AND the +/// live row's survival. No char golden reaches this shape (none has a `LOOKUP` +/// inside a partial at all), so nothing else catches it. +#[test] +fn per_element_pin_lowers_structurally_inside_a_lookup_table_arg() { + let fx = PinFixture::new(vec![]); + // The table argument reads a DIFFERENT element than the live site does + // (`old` vs `young`), so a pin that merely echoed the site's own row would + // pass vacuously. This is codex's exact shape: a target reading the source + // both normally and in a table argument. + let (ast, deps, occurrences) = fx.parse( + "pop[Region, young] + LOOKUP(pop[Region, old], input)", + &["input"], + ); + // Non-vacuity: the LIVE occurrence outside the LOOKUP is recorded (so the + // stream is not simply empty), while the table-argument one is not. + assert_eq!( + PinFixture::source_occurrences(&occurrences), + 1, + "exactly the non-table occurrence is recorded: {occurrences:?}" + ); + let slot_occurrences = SlotOccurrences::new(&occurrences); + let text = fx + .generate(&ast, &deps, &slot_occurrences.for_slot(0)) + .expect( + "the table-argument reference is lowerable structurally, so the partial must \ + be emitted -- declining it also drops the live site's own score", + ); + + assert!( + !text.contains("pop[region,"), + "the table-argument reference kept its DIMENSION-name subscript, which \ + cannot resolve in a scalar fragment (a silent zero); got: {text}" + ); + assert!( + text.contains("pop[region\u{B7}boston, age\u{B7}old]"), + "the table-argument reference must be pinned to THIS target element's own \ + coordinate for the iterated axis, with its literal element selector \ + qualified by the axis that declares it; got: {text}" + ); + assert!( + text.contains("lookup(pop[region\u{B7}boston, age\u{B7}old]"), + "the pin must land inside the LOOKUP's table argument, which the wrap holds \ + verbatim; got: {text}" + ); + assert!( + text.contains("pop[boston, young]"), + "the LIVE occurrence outside the LOOKUP must keep its bare row spelling -- \ + its score is exactly what the interim loud decline threw away; got: {text}" + ); + assert!( + !text.contains("previous(pop[region\u{B7}boston, age\u{B7}old]"), + "a table argument is static data, never a value read, so the wrap must not \ + freeze it (a PREVIOUS of a table has no value slot); got: {text}" + ); +} + +/// The loud net BEHIND the structural pin: a BARE table argument whose index the +/// rule cannot discharge statically is declined, and the whole partial abandoned. +/// +/// The rule resolves exactly three index forms -- the axis's own dimension name, +/// an element that axis declares, and a numeric literal. Everything else is +/// declined here, for one of two DIFFERENT reasons, and the difference matters +/// because only one of them is unconditional: +/// +/// - a RUNTIME read (`pop[Region, idx]`, `pop[Region, pop[Region, old]]`) COMPILES +/// fine; the problem is ceteris paribus. A BARE `LOOKUP` table argument is not +/// inside a freeze -- the wrap holds it verbatim, since a `PREVIOUS` of a table +/// has no value slot -- so the index stays LIVE, and +/// `codegen::extract_table_info` evaluates it to select the table element. The +/// `young` partial would then vary with the current-step value of `old`, +/// misattributing one row's influence to another. Pinning these purely because +/// it made the fragment COMPILE is what a reviewer caught on this branch. Inside +/// a freeze the same index is already lagged and is KEPT -- see +/// [`per_element_pin_keeps_a_runtime_table_index_inside_a_freeze`]; +/// - the rule having NO ANSWER (`pop[State, old]` on a source declared over +/// `[Region, Age]`) is a compilability verdict and is loud everywhere: resolving +/// it would need to decide that `State` reads `Region` through a positional +/// mapping rather than being a mismatched or transposed axis, and that per-axis +/// CLASSIFICATION is `db::ltm_ir`'s -- which recorded nothing here. +/// +/// Together with [`per_element_pin_lowers_structurally_inside_a_lookup_table_arg`] +/// and the frozen twin, this is the whole contract: statically-resolvable table +/// arguments are pinned and scored, already-lagged runtime ones are pinned as far +/// as they go, and the rest stays loud instead of emitting either an un-pinned +/// dimension-name subscript (a dropped fragment reading 0) or a plausible wrong +/// score. +#[test] +fn per_element_pin_declines_every_non_static_bare_table_arg_index() { + // Each case is `(label, equation, deps, extra project dims)`. `state` is a + // real dimension positionally parallel to `region` -- the shape whose + // resolution would need the mapping classifier. + let cases: [(&str, &str, &[&str], Vec); 3] = [ + ( + "a variable index -- the simplest runtime element read", + "pop[Region, young] + LOOKUP(pop[Region, idx], input)", + &["input", "idx"], + vec![], + ), + ( + "a nested source read selecting the element at runtime", + "pop[Region, young] + LOOKUP(pop[Region, pop[Region, old]], input)", + &["input"], + vec![], + ), + ( + "another dimension's name, which only the IR can classify", + "pop[Region, young] + LOOKUP(pop[State, old], input)", + &["input"], + vec![datamodel::Dimension::named( + "state".to_string(), + vec!["ny".to_string(), "ma".to_string()], + )], + ), + ]; + + for (label, eqn, deps, extra_dims) in cases { + let fx = PinFixture::new(extra_dims); + let (ast, deps, occurrences) = fx.parse(eqn, deps); + // Non-vacuity: the table argument is the reference WITHOUT an occurrence + // (the IR skips it whole), so the decline must come from the structural + // rule rather than from the IR. + assert_eq!( + PinFixture::source_occurrences(&occurrences), + 1, + "{label}: only the live occurrence outside the LOOKUP is recorded: \ + {occurrences:?}" + ); + let slot_occurrences = SlotOccurrences::new(&occurrences); + let err = fx.generate(&ast, &deps, &slot_occurrences.for_slot(0)); + assert!( + matches!( + err, + Err(PartialEquationError { + kind: PartialEquationErrorKind::UnfreezablePartial, + .. + }) + ), + "{label}: must be a loud Err, never an Ok equation carrying an \ + un-pinned subscript or an unattributed live read: {err:?}" + ); + } +} + +/// The mirror of the decline: a runtime table index that is ALREADY inside a +/// freeze is kept, and the score survives. +/// +/// `pop[Region, young] + PREVIOUS(LOOKUP(pop[Region, idx], input))` reaches the +/// very same no-occurrence table argument -- the IR skips a table argument wherever +/// it appears -- but here the enclosing `PREVIOUS` lags everything inside it, index +/// reads included, so `idx` is already ceteris-paribus and the partial is sound. +/// Refusing it (which an earlier revision of this branch did, because the refusal +/// keyed on "is a table argument" rather than "is not inside a freeze") drops a +/// perfectly good per-element score, and with it every loop score through the edge +/// -- the same over-decline that motivated this whole fix. +/// +/// So the freeze context is threaded in, and `Region` is still pinned: the pin is +/// what makes the fragment compile, and it is orthogonal to the freeze. +#[test] +fn per_element_pin_keeps_a_runtime_table_index_inside_a_freeze() { + for (label, eqn) in [ + ( + "inside a pre-existing PREVIOUS", + "pop[Region, young] + PREVIOUS(LOOKUP(pop[Region, idx], input))", + ), + ( + "inside a whole-frozen reducer", + "pop[Region, young] + SUM(other[LOOKUP(pop[Region, idx], input)])", + ), + ] { + let fx = PinFixture::new(vec![]); + let (ast, deps, occurrences) = fx.parse(eqn, &["input", "idx", "other"]); + assert_eq!( + PinFixture::source_occurrences(&occurrences), + 1, + "{label}: the table argument is un-recorded, so the rule is what decides: \ + {occurrences:?}" + ); + let slot_occurrences = SlotOccurrences::new(&occurrences); + let text = fx + .generate(&ast, &deps, &slot_occurrences.for_slot(0)) + .unwrap_or_else(|e| { + panic!( + "{label}: the index is already lagged by the enclosing freeze, so \ + the partial is ceteris-paribus and must be emitted: {e:?}" + ) + }); + assert!( + text.contains("pop[region\u{B7}boston, idx]"), + "{label}: the iterated axis must still be pinned (that is what makes the \ + fragment compile) and the already-lagged runtime index left alone; \ + got: {text}" + ); + assert!( + text.contains("pop[boston, young]"), + "{label}: the LIVE occurrence must keep its bare row spelling; got: {text}" + ); + } + + // The other half of the split: a freeze answers the CETERIS-PARIBUS question, + // never the COMPILABILITY one. `pop[State, old]` on a source declared over + // `[Region, Age]` still has no spelling this rule can produce, so a + // dimension-name subscript would survive into the scalar fragment and fail to + // compile -- inside a freeze exactly as outside one. It must stay loud. + let fx = PinFixture::new(vec![datamodel::Dimension::named( + "state".to_string(), + vec!["ny".to_string(), "ma".to_string()], + )]); + let (ast, deps, occurrences) = fx.parse( + "pop[Region, young] + PREVIOUS(LOOKUP(pop[State, old], input))", + &["input"], + ); + assert_eq!( + PinFixture::source_occurrences(&occurrences), + 1, + "the table argument is un-recorded here too: {occurrences:?}" + ); + let slot_occurrences = SlotOccurrences::new(&occurrences); + let err = fx.generate(&ast, &deps, &slot_occurrences.for_slot(0)); + assert!( + matches!( + err, + Err(PartialEquationError { + kind: PartialEquationErrorKind::UnfreezablePartial, + .. + }) + ), + "an index no pin can spell must stay loud even inside a freeze -- a freeze \ + cannot make a dimension-name subscript compile: {err:?}" + ); +} + +/// The pin-only descent must reach a source reference nested inside a source +/// subscript's own INDEX EXPRESSION, not just inside a range bound -- inside a +/// FREEZE, where pinning is the whole job. +/// +/// `PREVIOUS(pop[Region, pop[Region, young]])` reads a dynamically-selected +/// element of the source. The outer occurrence's second axis is not describable +/// per axis (it is an expression, not a coordinate), so that index reaches the +/// descent's index closure, and the INNER `pop[Region, young]` is a recorded +/// occurrence sitting in it. +/// +/// The closure descended a `Range`'s two endpoints but passed a plain `Expr` index +/// through untouched -- while its own comment claimed it descended "exactly as the +/// other-variable arm above does", and that arm handles both. The nested reference +/// therefore kept its DIMENSION-name subscript, in a scalar fragment that cannot +/// compile, with nothing set loud either. +/// +/// Everything here is already lagged by the enclosing `PREVIOUS`, index reads +/// included, so pinning is sufficient and NO further freeze is wanted (a nested +/// `PREVIOUS` would read two steps back). That is exactly what distinguishes this +/// from the table-argument case, which is not inside a freeze and is therefore +/// declined -- see +/// [`per_element_pin_declines_a_table_arg_with_a_runtime_index`]. +#[test] +fn per_element_pin_descends_into_a_source_subscript_index_expression() { + let fx = PinFixture::new(vec![]); + let (ast, deps, occurrences) = fx.parse( + "pop[Region, young] + PREVIOUS(pop[Region, pop[Region, young]])", + &[], + ); + // Non-vacuity: the index-nested occurrence must be recorded, or the closure + // would have nothing to pin and this test would pass on an empty walk. + assert!( + occurrences.iter().any(|o| o.index_nested + && matches!(&o.reference, crate::db::ltm_ir::OccurrenceRef::Variable(v) if v == "pop")), + "the fixture must record an index-nested `pop` occurrence: {occurrences:?}" + ); + let slot_occurrences = SlotOccurrences::new(&occurrences); + let text = fx + .generate(&ast, &deps, &slot_occurrences.for_slot(0)) + .expect("the per-element equation is derivable"); + + assert!( + !text.contains("pop[region,"), + "a source reference nested in an INDEX EXPRESSION kept its DIMENSION-name \ + subscript, which cannot resolve in a scalar fragment (a silent zero); \ + got: {text}" + ); + assert!( + text.contains("pop[region\u{B7}boston, pop[region\u{B7}boston, age\u{B7}young]]"), + "both the outer frozen read and the reference inside its index must be \ + pinned to this target element's row, QUALIFIED; got: {text}" + ); + assert!( + !text.contains("previous(pop[region\u{B7}boston, age\u{B7}young])"), + "the enclosing PREVIOUS already lags the index read, so the descent must \ + NOT add a second freeze (that would read two steps back); got: {text}" + ); + assert!( + text.contains("pop[boston, young]"), + "the LIVE occurrence must keep the bare row spelling; got: {text}" + ); +} + +/// One row of a pin-index verdict enumeration: a label, the source subscript as +/// written in the `LOOKUP` table argument, and the expected outcome in each freeze +/// context. `Some(spelling)` means the partial is EMITTED and its text contains +/// that spelling; `None` means it is DECLINED (`UnfreezablePartial` -> warned +/// skip). +type PinIndexCell<'a> = (&'a str, &'a str, Option<&'a str>, Option<&'a str>); + +/// Run one enumeration table against `fx`, both freeze columns per row. +/// +/// `live_ref` is the source reference the emitting site holds LIVE, spelled at +/// `fx`'s own shape -- it is what makes the equation a `PerElement` target at all, +/// and it is deliberately not the cell under test. The cell under test is the +/// table argument, which the IR records NOTHING for. +fn assert_pin_index_verdicts(fx: &PinFixture, live_ref: &str, cases: &[PinIndexCell<'_>]) { + // A DIMENSION-name subscript surviving into a scalar fragment is the silent + // zero this whole rule exists to prevent, so every cell forbids one on any + // dimension in play -- the source's own axes and the target's iterated dims. + let forbidden: Vec = fx + .source_dim_names + .iter() + .chain(fx.target_iterated_dims.iter()) + .map(|d| format!("pop[{d},")) + .collect(); + for (label, subscript, expect_bare, expect_frozen) in cases { + for (frozen, expected) in [(false, expect_bare), (true, expect_frozen)] { + let ctx = if frozen { "inside a freeze" } else { "bare" }; + let eqn = if frozen { + format!("{live_ref} + PREVIOUS(LOOKUP({subscript}, input))") + } else { + format!("{live_ref} + LOOKUP({subscript}, input)") + }; + let (ast, deps, occurrences) = fx.parse(&eqn, &["input", "idx"]); + // Non-vacuity, every cell: the table argument is the reference the IR + // records NOTHING for, so the rule is what decides -- not the IR. + assert_eq!( + PinFixture::source_occurrences(&occurrences), + 1, + "{label} ({ctx}): only the live occurrence outside the LOOKUP may be \ + recorded, or this cell tests the IR rather than the rule: \ + {occurrences:?}" + ); + let slot_occurrences = SlotOccurrences::new(&occurrences); + let got = fx.generate(&ast, &deps, &slot_occurrences.for_slot(0)); + match expected { + Some(spelling) => { + let text = got.unwrap_or_else(|e| { + panic!("{label} ({ctx}): expected an emitted partial: {e:?}") + }); + assert!( + text.contains(spelling), + "{label} ({ctx}): expected {spelling:?} in the partial; got: {text}" + ); + for bad in &forbidden { + assert!( + !text.contains(bad.as_str()), + "{label} ({ctx}): no DIMENSION-name subscript ({bad:?}) may \ + survive into a scalar fragment; got: {text}" + ); + } + // The emitting site must actually be held LIVE at its bare row, + // or the cell is not a `PerElement` instantiation at all and the + // table argument's verdict was reached in the wrong context. + assert!( + text.contains(&format!("pop[{}]", fx.row_parts_bare.join(", "))), + "{label} ({ctx}): the live occurrence must keep the bare row \ + spelling, or this fixture is not a PerElement instantiation; \ + got: {text}" + ); + } + None => assert!( + matches!( + got, + Err(PartialEquationError { + kind: PartialEquationErrorKind::UnfreezablePartial, + .. + }) + ), + "{label} ({ctx}): expected a loud UnfreezablePartial decline; got: {got:?}" + ), + } + } + } +} + +/// The full ENUMERATION: every index kind the rule can meet, in both freeze +/// contexts, with a stated verdict for each cell. +/// +/// Every previous finding in this machinery -- the `391bc3c1` regression and the +/// four review findings on this branch -- was a CELL, not a logic error: one shape +/// landing in the wrong bucket. Each was fixed after someone supplied the +/// counterexample. This test exists so the space is stated rather than sampled: +/// the rule sorts an index into exactly one of +/// [`super::post_transform::IndexVerdict`]'s four outcomes, and the outcome +/// depends on the index's spelling and (for `RuntimeRead` alone) on whether the +/// subtree is already frozen. Two columns, eighteen rows, no gaps. +/// +/// "No gaps" is a checkable claim, not a hope: `IndexExpr0` has exactly five +/// variants and every one has a row -- `Wildcard`, `StarRange`, `Range`, +/// `DimPosition`, and `Expr` (which fans out into the `Const` / `Var` / compound +/// rows, and the `Var` rows into every way a name can or cannot resolve). What the +/// table does NOT vary is subscript ARITY beyond the over-arity row, and that is +/// deliberate: `codegen::extract_table_info` accepts only a table argument +/// selecting exactly ONE element (a resolved `Var`, a `StaticSubscript` of +/// `view.size() == 1`, or a `Subscript` whose every index is +/// `SubscriptIndex::Single`), so an UNDER-arity subscript is `BadTable` for the same +/// reason the range/wildcard/star-range rows are unreachable. Rowing it would state +/// a verdict about a shape no compilable model produces. +/// +/// The table came back all-green once while still being wrong twice, which is +/// worth stating plainly: a mutation probe proves a table CONSTRAINS the code, and +/// nothing more. Whether each cell asserts the RIGHT verdict is a review question, +/// and whether the rows cover the space is a completeness question. Both defects +/// were of the second kind -- `@N` and a mapped dimension name were simply not +/// rows -- so a row added here is worth more than an assertion added to an +/// existing one. The MAPPED axis needs a different target dimension than the +/// source's, so its rows live in the sibling +/// [`per_element_pin_mapped_axis_verdict_enumeration`] over +/// [`PinFixture::mapped`]; everything resolvable against the source's own axis +/// names is here. +/// +/// Reading the table: see [`PinIndexCell`]. The unfrozen column is a bare `LOOKUP` +/// table argument; the frozen column is the same argument inside a pre-existing +/// `PREVIOUS`. Only the runtime-read rows differ between columns -- that is the +/// whole content of the compilability-vs-ceteris-paribus split. +/// +/// Two rows are marked UNREACHABLE and assert current behavior rather than a +/// derived requirement, because a compilable model cannot produce them: codegen's +/// `extract_table_info` rejects a range or wildcard table index outright +/// (`BadTable`, "range subscripts not supported in lookup tables"), so the +/// TARGET's own equation would fail to compile long before its link scores are +/// generated. Their frozen cells are therefore honestly uncertain: the rule +/// currently accepts them, and if they ever became reachable the failure would +/// surface as a fragment-compile Warning rather than a warned skip -- loud either +/// way, through a different channel. Left as-is deliberately rather than guessed +/// into `Unspellable`, since no reachable shape distinguishes the two choices. +/// +/// One further row (the numeric literal) is about the RULE's verdict, not about +/// downstream compilability: a numeric index into a NAMED dimension is not +/// resolvable by the compiler, but that is a property of the target's own +/// equation, which carries the same index. The rule's job is only to leave a +/// static selector alone, and that is what the cell pins. +#[test] +fn per_element_pin_index_verdict_enumeration() { + let cases: [PinIndexCell<'_>; 18] = [ + // --- static selectors: pinned, and identical in both contexts ---------- + ( + "the axis's own dimension name", + "pop[Region, young]", + Some("pop[region\u{B7}boston, age\u{B7}young]"), + Some("pop[region\u{B7}boston, age\u{B7}young]"), + ), + ( + "an element that axis declares", + "pop[Region, old]", + Some("pop[region\u{B7}boston, age\u{B7}old]"), + Some("pop[region\u{B7}boston, age\u{B7}old]"), + ), + ( + "an already dim\u{B7}elem-qualified element", + "pop[Region, age\u{B7}old]", + Some("pop[region\u{B7}boston, age\u{B7}old]"), + Some("pop[region\u{B7}boston, age\u{B7}old]"), + ), + ( + "a numeric literal (rule verdict only -- see fn docs)", + "pop[Region, 1]", + Some("pop[region\u{B7}boston, 1]"), + Some("pop[region\u{B7}boston, 1]"), + ), + ( + // `@N` reached the catch-all and was scored a RUNTIME read, so a bare + // table argument declined and every link score on the edge was dropped + // -- even though `compiler::context`'s subscript lowering resolves + // `DimPosition` to a concrete element offset in scalar context. It + // selects a FIXED element and reads nothing at the current step, so it + // is static in both senses the rule cares about, and needs no pin: the + // compiler that owns position syntax resolves it. + "an `@N` position index", + "pop[Region, @2]", + Some("pop[region\u{B7}boston, @2]"), + Some("pop[region\u{B7}boston, @2]"), + ), + ( + // The same defect in a different spelling: the catch-all never looked + // INSIDE a compound index, so arithmetic over literals scored a runtime + // read though it is exactly as static as the bare `1` two rows up. + "constant arithmetic over literals", + "pop[Region, 1 + 1]", + Some("pop[region\u{B7}boston, 1 + 1]"), + Some("pop[region\u{B7}boston, 1 + 1]"), + ), + ( + // A 0-arity BUILTIN index is where the widening deliberately STOPS. + // `reify_0_arity_builtins` turns `time` into an `Expr0::App` before this + // rule sees it, and telling `TIME` (varies every step) from `PI` (does + // not) inside `App` would mean a fourth copy of the builtin + // classification `builtins`/`compiler::invariance` own. So the arm stays + // loud in the bare column -- the CONSERVATIVE direction, a warned skip + // rather than a confident wrong score. A stated boundary, not a gap: if + // this ever needs to pin, the fix is to consult that classification, not + // to guess here. + "a 0-arity builtin index (deliberately still loud)", + "pop[Region, TIME]", + None, + Some("pop[region\u{B7}boston, time()]"), + ), + // --- unspellable: a COMPILABILITY verdict, so loud in BOTH contexts ---- + ( + "the source's own dim at a position the target does not project", + "pop[Region, Age]", + None, + None, + ), + ( + // The target iterates `Region`, not `State`, so nothing says WHICH + // `State` element this instantiation reads. Contrast the mapped table's + // rows, where the target DOES iterate `State`: there the shared + // correspondence names the element and the same spelling pins. + "another dimension's name the target does not iterate", + "pop[State, young]", + None, + None, + ), + ( + // A SUBDIMENSION name gets no special treatment: it is a dimension the + // target does not iterate, so it names no coordinate. (`bigcity` is a + // proper subdimension of the `region` axis by element containment.) + // Neither does the shared classifier treat one specially -- only its + // `StarRange` arm resolves a subdimension (`*:Sub`, GH #766), and a + // plain `Var` index naming one declines there too, so a target + // ITERATING a subdimension of the source axis cannot produce a + // `PerElement` site at all and never reaches this rule. + "a subdimension name of the source's axis", + "pop[BigCity, young]", + None, + None, + ), + ( + "the source's own dims TRANSPOSED", + "pop[Age, young]", + None, + None, + ), + ( + "an over-arity index no axis owns", + "pop[Region, young, young]", + None, + None, + ), + // --- runtime reads: a CETERIS-PARIBUS verdict, so context decides ------ + ( + "an undeclared bare name -- a variable selecting the element", + "pop[Region, idx]", + None, + Some("pop[region\u{B7}boston, idx]"), + ), + ( + "an arithmetic index expression", + "pop[Region, idx + 1]", + None, + Some("pop[region\u{B7}boston, idx + 1]"), + ), + ( + "a nested source subscript selecting the element", + "pop[Region, pop[Region, young]]", + None, + Some("pop[region\u{B7}boston, pop[region\u{B7}boston, age\u{B7}young]]"), + ), + // --- UNREACHABLE from a compilable model -- see fn docs --------------- + ( + "a range index (UNREACHABLE: codegen rejects it as a table index)", + "pop[Region, 1:2]", + None, + Some("pop[region\u{B7}boston,"), + ), + ( + "a wildcard index (UNREACHABLE: codegen rejects it as a table index)", + "pop[Region, *]", + None, + Some("pop[region\u{B7}boston,"), + ), + ( + "a star-range index (UNREACHABLE: codegen rejects it as a table index)", + "pop[Region, *:Age]", + None, + Some("pop[region\u{B7}boston,"), + ), + ]; + + // `state` and `bigcity` ride along in every cell so the two rows that name + // another dimension have real dimensions to name; they change no other row, + // since the rule resolves an index against the source's OWN axis at that + // position and neither is one of the target's iterated dims. + let fx = PinFixture::new(vec![ + datamodel::Dimension::named( + "state".to_string(), + vec!["ny".to_string(), "ma".to_string()], + ), + datamodel::Dimension::named("bigcity".to_string(), vec!["nyc".to_string()]), + ]); + assert!( + fx.dim_ctx.is_subdimension_of( + &crate::common::CanonicalDimensionName::from_raw("bigcity"), + &crate::common::CanonicalDimensionName::from_raw("region"), + ), + "the subdimension row needs `bigcity` to genuinely be a subdimension of the \ + source's `region` axis, or it duplicates the unrelated-dimension row" + ); + + assert_pin_index_verdicts(&fx, "pop[Region, young]", &cases); +} + +/// The NAME-COLLISION half of the enumeration: an index name that is BOTH an element +/// of the source's axis at that position AND a dimension the target iterates. +/// +/// XMILE permits the collision, and `compiler::subscript`'s `normalize_subscripts3` +/// breaks it toward the ELEMENT: it looks the index up in the axis's own +/// `indexed_elements` first ("takes priority") and only falls back to the +/// dimension-name `ActiveDimRef` reading. The pin has to break it the same way or it +/// contradicts the equation's actual meaning -- and both wrong outcomes are bad in +/// the two directions this branch cares about. Reading `region` as an iteration over +/// `Region` finds no `Region`/`Bucket` mapping and declines, dropping every score on +/// a valid edge (the silent-zero direction); and if such a mapping DID exist it would +/// pin the correspondence's element instead of the literal one, which is a compilable +/// CONFIDENTLY WRONG row -- the outcome worse than none. +/// +/// The control row beside it is the same axis's OTHER element, which no dimension is +/// named after: it must resolve identically, so a fix that merely special-cased the +/// colliding name would not pass both. +#[test] +fn per_element_pin_colliding_element_name_verdict_enumeration() { + let fx = PinFixture::colliding_element(); + assert!( + fx.dim_ctx.is_dimension_name("region"), + "the collision needs `region` to be a real DIMENSION name as well as an \ + element of the `bucket` axis, or this test proves nothing" + ); + + let cases: [PinIndexCell<'_>; 2] = [ + ( + "an axis element whose name is also an iterated dimension", + "pop[Region, region]", + Some("pop[region\u{B7}boston, bucket\u{B7}region]"), + Some("pop[region\u{B7}boston, bucket\u{B7}region]"), + ), + ( + "the control: the same axis's non-colliding element", + "pop[Region, old]", + Some("pop[region\u{B7}boston, bucket\u{B7}old]"), + Some("pop[region\u{B7}boston, bucket\u{B7}old]"), + ), + ]; + assert_pin_index_verdicts(&fx, "pop[Region, old]", &cases); +} + +/// The MAPPED half of the enumeration: the same rule, asked about an index naming a +/// dimension the target ITERATES that is not the source axis's own name. +/// +/// This is the second review finding, and it is the case a name comparison cannot +/// decide. `growth[State] = pop[State, young] + LOOKUP(pop[State, old], input)` over +/// a source declared `pop[Region, Age]` is a perfectly good model when a positional +/// `State`/`Region` mapping exists: the executed simulation reads `Region`'s element +/// at the same position as the `State` element being computed. The rule used to +/// judge `State` unspellable purely because the name differed from `Region`, which +/// dropped the whole `pop -> growth` score edge. +/// +/// The verdicts are not this rule's opinion. Each row is whatever +/// `DimensionsContext::mapped_element_correspondence` says, reached through +/// [`super::post_transform::per_element_row_for_target`] -- the SAME derivation the +/// occurrence-driven pin and the link-score NAME use, and the same one +/// `ltm_agg::classify_axis_access` gates its `Iterated` arm on (through +/// `iterated_axis_slot_elements`, the correspondence's preimage inversion). So the +/// rows below double as an agreement statement: a mapped pair this rule pins is +/// exactly a mapped pair the classifier calls `Iterated`, in both declaration +/// directions, and an element-mapped pair declines in both. +/// +/// Both columns are the SAME for every row here, and that is the point: these are +/// `Pinned` and `Unspellable` verdicts, neither of which the freeze context can +/// change. A mapped index is a static selector once the correspondence names its +/// element, and unspellable when it does not. +#[test] +fn per_element_pin_mapped_axis_verdict_enumeration() { + // `ma` is `State`'s second element, so the positional correspondence reads + // `Region`'s second -- `boston`. A row that pins must say so. + let positional: [PinIndexCell<'_>; 2] = [ + ( + "a mapped dimension name (the iterated axis)", + "pop[State, old]", + Some("pop[region\u{B7}boston, age\u{B7}old]"), + Some("pop[region\u{B7}boston, age\u{B7}old]"), + ), + ( + // The mapped axis composes with the ordinary literal-element arm rather + // than replacing it: only the axis whose index names an iterated dim + // goes through the correspondence. + "a mapped dimension name beside a literal element", + "pop[State, age\u{B7}old]", + Some("pop[region\u{B7}boston, age\u{B7}old]"), + Some("pop[region\u{B7}boston, age\u{B7}old]"), + ), + ]; + let declined: [PinIndexCell<'_>; 1] = [( + "an element-mapped (non-positional) pair", + "pop[State, old]", + None, + None, + )]; + + // Declaration direction must not matter: `mapped_element_correspondence` + // honors a mapping declared on either dimension (GH #757), so a reverse-declared + // positional pair pins identically. A forward-only gate here would silently drop + // half the mapped models. + for declare_on_state in [true, false] { + let fx = PinFixture::mapped(declare_on_state, vec![]); + assert_pin_index_verdicts(&fx, "pop[State, young]", &positional); + + // An EXPLICIT element map is declined even though it names a correspondence: + // the executed A2A lowering resolves mapped references POSITIONALLY and + // ignores the map (GH #756), so pinning the row the map names would spell a + // read the simulation never performs -- a compilable, confidently wrong + // score, which is the one outcome worse than none. + let fx = PinFixture::mapped( + declare_on_state, + vec![ + ("ny".to_string(), "boston".to_string()), + ("ma".to_string(), "nyc".to_string()), + ], + ); + assert_pin_index_verdicts(&fx, "pop[State, young]", &declined); + } +} diff --git a/src/simlin-engine/src/ltm_augment_post_transform.rs b/src/simlin-engine/src/ltm_augment_post_transform.rs index 05559cf78..134771978 100644 --- a/src/simlin-engine/src/ltm_augment_post_transform.rs +++ b/src/simlin-engine/src/ltm_augment_post_transform.rs @@ -2,33 +2,40 @@ // Use of this source code is governed by the Apache License, // Version 2.0, that can be found in the LICENSE file. -//! Post-transform lowering for LTM link-score equations (Track A stage 1). +//! Concrete-form lowerings for LTM link-score equations. //! //! The `agg → target` and `PerElement` link-score generators in `ltm_augment` //! were inverted by the "transform-first" restructuring: the ceteris-paribus -//! wrap (`wrap_changed_first_ast`) now runs on the target's OWN equation text -//! -- hoisted reducers still spelled `SUM(...)`, the live source's occurrence -//! held at its actual shape -- and the concrete-form rewrite runs AFTERWARD, on -//! the already-wrapped `Expr0`. This module owns those post-transform lowerings: +//! wrap (`wrap_changed_first_ast`) runs on the target's OWN equation -- hoisted +//! reducers still spelled `SUM(...)`, the live source's occurrence held at its +//! actual shape -- and the concrete-form rewrite happens around it. This module +//! owns those lowerings: //! //! - [`substitute_reducers_in_expr0`] rewrites each hoisted reducer subtree in //! the wrapped AST to its synthetic aggregate node's bare name (the held-live //! reducer to `agg`, each frozen `PREVIOUS(SUM(..))` to `PREVIOUS(agg)`); -//! - [`rewrite_per_element_source_refs`] row-pins every live/frozen source -//! occurrence of a `PerElement` edge to its concrete per-element subscript, +//! - the `PerElement` ROW PINNING -- [`pin_source_subscript_indices`] for one +//! occurrence and [`pin_only_source_refs`] for a subtree the wrap froze -- //! supported by [`PerElementRefCtx`], [`per_element_row_for_target`] (the //! single row derivation the link-score NAME and equation both consume), and -//! [`qualify_axis_element`]. +//! [`qualify_axis_element`]. These are called FROM the ceteris-paribus wrap as +//! it descends, not as a pass over its output: the wrap is the only place that +//! knows both the occurrence (by path) and whether it is about to freeze the +//! reference, and that second fact is what selects the bare-row spelling for +//! the live occurrence over the qualified-row spelling for every other one. +//! Running them afterward forced the lowering to re-derive each occurrence's +//! per-axis access with an Expr0 classifier, since a `SiteId` computed on the +//! original AST cannot address a tree the wrap has inserted `PREVIOUS` nodes +//! into; that classifier is now deleted. //! -//! Running the wrap on the own text and lowering afterward keeps the emitted -//! equation byte-identical while making the wrap occurrence-addressable -- it is -//! now keyed on the target's OWN occurrence stream. That is the whole point of -//! the inversion: stage 2/3 can retarget the wrap onto the `db::ltm_ir` -//! occurrence IR without touching these lowerings, because they deliberately do -//! NOT re-classify (each is a pure text-keyed / shape-keyed AST rewrite). See -//! the generators in `ltm_augment` -- `generate_agg_to_scalar_target_equation`, +//! Running the wrap on the target's own equation is what makes it +//! occurrence-addressable: it is keyed on the target's OWN occurrence stream, so +//! every decision comes from `db::ltm_ir`'s one classification. NONE of the +//! lowerings here re-classifies -- the agg substitution is a pure text-keyed AST +//! rewrite, and the row pinning reads `OccurrenceSite::axes`. See the generators +//! in `ltm_augment` -- `generate_agg_to_scalar_target_equation`, //! `generate_scalar_to_element_equation`, and `generate_per_element_link_equation` -//! -- for the wrap-then-lower composition that calls these. +//! -- for the compositions that call these. //! //! Split out of `ltm_augment.rs` to keep that file under the project //! line-count lint; included via `#[path]`, so `super::*` resolves the parent's @@ -38,23 +45,26 @@ use std::collections::HashMap; use crate::ast::{Expr0, IndexExpr0, print_eqn}; use crate::builtins::UntypedBuiltinFn; -use crate::canonicalize; use crate::common::{Canonical, Ident, RawIdent}; -use super::{ - IteratedDimCtx, classify_expr0_per_element_axes, expr0_iterated_axis_lines_up, - qualify_element_csv, -}; +use super::{OccurrenceLookup, qualify_element_csv}; +use crate::db::ltm_ir::{OccurrenceAxis, OccurrenceSite}; #[cfg(test)] use super::PartialEquationError; #[cfg(test)] use crate::lexer::LexerType; -/// Read-only context for [`rewrite_per_element_source_refs`]: everything the -/// walker needs to substitute the live source's subscript indices for one +/// Read-only context for the `PerElement` row-pinning lowering: everything it +/// needs to substitute the live source's subscript indices for one /// `(site, target element)` instantiation of a `PerElement` link score /// (GH #525, T6 of the shape-expressiveness design). +/// +/// Notably absent since the lowering became IR-driven: the source's per-axis +/// element-name lists and the iterated-dim recognition context. Those were the +/// inputs to the Expr0 per-axis CLASSIFIER this lowering used to run; the +/// per-axis truth now comes off the occurrence IR (`OccurrenceSite::axes`), so +/// only the projection data survives. pub(super) struct PerElementRefCtx<'a> { /// The live source variable (canonical). pub(super) from: &'a Ident, @@ -62,21 +72,16 @@ pub(super) struct PerElementRefCtx<'a> { /// ([`RefShape::PerElement`]'s `axes`). pub(super) site_axes: &'a [crate::ltm_agg::AxisRead], /// The row this `(site, e)` instantiation reads -- BARE element names, - /// one per source axis (parallel to `site_axes`). The wrap already held - /// the `site_axes`-shaped occurrence live (its shape equals `site_axes`); - /// this lowering rewrites that live occurrence to exactly these bare - /// indices so it re-prints as the historical `from[]`. + /// one per source axis (parallel to `site_axes`). The wrap holds the + /// `site_axes`-shaped occurrence live (its shape equals `site_axes`); the + /// lowering rewrites that live occurrence to exactly these bare indices so + /// it prints as the historical `from[]`. pub(super) row_parts_bare: &'a [String], - /// Element-name lists per source axis (position-strict resolution). - pub(super) source_dim_elements: &'a [Vec], /// The source's declared dimensions (for index qualification). pub(super) from_dims: &'a [crate::dimensions::Dimension], /// Target-iterated dim (canonical) -> (element of `e` for that dim, /// its index within the dim) -- the projection data `e` supplies. pub(super) target_elem_by_dim: &'a HashMap, - /// The iterated-dim recognition context (live source's axes + target - /// iterated dims + mapping context). - pub(super) iter_ctx: &'a IteratedDimCtx<'a>, pub(super) dim_ctx: &'a crate::dimensions::DimensionsContext, } @@ -101,7 +106,7 @@ pub(super) fn qualify_axis_element(elem: &str, dim: &crate::dimensions::Dimensio /// This is the SINGLE row derivation for the `PerElement` family's /// emission: the link-score NAME's row (computed by /// `emit_per_element_link_scores`) and the equation's live-reference row -/// (computed by [`rewrite_per_element_source_refs`]) both come from here, +/// (computed by [`pin_source_subscript_indices`]) both come from here, /// so they cannot disagree. pub(crate) fn per_element_row_for_target( axes: &[crate::ltm_agg::AxisRead], @@ -130,228 +135,707 @@ pub(crate) fn per_element_row_for_target( .collect() } -/// Row-pin every reference to the live source inside a `PerElement` link -/// score's target body so the final scalar equation is well-formed for one -/// `(site, target element)` instantiation -- the index-substitution mechanism -/// of `pin_body_to_row` (the GH #744 reducer-body machinery) lifted to -/// target-equation bodies (T6 of the shape-expressiveness design). +/// The per-axis [`crate::ltm_agg::AxisRead`] slice of a live-source subscript +/// occurrence, or `None` when the occurrence is not fully describable per axis. +/// +/// This is the IR-driven replacement for the retired Expr0 per-axis classifier +/// (`classify_expr0_per_element_axes`). The equivalence is exact rather than +/// approximate: that classifier accepted an index only when it was an +/// iterated-dimension name lined up with the source's axis at that position, or +/// a literal element of that axis, with the arity matching on all of +/// indices / source dims / source element lists. The IR reaches the SAME +/// per-axis verdict through [`crate::ltm_agg::classify_axis_access`] -- the +/// shared classifier the retired `expr0_iterated_axis_lines_up` mirrored gate +/// for gate -- so "every axis `Iterated` or `Pinned`, arity equal to the +/// source's declared arity" is the same predicate. A `Reduced` axis (a wildcard +/// or star-range index), a `MismatchedIterated` one, and a `Dynamic` one all +/// fall out here exactly as they produced `None` there. An over-arity index can +/// never be `Iterated` (the IR only consults `classify_axis_access` where the +/// source has an axis at that position), so the old `i < from_dims.len()` guard +/// is implied rather than dropped. +fn axes_as_read_slice(occ: &OccurrenceSite, arity: usize) -> Option> { + use crate::ltm_agg::AxisRead; + if occ.axes.is_empty() || occ.axes.len() != arity { + return None; + } + occ.axes + .iter() + .map(|a| match a { + OccurrenceAxis::Pinned(e) => Some(AxisRead::Pinned(e.clone())), + OccurrenceAxis::Iterated { dim, source_dim } => Some(AxisRead::Iterated { + dim: dim.clone(), + source_dim: source_dim.clone(), + }), + OccurrenceAxis::Reduced { .. } + | OccurrenceAxis::MismatchedIterated { .. } + | OccurrenceAxis::Dynamic => None, + }) + .collect() +} + +/// The row indices for one occurrence of the live source, QUALIFIED +/// (`region\u{B7}a`) so a frozen read compiles to a direct LoadPrev in the +/// scalar fragment. +fn qualified_row_indices(row: &[String], ctx: &PerElementRefCtx<'_>) -> Vec { + row.iter() + .zip(ctx.from_dims) + .map(|(part, dim)| { + IndexExpr0::Expr(Expr0::Var( + RawIdent::new_from_str(&qualify_axis_element(part, dim)), + crate::ast::Loc::default(), + )) + }) + .collect() +} + +/// Row-pin ONE subscript occurrence of the live source for a `PerElement` link +/// score, given the occurrence the IR recorded at its node +/// (GH #525, T6 of the shape-expressiveness design). +/// +/// `live` is whether the ceteris-paribus wrap is leaving this occurrence LIVE -- +/// its shape equals the emitting site's AND it is not inside a subtree the wrap +/// froze. Only the wrap can answer that, which is why the pinning runs inside +/// the wrap's own traversal rather than as a pass over the wrapped tree; see +/// the module docs. +/// +/// - the LIVE occurrence is rewritten to the row's BARE element indices, so it +/// prints as the historical `from[]`; +/// - any other fully-describable occurrence (a different `PerElement` shape, an +/// all-`Iterated` Bare-shaped subscript, an all-`Pinned` literal subscript) is +/// rewritten to ITS OWN row for this target element, QUALIFIED, so the freeze +/// the wrap puts around it compiles to a direct LoadPrev; +/// - a partially-describable subscript (a wildcard slice, a dynamic index) gets +/// only its `Iterated` indices substituted (qualified; meaning-preserving -- +/// the iterated dim IS that element in this slot) and leaves the rest to +/// `recurse_index`, which is the wrap's own index pass, so a genuinely dynamic +/// index still gets its `PREVIOUS(idx)` lag. +/// +/// A node with NO recorded occurrence is left untouched HERE. That is the loud +/// path, not a silent one: the wrap's own `missing_occurrence` guard fires on a +/// live-source subscript whose path misses on a non-empty stream, and the caller +/// abandons the partial with a warning. ([`pin_only_source_refs`] is the one +/// caller that first substitutes such a subscript's indices structurally, by +/// name, because a `LOOKUP` table argument legitimately has no occurrence and +/// still has to compile; see [`pin_dimension_name_indices`].) +pub(super) fn pin_source_subscript_indices( + indices: Vec, + node_occ: Option<&OccurrenceSite>, + ctx: &PerElementRefCtx<'_>, + live: bool, + mut recurse_index: impl FnMut(usize, IndexExpr0) -> IndexExpr0, +) -> Vec { + let describable = node_occ.and_then(|o| axes_as_read_slice(o, ctx.from_dims.len())); + if let Some(occ_axes) = describable { + if live && occ_axes == ctx.site_axes { + return ctx + .row_parts_bare + .iter() + .map(|p| { + IndexExpr0::Expr(Expr0::Var( + RawIdent::new_from_str(p), + crate::ast::Loc::default(), + )) + }) + .collect(); + } + if let Some(row) = + per_element_row_for_target(&occ_axes, ctx.target_elem_by_dim, ctx.dim_ctx) + { + return qualified_row_indices(&row, ctx); + } + return indices; + } + // Partially describable: substitute only the axes the IR classified + // `Iterated`, and hand every other index to the wrap's own index pass. + let axes = node_occ.map(|o| o.axes.as_slice()).unwrap_or(&[]); + indices + .into_iter() + .enumerate() + .map(|(i, idx)| { + let substituted = match (axes.get(i), ctx.from_dims.get(i)) { + (Some(OccurrenceAxis::Iterated { dim, source_dim }), Some(from_dim)) => { + let ax = crate::ltm_agg::AxisRead::Iterated { + dim: dim.clone(), + source_dim: source_dim.clone(), + }; + per_element_row_for_target( + std::slice::from_ref(&ax), + ctx.target_elem_by_dim, + ctx.dim_ctx, + ) + .map(|row| qualify_axis_element(&row[0], from_dim)) + } + _ => None, + }; + match substituted { + Some(part) => IndexExpr0::Expr(Expr0::Var( + RawIdent::new_from_str(&part), + crate::ast::Loc::default(), + )), + None => recurse_index(i, idx), + } + }) + .collect() +} + +/// Row-pin a BARE `Var` reference to the live source (the mixed +/// `Bare`+`PerElement` edge's other site): each axis reads the target element's +/// coordinate for that axis's own dimension (same-element semantics), qualified. +/// `None` when some axis does not resolve -- the caller then leaves the bare +/// reference for the wrap's conservative freeze. +pub(super) fn pin_bare_source_ref(ctx: &PerElementRefCtx<'_>) -> Option> { + let bare_axes: Vec = ctx + .from_dims + .iter() + .map(|d| crate::ltm_agg::AxisRead::Iterated { + dim: d.name().to_string(), + source_dim: d.name().to_string(), + }) + .collect(); + per_element_row_for_target(&bare_axes, ctx.target_elem_by_dim, ctx.dim_ctx) + .map(|row| qualified_row_indices(&row, ctx)) +} + +/// What [`pin_dimension_name_indices`] can say about ONE index of a source +/// subscript the occurrence IR records nothing for. +enum IndexVerdict { + /// A static selector this rule rewrites: an iterated coordinate, or a literal + /// element qualified with its own axis. + Pinned(String), + /// A static selector already spelled the way this rule would spell it (a + /// numeric literal, an `@N` position, an already-`dim·elem` name). + Static, + /// No pin can spell it, because the SHARED row derivation + /// ([`per_element_row_for_target`]) cannot resolve the axis: a dimension the + /// target does not iterate, an iterated dimension with no usable positional + /// correspondence to this source axis (unmapped, element-mapped, or a + /// transposition), an index no axis owns. Left alone it keeps a + /// DIMENSION-name subscript, which cannot resolve in a scalar fragment, so + /// this is loud ALWAYS -- a compilability verdict, independent of freezing. + Unspellable, + /// A RUNTIME read selecting the element: a variable (`pop[Region, idx]`) or a + /// nested expression (`pop[Region, pop[Region, old]]`). It COMPILES as it + /// stands, so this is purely a ceteris-paribus question -- see the `frozen` + /// discussion on [`pin_dimension_name_indices`]. + RuntimeRead, +} + +/// Row-pin a source subscript the occurrence IR deliberately records NOTHING +/// for, by NAME alone. Returns the rewritten indices plus whether the rule +/// DISCHARGED the subscript. +/// +/// This is a lowering-COMPLETENESS rule, not a classifier, and that distinction +/// is the whole reason it exists. `db::ltm_ir` records no occurrence under a +/// `LOOKUP` TABLE argument (`BuiltinContents::LookupTable(_) => {}` -- "a +/// graphical-function table reference is static data, not a causal edge"), and it +/// is RIGHT: such a reference carries no causal edge, so it earns no attribution, +/// no element edge and no score. But the lowering is still obliged to emit a +/// COMPILABLE scalar fragment, and a dimension-name subscript +/// (`effect[region, old]`) cannot resolve in one. Compilability and attribution +/// are two separate obligations, and discharging the second does not require the +/// first -- which is why this asks the IR for nothing. +/// +/// It consults NO occurrence and infers NO shape. Per index, by name: +/// +/// - an index the source's axis at that position DECLARES as an element (or an +/// already-`dim·elem`-qualified one) is a literal selector, qualified with that +/// axis (`old` -> `age·old`). This is tried FIRST, because it is the order +/// `compiler::subscript`'s own `normalize_subscripts3` resolves a subscript index +/// in: it looks the name up in the axis's `indexed_elements` and only falls back to +/// the dimension-name (A2A `ActiveDimRef`) reading if that misses. The two readings +/// collide when a dimension declares an element whose name is ALSO a dimension the +/// target iterates (`Category = [Region, x]` beside a `Region` dimension), and +/// there the element must win or this rule spells a row the simulation never reads. +/// (`ltm_agg::classify_axis_access` has the opposite order -- GH #986. That is a +/// precision gap rather than a wrong answer TODAY: its dimension-first reading then +/// finds no mapping and declines the axis, so the reference falls to the +/// conservative path instead of being mis-resolved. It is filed rather than fixed +/// here because that classifier feeds reducer hoisting, the reference-shape IR, and +/// element-edge emission, so reordering it moves decisions well outside this rule.) +/// Qualifying rather than leaving the element bare matters even though it would very +/// likely resolve bare (see `wrap_non_matching_in_previous`'s +/// `skip_index_qualification`): the wrap's generic `qualify_element_index` cannot +/// qualify an element name several dimensions declare, so a half-qualified subscript +/// is the one spelling whose compilability depends on the model's element names. +/// Qualifying here also makes this rule's output byte-identical to the +/// pre-`391bc3c1` pass's, which is the conservative thing for a regression fix to be; +/// - otherwise, an index spelling one of the TARGET's ITERATED dimensions is replaced by the +/// source element this target element reads on that axis -- derived by handing +/// an [`crate::ltm_agg::AxisRead::Iterated`] for the `(index dim, source axis +/// dim)` pair to [`per_element_row_for_target`], the SAME single row derivation +/// the occurrence-driven pin uses. That is what makes the identity axis +/// (`pop[Region, ..]` over a `Region` axis, the structural substitution +/// [`pin_bare_source_ref`] performs for a bare `Var`) and a positionally-MAPPED +/// axis (`effect[State, ..]` over a `Region` axis with a `State`/`Region` +/// mapping, either declaration direction -- GH #527 / #757) ONE arm rather than +/// two: the derivation resolves both through +/// `DimensionsContext::mapped_element_correspondence`, so this rule accepts +/// EXACTLY the mapped pairs `ltm_agg::classify_axis_access` accepts (that +/// classifier's `Iterated` arm gates on `iterated_axis_slot_elements`, the +/// preimage inversion of the same correspondence). An axis the derivation +/// declines -- no mapping, an explicit element map (GH #756: execution resolves +/// positionally and ignores it), a transposition, a dimension this target does +/// not iterate -- is `IndexVerdict::Unspellable`, and it is unspellable because +/// the SHARED derivation says so, not because the name differs; +/// - an index that selects a FIXED element is kept verbatim, because it needs no pin +/// and reads nothing at the current step. Three spellings qualify: a numeric +/// literal, arithmetic over numeric literals (`1 + 1` -- see +/// [`index_expr_selects_a_fixed_element`], whose base case is that literal), and an +/// `@N` POSITION index, which `compiler::context`'s subscript lowering resolves to +/// a concrete element offset in scalar context (which a link-score fragment is). +/// Spelling `@N` out as an element name here would be a SECOND implementation of +/// position syntax living outside the compiler that owns it; keeping it verbatim +/// leaves the one. +/// +/// Everything else is one of the two loud verdicts on [`IndexVerdict`], and +/// `frozen` is what separates them. `IndexVerdict::Unspellable` is loud +/// unconditionally: it is a COMPILABILITY verdict. `IndexVerdict::RuntimeRead` is +/// loud only when `frozen` is false, and that is a CETERIS-PARIBUS verdict: +/// +/// - a bare `LOOKUP` table argument is NOT inside a freeze -- the wrap holds it +/// verbatim rather than wrapping it, since a `PREVIOUS` of a table has no value +/// slot -- so a runtime index there stays LIVE in the emitted partial. +/// `codegen::extract_table_info` evaluates it to select the table element, so +/// the partial isolating one row would vary with the current-step value of +/// another. A descent that wraps nothing cannot fix that, and a +/// compilable-but-wrong score is worse than none (GH #311 / #661 / #743), so the +/// partial is abandoned; +/// - but the SAME `LOOKUP` nested inside a pre-existing `PREVIOUS`/`INIT`, inside +/// a whole-frozen reducer, or inside a frozen other-dep's subscript, IS already +/// lagged -- index reads included -- so ceteris paribus already holds and +/// refusing would drop a perfectly good score for no reason. +/// +/// So the caller passes the freeze context it alone knows, exactly as the wrap +/// threads its own `frozen` flag beside `path`. +/// +/// There is no `RefShape` here, no live-vs-frozen decision, and this can never +/// make the reference live-selectable (the pin-only descent records no +/// `live_ref`). It builds an `AxisRead` only to ASK the shared row derivation a +/// question; it never decides an access shape. Do NOT grow it into a per-axis +/// classifier: the second classifier family was deleted on purpose (`391bc3c1`), +/// and every per-axis question it needs answered is already answered by +/// [`per_element_row_for_target`] and the `DimensionsContext` beneath it. +/// +/// (`ltm_agg::classify_axis_access` -- the per-axis classifier the IR itself uses +/// -- is deliberately NOT the helper consulted here, for two reasons. It consumes +/// `IndexExpr2`, and this descent walks the `Expr0` the wrap lowered. More +/// importantly it answers a DIFFERENT question: "is this axis access statically +/// describable enough to hoist a reducer / emit an element edge?" -- which is why +/// it declines `@N`, correctly for hoisting and wrongly for compilability, and why +/// it has a `Reduced` verdict that means nothing to a pin. The shared answer this +/// rule needs is one level down, at the row derivation and the correspondence, and +/// both classifiers bottom out there.) +/// +/// The caller turns `discharged == false` into `WrapOutcome::missing_occurrence`, +/// i.e. a warned skip. +fn pin_dimension_name_indices( + indices: Vec, + ctx: &PerElementRefCtx<'_>, + frozen: bool, +) -> (Vec, bool) { + let mut discharged = true; + let indices = indices + .into_iter() + .enumerate() + .map(|(i, idx)| { + let (name, loc) = match &idx { + // An `@N` POSITION selector is static -- `compiler::context` + // resolves it to a concrete element offset in scalar context -- so + // it neither needs a pin nor reads anything at the current step. + IndexExpr0::DimPosition(..) => return idx, + IndexExpr0::Expr(Expr0::Var(name, loc)) => { + (crate::common::canonicalize(name.as_str()).to_string(), *loc) + } + // A numeric selector, and arithmetic over numeric selectors, is + // static: nothing to pin, nothing to lag. See + // [`index_expr_selects_a_fixed_element`] -- the bare `Const` is just + // its base case. + IndexExpr0::Expr(e) if index_expr_selects_a_fixed_element(e) => return idx, + // Any other compound index expression selects the element at + // runtime. (A range, wildcard or star-range cannot be a table index + // at all -- `codegen::extract_table_info` needs a subscript selecting + // exactly ONE element and rejects anything wider as `BadTable` -- so + // treating them the same way costs nothing.) + _ => { + return verdict_into_index( + IndexVerdict::RuntimeRead, + idx, + frozen, + &mut discharged, + ); + } + }; + let Some(dim) = ctx.from_dims.get(i) else { + // Over-arity: no axis owns this index, so nothing names its owner + // and there is nothing to resolve it against. + return verdict_into_index(IndexVerdict::Unspellable, idx, frozen, &mut discharged); + }; + // A literal element selector of THIS axis qualifies to `dim·elem`; + // `qualify_axis_element` returns the name unchanged for anything the axis + // does not declare, testing exactly the axis's own `indexed_elements` -- + // the same membership `compiler::subscript`'s own resolution tests, and + // it is tried BEFORE the dimension-name reading for the same reason. + let axis_element = qualify_axis_element(&name, dim); + let verdict = if ctx.dim_ctx.lookup(&name).is_some() { + IndexVerdict::Static + } else if axis_element != name { + IndexVerdict::Pinned(axis_element) + } else if ctx.target_elem_by_dim.contains_key(&name) { + // The index names one of the TARGET's ITERATED dimensions, so this + // axis reads whatever element this target element projects onto it. + // WHICH element that is -- the identity for a same-named axis, the + // positional correspondence for a mapped one -- is the shared row + // derivation's answer, not this rule's: it declines an unmapped or + // element-mapped pair, and a name-directed pin therefore accepts + // exactly the pairs the occurrence-driven one does. + let axis = crate::ltm_agg::AxisRead::Iterated { + dim: name.clone(), + source_dim: dim.name().to_string(), + }; + match per_element_row_for_target( + std::slice::from_ref(&axis), + ctx.target_elem_by_dim, + ctx.dim_ctx, + ) { + Some(row) => IndexVerdict::Pinned(qualify_axis_element(&row[0], dim)), + None => IndexVerdict::Unspellable, + } + } else if ctx.dim_ctx.is_dimension_name(&name) { + // A dimension name no target coordinate projects onto this axis. + IndexVerdict::Unspellable + } else { + // Neither an element of this axis nor a dimension: a variable read + // selecting the element at runtime. + IndexVerdict::RuntimeRead + }; + let verdict = match verdict { + // A pin that changes nothing keeps its own node. + IndexVerdict::Pinned(part) if part == name => IndexVerdict::Static, + other => other, + }; + match verdict { + IndexVerdict::Pinned(part) => { + IndexExpr0::Expr(Expr0::Var(RawIdent::new_from_str(&part), loc)) + } + other => verdict_into_index(other, idx, frozen, &mut discharged), + } + }) + .collect(); + (indices, discharged) +} + +/// Whether a subscript index expression selects a FIXED element -- built entirely +/// from numeric literals, so it reads nothing at any step and cannot vary with the +/// ceteris-paribus wrap's choice of what to hold live. /// -/// This runs as a POST-transform lowering of the already-wrapped AST (Track A -/// stage 1): the ceteris-paribus wrap (`wrap_changed_first_ast` with -/// `PerElement { site_axes }` held live) already held the emitting site's -/// occurrence live and froze every other reference at `PREVIOUS`; this pass -/// then substitutes each source occurrence's iterated-dim indices for concrete -/// elements. The distinction the wrap already encoded -- live occurrence not -/// nested in a wrap-inserted `PREVIOUS`, frozen occurrences nested inside one -/// -- lines up with the `force_qualified` flag below, so the lowering reads -/// "live vs frozen" off `PREVIOUS`-nesting rather than re-deciding it: +/// This is the predicate behind `IndexVerdict::Static` for a compound index. Its +/// base case is the bare `Const` the rule always left alone; `LOOKUP(pop[Region, +/// 1 + 1], x)` is exactly as static as `LOOKUP(pop[Region, 2], x)` and the rule used +/// to decline the first only because its catch-all never looked inside. /// -/// - an occurrence whose per-axis classification EQUALS the emitting site's -/// `axes` (and is not `force_qualified`, i.e. the wrap left it live) is -/// rewritten to the row's BARE element indices, so it re-prints as the -/// historical `from[]`; -/// - any other fully-classifiable occurrence (a different `PerElement` -/// shape, an all-`Iterated` Bare-shaped subscript, an all-`Pinned` -/// literal subscript) is rewritten to ITS OWN row for this target -/// element, QUALIFIED (`region\u{B7}a`) so its wrap-inserted `PREVIOUS(...)` -/// freeze compiles to a direct LoadPrev in the scalar fragment; -/// - a BARE `Var` occurrence of the source (the mixed `Bare`+`PerElement` -/// edge's other site) is pinned to the target element's projection onto -/// the source's own axes when every axis resolves (same-element -/// semantics), qualified -- else left as the wrap's conservative freeze; -/// - a partially-classifiable subscript (a wildcard slice, a dynamic -/// index) gets only its resolvable iterated-dim indices substituted -/// (qualified; meaning-preserving -- the iterated dim IS that element -/// in this slot), leaving the rest as the wrap left it. +/// It is deliberately the NARROWEST sound predicate, not a general invariance test. +/// `compiler::invariance::exprs_are_invariant` is the engine's run-invariance +/// derivation, but it consumes LOWERED `Expr` plus an offset-classification +/// callback, neither of which exists in this position -- and its notion is *wider* +/// than this rule's obligation anyway: `Dt`, `StartTime` and `INIT(x)` of any +/// variable are all run-invariant, yet whether a table index may read a variable's +/// init buffer is an ATTRIBUTION question, and attribution is the wrap's vocabulary, +/// not this rule's. So this decides only the half it can decide alone. /// -/// Inside `PREVIOUS(...)`/`INIT(...)` calls (both the wrap-inserted freezes and -/// any already in the source equation) the live-match rewrite is suppressed -/// (`force_qualified`): the contents are lagged/frozen and could never be the -/// live reference -- qualified substitution keeps the frozen read compiling to -/// a direct slot. -pub(super) fn rewrite_per_element_source_refs( +/// The match is exhaustive over `Expr0` on purpose -- no catch-all -- so a new +/// variant forces a decision here rather than silently inheriting `false`: +/// +/// - `Var` reads model state (or names an element, which the caller's own name arms +/// resolved before reaching this predicate); +/// - `Subscript` reads an array element; +/// - `App` is where a 0-arity builtin lands after `reify_0_arity_builtins`, and +/// `TIME` and `PI` are indistinguishable here without re-implementing the builtin +/// classification that `builtins`/`compiler::invariance` own. So the whole arm +/// stays `false`: a `PI`-indexed table declines CONSERVATIVELY (a loud skip, the +/// safe direction) rather than being sorted by a fourth copy of that knowledge. +fn index_expr_selects_a_fixed_element(expr: &Expr0) -> bool { + match expr { + Expr0::Const(..) => true, + Expr0::Op1(_, inner, _) => index_expr_selects_a_fixed_element(inner), + Expr0::Op2(_, lhs, rhs, _) => { + index_expr_selects_a_fixed_element(lhs) && index_expr_selects_a_fixed_element(rhs) + } + Expr0::If(cond, then_e, else_e, _) => { + index_expr_selects_a_fixed_element(cond) + && index_expr_selects_a_fixed_element(then_e) + && index_expr_selects_a_fixed_element(else_e) + } + Expr0::Var(..) | Expr0::Subscript(..) | Expr0::App(..) => false, + } +} + +/// Apply a non-rewriting [`IndexVerdict`]: keep the index as it stands, and clear +/// `discharged` when the verdict is loud in this freeze context (see +/// [`pin_dimension_name_indices`] -- `Unspellable` always, `RuntimeRead` only +/// outside a freeze). +fn verdict_into_index( + verdict: IndexVerdict, + idx: IndexExpr0, + frozen: bool, + discharged: &mut bool, +) -> IndexExpr0 { + match verdict { + IndexVerdict::Pinned(_) | IndexVerdict::Static => {} + IndexVerdict::Unspellable => *discharged = false, + IndexVerdict::RuntimeRead => { + if !frozen { + *discharged = false; + } + } + } + idx +} + +/// Row-pin the live source's references inside a subtree the ceteris-paribus +/// wrap declines to descend into -- a pre-existing `PREVIOUS`/`INIT` call, the +/// GH #517 whole-frozen reducer, or a `LOOKUP` table argument. +/// +/// This is the pin-only descent: it performs the lowering the wrap would have +/// performed had it recursed, driven by the SAME structural path cursor and the +/// SAME occurrence IR, and it wraps nothing. Everything it reaches is inside a +/// frozen subtree (or is static table data), so no occurrence here can be the +/// live reference -- every pin is qualified. +/// +/// Why this rather than tagging the tree during the wrap (the alternative +/// `86df68bf`'s rustdoc weighed): a tag needs either a descent whose only +/// purpose is to write tags, or a `Loc`-keyed side map from node position to +/// occurrence -- a second representation of the classification, keyed on a +/// coordinate the wrap rewrites, with its own drift surface. This carries no map +/// and writes no tag; it reads the one IR by path and does the actual work. +/// +/// A SUBSCRIPTED reference to the source with NO recorded occurrence has no +/// per-axis classification to pin it with, and must never be emitted un-pinned: +/// its DIMENSION-name subscript (`pop[region, young]`) cannot resolve in a scalar +/// link-score fragment -- the fragment is dropped, the variable keeps a layout +/// slot with no bytecode, and the score reads a constant 0. Two mechanisms cover +/// it, in order: +/// +/// - [`pin_dimension_name_indices`] discharges it STRUCTURALLY when every +/// dimension-name index spells one of the source's own declared dims. That is +/// the reachable case -- a `LOOKUP` TABLE argument, which the IR skips as +/// static data. Compilability, not attribution: the reference is still not a +/// causal edge and still earns no score, it just has to compile. +/// - anything the rule cannot discharge sets `unlowerable`, which the caller +/// turns into `WrapOutcome::missing_occurrence`, i.e. a warned skip. The known +/// shape is fixed; the unknown class stays LOUD. +/// +/// The two other pin-only descents (a pre-existing `PREVIOUS`/`INIT` call, a +/// whole-frozen reducer) are walked by the IR, so they reach neither mechanism. A +/// BARE `Var` reaches neither either: [`pin_bare_source_ref`] pins it +/// structurally from the source's own declared dims, needing no occurrence. +pub(super) fn pin_only_source_refs( expr: Expr0, ctx: &PerElementRefCtx<'_>, - force_qualified: bool, + occ: &OccurrenceLookup<'_>, + path: &[u16], + unlowerable: &mut bool, + frozen: bool, ) -> Expr0 { - let qualify_row = |row: &[String]| -> Vec { - row.iter() - .zip(ctx.from_dims) - .map(|(part, dim)| { - IndexExpr0::Expr(Expr0::Var( - RawIdent::new_from_str(&qualify_axis_element(part, dim)), - crate::ast::Loc::default(), - )) - }) - .collect() - }; match expr { Expr0::Const(..) => expr, Expr0::Var(ref ident, loc) => { if &Ident::::new(ident.as_str()) != ctx.from { return expr; } - // Same-element pin of a bare source reference: each axis reads - // the target element's coordinate for that axis's own dim. - let bare_axes: Vec = ctx - .from_dims - .iter() - .map(|d| crate::ltm_agg::AxisRead::Iterated { - dim: d.name().to_string(), - source_dim: d.name().to_string(), - }) - .collect(); - match per_element_row_for_target(&bare_axes, ctx.target_elem_by_dim, ctx.dim_ctx) { - Some(row) => Expr0::Subscript(ident.clone(), qualify_row(&row), loc), + match pin_bare_source_ref(ctx) { + Some(indices) => Expr0::Subscript(ident.clone(), indices, loc), None => expr, } } Expr0::Subscript(ident, indices, loc) => { if &Ident::::new(ident.as_str()) != ctx.from { - // Another variable's subscript: recurse into expression - // indices (a nested source reference can hide there) AND into a - // range's two endpoints, which are also `Expr0` - // (`other[from[D, young]:3]`). The IR walker records occurrences - // under both (`IndexExpr2::Range` pushes children 0 and 1), and - // the sibling `substitute_reducers_in_expr0` descends both, so - // skipping them here left a recorded source occurrence - // un-pinned: its dimension-name subscript survived into the - // scalar equation, which either fails to compile or reads the - // wrong element. - let indices = - indices - .into_iter() - .map(|idx| match idx { - IndexExpr0::Expr(e) => IndexExpr0::Expr( - rewrite_per_element_source_refs(e, ctx, force_qualified), - ), - IndexExpr0::Range(l, r, loc) => IndexExpr0::Range( - rewrite_per_element_source_refs(l, ctx, force_qualified), - rewrite_per_element_source_refs(r, ctx, force_qualified), - loc, + // Another variable's subscript: descend into expression indices + // and range endpoints, where a nested source reference can hide + // (`other[from[D, young]]`, `other[from[a]:3]`). The IR records + // occurrences under both, so skipping them would leave a + // recorded source occurrence un-pinned -- its dimension-name + // subscript would survive into the scalar equation, which either + // fails to compile or reads the wrong element. + let indices = indices + .into_iter() + .enumerate() + .map(|(i, idx)| { + let idx_path = super::child_path(path, i); + match idx { + IndexExpr0::Expr(e) => IndexExpr0::Expr(pin_only_source_refs( + e, + ctx, + occ, + &idx_path, + unlowerable, + frozen, + )), + IndexExpr0::Range(l, r, rloc) => IndexExpr0::Range( + pin_only_source_refs( + l, + ctx, + occ, + &super::child_path(&idx_path, 0), + unlowerable, + frozen, + ), + pin_only_source_refs( + r, + ctx, + occ, + &super::child_path(&idx_path, 1), + unlowerable, + frozen, + ), + rloc, ), // Wildcard / star-range / `@N` carry no `Expr0`. other => other, - }) - .collect(); + } + }) + .collect(); return Expr0::Subscript(ident, indices, loc); } - if let Some(occ_axes) = - classify_expr0_per_element_axes(&indices, ctx.source_dim_elements, ctx.iter_ctx) - { - if !force_qualified && occ_axes == ctx.site_axes { - // The emitting site's own shape: lower the occurrence - // (already held live or frozen by the preceding - // ceteris-paribus wrap) to the concrete row this score - // targets, spelled as bare elements. - let indices = ctx - .row_parts_bare - .iter() - .map(|p| { - IndexExpr0::Expr(Expr0::Var( - RawIdent::new_from_str(p), - crate::ast::Loc::default(), - )) - }) - .collect(); - return Expr0::Subscript(ident, indices, loc); - } - if let Some(row) = - per_element_row_for_target(&occ_axes, ctx.target_elem_by_dim, ctx.dim_ctx) - { - return Expr0::Subscript(ident, qualify_row(&row), loc); - } - return Expr0::Subscript(ident, indices, loc); + let node_occ = occ.get(path); + // With no recorded occurrence there is no per-axis classification to + // pin with, but the fragment still has to COMPILE, so the structural + // rule substitutes the source's own dimension-name indices by name. + // What it cannot discharge stays loud -- see the fn docs and + // [`pin_dimension_name_indices`]. `Some` occurrences bypass it + // entirely, so nothing the IR classifies changes spelling. + let (indices, discharged) = match node_occ { + Some(_) => (indices, true), + None => pin_dimension_name_indices(indices, ctx, frozen), + }; + if !discharged { + *unlowerable = true; } - // Partially classifiable: substitute only the resolvable - // iterated-dim indices (qualified), leave the rest (wildcards, - // literals, dynamic expressions) for the wrap's conservative - // handling. - let indices = indices - .into_iter() - .enumerate() - .map(|(i, idx)| { - let substituted = match &idx { - IndexExpr0::Expr(Expr0::Var(name, _)) => { - let d = canonicalize(name.as_str()).into_owned(); - if i < ctx.from_dims.len() - && ctx.iter_ctx.target_iterated_dims.contains(&d) - && expr0_iterated_axis_lines_up( - &d, - i, - ctx.source_dim_elements, - ctx.iter_ctx, - ) - { - let ax = crate::ltm_agg::AxisRead::Iterated { - dim: d, - source_dim: ctx.iter_ctx.source_dim_names[i].clone(), - }; - per_element_row_for_target( - std::slice::from_ref(&ax), - ctx.target_elem_by_dim, - ctx.dim_ctx, - ) - .map(|row| qualify_axis_element(&row[0], &ctx.from_dims[i])) - } else { - None - } - } - _ => None, - }; - match substituted { - Some(part) => IndexExpr0::Expr(Expr0::Var( - RawIdent::new_from_str(&part), - crate::ast::Loc::default(), + let indices = pin_source_subscript_indices( + indices, + node_occ, + ctx, + // Inside a frozen subtree nothing can be the live reference. + false, + |i, idx| { + // An index this occurrence's axes did not resolve: descend + // for a nested source reference, exactly as the + // other-variable arm above does -- BOTH an expression index + // and a range bound. A source reference nested in an + // expression index is reachable (a dynamic table element, + // `LOOKUP(pop[Region, pop[Region, young]], x)`, which + // `codegen::extract_table_info`'s `Expr::Subscript` arm + // supports), and skipping it left its dimension-name + // subscript in the scalar fragment with nothing set loud. + let idx_path = super::child_path(path, i); + match idx { + IndexExpr0::Expr(e) => IndexExpr0::Expr(pin_only_source_refs( + e, + ctx, + occ, + &idx_path, + unlowerable, + frozen, )), - // Not a resolvable iterated-dim index. A nested source - // reference can still hide inside a range endpoint - // (`from[a:from[b]]`), and the IR records it, so descend - // rather than leaving it un-pinned -- same reason as the - // other-variable branch above. An `Expr` index that did - // not substitute is left to the wrap's conservative - // handling (recursing would double-pin the source's own - // axis). - None => match idx { - IndexExpr0::Range(l, r, rloc) => IndexExpr0::Range( - rewrite_per_element_source_refs(l, ctx, force_qualified), - rewrite_per_element_source_refs(r, ctx, force_qualified), - rloc, + IndexExpr0::Range(l, r, rloc) => IndexExpr0::Range( + pin_only_source_refs( + l, + ctx, + occ, + &super::child_path(&idx_path, 0), + unlowerable, + frozen, ), - other => other, - }, + pin_only_source_refs( + r, + ctx, + occ, + &super::child_path(&idx_path, 1), + unlowerable, + frozen, + ), + rloc, + ), + // Wildcard / star-range / `@N` carry no `Expr0`. + other => other, } - }) - .collect(); + }, + ); Expr0::Subscript(ident, indices, loc) } Expr0::App(UntypedBuiltinFn(name, args), loc) => { - let lagged = name.eq_ignore_ascii_case("previous") || name.eq_ignore_ascii_case("init"); let args = args .into_iter() - .map(|a| rewrite_per_element_source_refs(a, ctx, force_qualified || lagged)) + .enumerate() + .map(|(i, a)| { + pin_only_source_refs( + a, + ctx, + occ, + &super::child_path(path, i), + unlowerable, + frozen, + ) + }) .collect(); Expr0::App(UntypedBuiltinFn(name, args), loc) } Expr0::Op1(op, inner, loc) => Expr0::Op1( op, - Box::new(rewrite_per_element_source_refs( + Box::new(pin_only_source_refs( *inner, ctx, - force_qualified, + occ, + &super::child_path(path, 0), + unlowerable, + frozen, )), loc, ), Expr0::Op2(op, l, r, loc) => Expr0::Op2( op, - Box::new(rewrite_per_element_source_refs(*l, ctx, force_qualified)), - Box::new(rewrite_per_element_source_refs(*r, ctx, force_qualified)), + Box::new(pin_only_source_refs( + *l, + ctx, + occ, + &super::child_path(path, 0), + unlowerable, + frozen, + )), + Box::new(pin_only_source_refs( + *r, + ctx, + occ, + &super::child_path(path, 1), + unlowerable, + frozen, + )), loc, ), Expr0::If(c, t, f, loc) => Expr0::If( - Box::new(rewrite_per_element_source_refs(*c, ctx, force_qualified)), - Box::new(rewrite_per_element_source_refs(*t, ctx, force_qualified)), - Box::new(rewrite_per_element_source_refs(*f, ctx, force_qualified)), + Box::new(pin_only_source_refs( + *c, + ctx, + occ, + &super::child_path(path, 0), + unlowerable, + frozen, + )), + Box::new(pin_only_source_refs( + *t, + ctx, + occ, + &super::child_path(path, 1), + unlowerable, + frozen, + )), + Box::new(pin_only_source_refs( + *f, + ctx, + occ, + &super::child_path(path, 2), + unlowerable, + frozen, + )), loc, ), } diff --git a/src/simlin-engine/src/ltm_augment_tests.rs b/src/simlin-engine/src/ltm_augment_tests.rs index 7b3c5371e..7d197566d 100644 --- a/src/simlin-engine/src/ltm_augment_tests.rs +++ b/src/simlin-engine/src/ltm_augment_tests.rs @@ -83,7 +83,7 @@ fn classify_expr0_rejects_out_of_range_integer_literal() { Loc::default(), ))]; - let shape = classify_expr0_subscript_shape(&indices, &dims, None); + let shape = classify_expr0_subscript_shape(&indices, &dims, None, None); assert_eq!( shape, RefShape::DynamicIndex, @@ -103,7 +103,7 @@ fn classify_expr0_rejects_out_of_range_integer_literal() { 1.0, Loc::default(), ))]; - let in_range_shape = classify_expr0_subscript_shape(&in_range, &dims, None); + let in_range_shape = classify_expr0_subscript_shape(&in_range, &dims, None, None); assert_eq!( in_range_shape, RefShape::FixedIndex(vec!["1".to_string()]), @@ -139,7 +139,7 @@ fn classify_expr0_canonicalizes_integer_literal_subscript() { Loc::default(), ))]; - let shape = classify_expr0_subscript_shape(&indices, &dims, None); + let shape = classify_expr0_subscript_shape(&indices, &dims, None, None); assert_eq!( shape, RefShape::FixedIndex(vec!["1".to_string()]), @@ -1086,7 +1086,7 @@ fn test_generate_special_chars_quoted() { /// Owned backing storage for a [`ReducerBodyCtx`] in tests. struct BodyCtxFixture { - body_text: String, + body: Expr0, live_source: String, arrayed_dep_dims: std::collections::HashMap, model_deps: HashSet, @@ -1110,7 +1110,9 @@ impl BodyCtxFixture { .chain(scalars.iter().map(|s| s.to_string())) .collect(); BodyCtxFixture { - body_text: body_text.to_string(), + body: Expr0::new(body_text, LexerType::Equation) + .expect("the fixture body parses") + .expect("the fixture body is non-empty"), live_source: live_source.to_string(), arrayed_dep_dims, model_deps, @@ -1129,7 +1131,7 @@ impl BodyCtxFixture { fn ctx(&self) -> ReducerBodyCtx<'_> { ReducerBodyCtx { - body_text: &self.body_text, + body: &self.body, live_source: &self.live_source, arrayed_dep_dims: &self.arrayed_dep_dims, model_deps: &self.model_deps, @@ -1453,7 +1455,7 @@ fn test_body_aware_nested_reducer_falls_back_to_delta_ratio() { /// `classify_reducer` must surface the reducer argument's canonical text. #[test] -fn test_classify_reducer_returns_body_text() { +fn test_classify_reducer_returns_body_ast() { let pop = subscript_wildcard("pop"); let weight = subscript_wildcard("weight"); let one = Expr2::Const("1".to_string(), 1.0, Loc::default()); @@ -1474,14 +1476,23 @@ fn test_classify_reducer_returns_body_text() { let expr = Expr2::App(BuiltinFn::Sum(Box::new(body)), None, Loc::default()); let var = var_with_expr(expr); let result = classify_reducer(&var, "weight").expect("expected a classified reducer"); - assert_eq!(result.body_text, "pop[*] * (1 - weight[*])"); + // The classifier now hands back the body's AST rather than its printed + // text; printing it is how this test states the expectation, not how the + // consumers read it. + assert_eq!(print_eqn(&result.body), "pop[*] * (1 - weight[*])"); } /// `expr_reference_idents` collects canonical heads (including inside /// subscript index expressions) but not function names. #[test] fn test_expr_reference_idents() { - let idents = expr_reference_idents("pop[*] * SAFEDIV(scale, other[idx + 1], 0)"); + let body = Expr0::new( + "pop[*] * SAFEDIV(scale, other[idx + 1], 0)", + LexerType::Equation, + ) + .expect("the fixture parses") + .expect("the fixture is non-empty"); + let idents = expr_reference_idents(&body); assert!(idents.contains("pop")); assert!(idents.contains("scale")); assert!(idents.contains("other")); @@ -1808,7 +1819,6 @@ fn test_partial_equation_iterated_dim_source_normalized_to_bare() { let iter_ctx = IteratedDimCtx { source_dim_names: &source_dim_names, target_iterated_dims: &target_iterated_dims, - dim_ctx: None, dep_dims: None, }; @@ -2942,7 +2952,7 @@ fn partial_freezes_whole_reducer_over_index_nested_live_source() { /// `PREVIOUS(PREVIOUS(g))`. This complements /// `partial_equation_does_not_rewrap_inside_previous` (which uses the /// two-argument SAMPLE-IF-TRUE `PREVIOUS(target, input)` shape) with the exact -/// `to = from + PREVIOUS(g)` shape the stage-2 `occ.already_lagged` field must +/// `to = from + PREVIOUS(g)` shape the wrap's structural PREVIOUS/INIT skip must /// reproduce. #[test] fn partial_leaves_already_lagged_other_dep_untouched() { @@ -4755,12 +4765,19 @@ fn sgft( target_ref: &str, gf_table_ref: Option<&str>, ) -> Result { + // A genuine text entry point: the tests spell the target equation as a + // string, so it is parsed here once. Production hands the transform an + // `Expr0` lowered straight from the target's `Expr2`. + let Ok(Some(ast)) = crate::ast::Expr0::new(equation_text, crate::lexer::LexerType::Equation) + else { + return Err(PartialEquationError::new(equation_text)); + }; let occ_sites = - build_wrap_test_occurrences(equation_text, from, deps, source_dim_elements, iter_ctx); + build_wrap_test_occurrences(&ast, from, deps, source_dim_elements, iter_ctx, dims_ctx); let slot_occurrences = SlotOccurrences::new(&occ_sites); let occ = slot_occurrences.for_slot(0); shaped_guard_form_text( - equation_text, + &ast, deps, from, shape, @@ -4801,8 +4818,11 @@ fn wrap_missing_live_source_occurrence_is_loud_not_silent_freeze() { // A correctly-aligned stream, then drop pop's occurrence: the lookup stays // non-empty (helper survives) but `pop[nyc]`'s node lookup now misses. + let ast = crate::ast::Expr0::new(equation, crate::lexer::LexerType::Equation) + .expect("the equation parses") + .expect("the equation is non-empty"); let desynced: Vec = - build_wrap_test_occurrences(equation, &live, &deps, &source_dims, None) + build_wrap_test_occurrences(&ast, &live, &deps, &source_dims, None, None) .into_iter() .filter(|o| !matches!(&o.reference, OccurrenceRef::Variable(v) if v == "pop")) .collect(); @@ -4813,9 +4833,8 @@ fn wrap_missing_live_source_occurrence_is_loud_not_silent_freeze() { "the guard scope requires a non-empty lookup -- helper must survive the drop" ); - let (_ast, out) = - wrap_changed_first_ast(equation, &deps, &live, &shape, None, None, None, &occ) - .expect("the equation parses"); + let (_wrapped, out) = + wrap_changed_first_ast(&ast, &deps, &live, &shape, None, None, None, &occ, None); assert!( out.missing_occurrence, "a live-source subscript path-miss on a non-empty lookup must flag the desync" @@ -4830,7 +4849,7 @@ fn wrap_missing_live_source_occurrence_is_loud_not_silent_freeze() { // changed-first partial. Changed-last is not even attempted: it would read // the SAME desynced stream. let err = shaped_guard_form_text( - equation, + &ast, &deps, &live, &shape, @@ -4873,7 +4892,6 @@ fn shaped_guard_form_falls_back_to_changed_last_for_unfreezable_co_source() { let iter_ctx = IteratedDimCtx { source_dim_names: &source_dim_names, target_iterated_dims: &target_iterated, - dim_ctx: None, dep_dims: None, }; let text = sgft( @@ -5060,7 +5078,6 @@ fn gh526_transposed_other_dep_with_threaded_dims_is_unfreezable() { let iter_ctx = IteratedDimCtx { source_dim_names: &source_dim_names, target_iterated_dims: &target_iterated_dims, - dim_ctx: None, dep_dims: Some(&dep_dims), }; let result = build_partial_equation_shaped( @@ -5166,7 +5183,6 @@ fn shaped_guard_form_declines_bare_arrayed_feeder_of_unhoisted_reducer() { let iter_ctx = IteratedDimCtx { source_dim_names: &source_dim_names, target_iterated_dims: &target_iterated, - dim_ctx: None, dep_dims: None, }; let err = sgft( @@ -5247,7 +5263,6 @@ fn gh526_natural_and_unthreadable_other_deps_keep_collapse() { let iter_ctx = IteratedDimCtx { source_dim_names: &source_dim_names, target_iterated_dims: &target_iterated_dims, - dim_ctx: None, dep_dims: Some(&dep_dims), }; let partial = build_partial_equation_shaped( @@ -5271,7 +5286,6 @@ fn gh526_natural_and_unthreadable_other_deps_keep_collapse() { let iter_ctx_unthreaded = IteratedDimCtx { source_dim_names: &source_dim_names, target_iterated_dims: &target_iterated_dims, - dim_ctx: None, dep_dims: Some(&empty_dep_dims), }; let partial = build_partial_equation_shaped( @@ -5304,18 +5318,14 @@ fn gh526_natural_and_unthreadable_other_deps_keep_collapse() { /// `arrayed_target_slot_scores` characterization golden. #[test] fn slot_occurrence_index_groups_every_slot() { - use crate::db::ltm_ir::{OccurrenceRouting, OccurrenceSite, SiteId}; + use crate::db::ltm_ir::{OccurrenceSite, SiteId}; let occ_at = |path: &[u16], name: &str| OccurrenceSite { site_id: SiteId(path.to_vec().into_boxed_slice()), reference: OccurrenceRef::Variable(name.to_string()), shape: RefShape::Bare, axes: Vec::new(), - target_element: None, - routing: OccurrenceRouting::Direct, in_reducer: false, - reducer_keys: Vec::new(), - already_lagged: false, index_nested: false, }; @@ -5404,76 +5414,6 @@ fn child_path_maps_over_arity_child_to_the_unaddressable_sentinel() { ); } -/// The per-element row-pinning lowering must descend into a subscript RANGE's -/// endpoints, not just its plain expression indices. -/// -/// A source reference can hide in a range bound under another variable's -/// subscript (`other[pop[region]:3]`). The IR walker records an occurrence there -/// (`IndexExpr2::Range` pushes children 0 and 1) and the sibling lowering -/// `substitute_reducers_in_expr0` descends both endpoints -- but -/// `rewrite_per_element_source_refs` matched only `IndexExpr0::Expr` and passed -/// a `Range` through untouched. The recorded occurrence was then left un-pinned: -/// its dimension-name subscript survived into the scalar per-element equation, -/// which either fails to compile (a `PREVIOUS`-of-dim-name capture helper) or -/// reads the wrong element. -#[test] -fn per_element_pin_descends_into_range_endpoints() { - use crate::dimensions::DimensionsContext; - use crate::ltm_agg::AxisRead; - - let region = make_named_dimension("region", &["nyc", "boston"]); - let from_dims = vec![region.clone()]; - let source_dim_elements = vec![vec!["nyc".to_string(), "boston".to_string()]]; - let source_dim_names = vec!["region".to_string()]; - let target_iterated_dims = vec!["region".to_string()]; - let dim_ctx = DimensionsContext::from( - [datamodel::Dimension::named( - "region".to_string(), - vec!["nyc".to_string(), "boston".to_string()], - )] - .as_slice(), - ); - let iter_ctx = IteratedDimCtx { - source_dim_names: &source_dim_names, - target_iterated_dims: &target_iterated_dims, - dim_ctx: Some(&dim_ctx), - dep_dims: None, - }; - let from = Ident::::new("pop"); - let site_axes = vec![AxisRead::Iterated { - dim: "region".to_string(), - source_dim: "region".to_string(), - }]; - let row_parts_bare = vec!["boston".to_string()]; - let mut target_elem_by_dim = HashMap::new(); - target_elem_by_dim.insert("region".to_string(), ("boston".to_string(), 1usize)); - - let ctx = super::post_transform::PerElementRefCtx { - from: &from, - site_axes: &site_axes, - row_parts_bare: &row_parts_bare, - source_dim_elements: &source_dim_elements, - from_dims: &from_dims, - target_elem_by_dim: &target_elem_by_dim, - iter_ctx: &iter_ctx, - dim_ctx: &dim_ctx, - }; - - // `pop[region]` sits in the LOWER bound of a range index of `other`. - let ast = Expr0::new("other[pop[region]:3]", LexerType::Equation) - .expect("fixture parses") - .expect("fixture is non-empty"); - let lowered = super::post_transform::rewrite_per_element_source_refs(ast, &ctx, false); - let text = print_eqn(&lowered); - - assert!( - !text.contains("pop[region]"), - "the source reference inside the range bound must be pinned, not left \ - with its dimension-name subscript; got: {text}" - ); - assert!( - text.contains("boston"), - "the range-bound occurrence must be pinned to this instantiation's row; \ - got: {text}" - ); -} +#[cfg(test)] +#[path = "ltm_augment_pin_tests.rs"] +mod pin_tests; diff --git a/src/simlin-engine/src/ltm_augment_wrap_test_support.rs b/src/simlin-engine/src/ltm_augment_wrap_test_support.rs index 3582ce8d0..83a9855d9 100644 --- a/src/simlin-engine/src/ltm_augment_wrap_test_support.rs +++ b/src/simlin-engine/src/ltm_augment_wrap_test_support.rs @@ -20,10 +20,10 @@ use super::*; use crate::db::ltm_ir::OccurrenceRef; /// Reconstruct the occurrence-IR stream ([`OccurrenceSite`]s) for one target -/// equation from its raw text, for the `#[cfg(test)]` wrap unit tests +/// equation's parsed `Expr0`, for the `#[cfg(test)]` wrap unit tests /// ([`build_partial_equation_shaped`]). Production gets the stream from /// `db::ltm_ir::model_ltm_reference_sites` (the `Expr2` walk); this rebuilds an -/// equivalent stream on the reparsed `Expr0` so those focused text-in/text-out +/// equivalent stream on the parsed `Expr0` so those focused text-in/text-out /// tests keep exercising the real production wrap with byte-identical inputs. /// /// The paths start at slot `0` (mirroring `walk_all_in_expr`'s single-body slot @@ -34,26 +34,25 @@ use crate::db::ltm_ir::OccurrenceRef; /// [`other_dep_occurrence_axes`] (only its verdict is consumed). #[cfg(test)] pub(crate) fn build_wrap_test_occurrences( - equation_text: &str, + ast: &Expr0, live_source: &Ident, deps: &HashSet>, source_dim_elements: &[Vec], iter_ctx: Option<&IteratedDimCtx<'_>>, + dim_ctx: Option<&crate::dimensions::DimensionsContext>, ) -> Vec { - let Ok(Some(ast)) = Expr0::new(equation_text, LexerType::Equation) else { - return Vec::new(); - }; let mut recorded = deps.clone(); recorded.insert(live_source.clone()); let mut out = Vec::new(); // Slot 0 for a scalar/A2A body (matching `walk_all_in_expr`). let mut path = vec![0u16]; walk_wrap_test_occurrences( - &ast, + ast, live_source, &recorded, source_dim_elements, iter_ctx, + dim_ctx, false, &mut path, &mut out, @@ -80,47 +79,86 @@ pub(crate) fn test_occurrences_for_var( let Some(ast) = to_var.ast() else { return out; }; - let walk_slot = |text: &str, slot: u16, out: &mut Vec| { - if let Ok(Some(e0)) = Expr0::new(text, LexerType::Equation) { - let mut path = vec![slot]; - walk_wrap_test_occurrences( - &e0, - from, - &recorded, - source_dim_elements, - None, - false, - &mut path, - out, - ); - } + let walk_slot = |expr: &crate::ast::Expr2, slot: u16, out: &mut Vec| { + let e0 = crate::patch::expr2_to_expr0(expr); + let mut path = vec![slot]; + walk_wrap_test_occurrences( + &e0, + from, + &recorded, + source_dim_elements, + None, + None, + false, + &mut path, + out, + ); }; match ast { Ast::Scalar(expr) | Ast::ApplyToAll(_, expr) => { - walk_slot(&crate::patch::expr2_to_string(expr), 0, &mut out); + walk_slot(expr, 0, &mut out); } Ast::Arrayed(_, per_elem, default_expr, _) => { let mut keys: Vec<_> = per_elem.keys().collect(); keys.sort(); for (slot, k) in keys.iter().enumerate() { - walk_slot( - &crate::patch::expr2_to_string(&per_elem[*k]), - slot as u16, - &mut out, - ); + walk_slot(&per_elem[*k], slot as u16, &mut out); } if let Some(def) = default_expr { - walk_slot( - &crate::patch::expr2_to_string(def), - keys.len() as u16, - &mut out, - ); + walk_slot(def, keys.len() as u16, &mut out); } } } out } +/// The per-axis classification of one index of a LIVE-SOURCE subscript, mirroring +/// `db::ltm_ir::classify_occurrence_axes`'s arm order: an ITERATED-dimension name +/// that lines up with the source's axis at this position wins first (a dimension +/// name is not a literal element, so testing the literal arm first would +/// mis-classify it `Dynamic`), then a literal element, then dynamic. +/// +/// The row-pinning lowering reads these axes to decide which index is a +/// projected coordinate and which is a fixed literal, so a builder that only +/// ever produced `Pinned`/`Dynamic` would leave every iterated axis un-pinned -- +/// silently, and only in the tests, which is the worst place for a divergence +/// from production to hide. +#[cfg(test)] +pub(crate) fn live_source_occurrence_axis( + idx: &IndexExpr0, + i: usize, + source_dim_elements: &[Vec], + iter_ctx: Option<&IteratedDimCtx<'_>>, + dim_ctx: Option<&crate::dimensions::DimensionsContext>, +) -> OccurrenceAxis { + if let (Some(ic), IndexExpr0::Expr(Expr0::Var(name, _))) = (iter_ctx, idx) { + let d = crate::canonicalize(name.as_str()).into_owned(); + if ic.target_iterated_dims.iter().any(|t| t == &d) { + // A target-iterated dimension NAME. It is `Iterated` when it lines up + // with the source's axis at this position, and `MismatchedIterated` + // when it does not (the GH #526 transposed / arity-mismatched / + // unusably-mapped case) -- NOT `Dynamic`, which is what a genuine + // `pop[i+1]` is. Every consumer currently treats the two the same + // ("not describable per axis"), so conflating them was invisible; + // `classify_occurrence_axes` distinguishes them, and the corpus gate + // compares this against it field for field. + if i < ic.source_dim_names.len() + && expr0_iterated_axis_lines_up(&d, i, source_dim_elements, ic, dim_ctx) + { + return OccurrenceAxis::Iterated { + dim: d, + source_dim: ic.source_dim_names[i].clone(), + }; + } + return OccurrenceAxis::MismatchedIterated { dim: d }; + } + } + match resolve_literal_element_index(idx, i, source_dim_elements) { + Some(e) => OccurrenceAxis::Pinned(e), + None => OccurrenceAxis::Dynamic, + } +} + /// One synthesized test occurrence: only `site_id`, `reference`, `shape`, /// `axes`, and `index_nested` are read by the wrap; the rest carry inert /// defaults. @@ -137,11 +175,7 @@ fn test_occurrence( reference: OccurrenceRef::Variable(ident.as_str().to_string()), shape, axes, - target_element: None, - routing: crate::db::ltm_ir::OccurrenceRouting::Direct, in_reducer: false, - reducer_keys: Vec::new(), - already_lagged: false, index_nested, } } @@ -159,6 +193,7 @@ fn walk_wrap_test_occurrences( recorded: &HashSet>, source_dim_elements: &[Vec], iter_ctx: Option<&IteratedDimCtx<'_>>, + dim_ctx: Option<&crate::dimensions::DimensionsContext>, index_nested: bool, path: &mut Vec, out: &mut Vec, @@ -170,6 +205,7 @@ fn walk_wrap_test_occurrences( recorded, source_dim_elements, iter_ctx, + dim_ctx, index_nested, path, out, @@ -193,16 +229,23 @@ fn walk_wrap_test_occurrences( let c = Ident::::new(name.as_str()); if recorded.contains(&c) { let (shape, axes) = if &c == live_source { - let shape = - classify_expr0_subscript_shape(indices, source_dim_elements, iter_ctx); + let shape = classify_expr0_subscript_shape( + indices, + source_dim_elements, + iter_ctx, + dim_ctx, + ); let axes = indices .iter() .enumerate() .map(|(i, idx)| { - match resolve_literal_element_index(idx, i, source_dim_elements) { - Some(e) => OccurrenceAxis::Pinned(e), - None => OccurrenceAxis::Dynamic, - } + live_source_occurrence_axis( + idx, + i, + source_dim_elements, + iter_ctx, + dim_ctx, + ) }) .collect(); (shape, axes) @@ -210,7 +253,7 @@ fn walk_wrap_test_occurrences( // Other dep: only the verdict (from its axes) is consumed. ( RefShape::Bare, - other_dep_occurrence_axes(&c, indices, iter_ctx), + other_dep_occurrence_axes(&c, indices, iter_ctx, dim_ctx), ) }; out.push(test_occurrence(&c, path, shape, axes, index_nested)); @@ -272,3 +315,427 @@ fn walk_wrap_test_occurrences( } } } + +// ── The `#[cfg(test)]` Expr0 access-shape classifier family ──────────────── +// +// These live HERE, beside their only consumer, rather than in `ltm_augment.rs`. +// Production decides access shape exactly once, on the target's `Expr2` AST +// (`db::ltm_ir`), and both consumers -- causal-edge emission and the +// ceteris-paribus wrap -- read that one classification. What survives below is a +// reconstruction of an occurrence stream on a parsed `Expr0`, so the focused +// text-in/text-out wrap unit tests can drive the real production wrap without a +// db. Keeping it in the test-support module makes that status structural rather +// than a `#[cfg(test)]` attribute a reader has to notice, and keeps +// `ltm_augment.rs` under the project line cap. + +/// Recognize an *iterated-dimension* `Expr0` subscript on the *live source* +/// -- one whose indices are exactly the target equation's iterated +/// dimensions, in the position matching the live source's declared +/// dimension order -- the Expr0-AST sibling of +/// `db::ltm_ir::classify_iterated_dim_shape` (GH #511). +/// +/// `live_source[d_0, d_1, ...]` is the iterated-dim case iff: +/// 1. it has exactly one index per source dimension (`indices.len() == +/// ctx.source_dim_names.len()`), and +/// 2. every index `d_i` is a bare `Var` naming a dimension that is one of +/// the target equation's iterated dimensions, *and* +/// 3. for each `i`, `d_i` is either the same name as the source's `i`-th +/// declared dimension `ctx.source_dim_names[i]`, or (when a +/// `DimensionsContext` is available) a dimension that *maps to* it (the +/// AC3.5 mapped-dimension case). +/// +/// When it matches, `wrap_non_matching_in_previous` collapses the subscript +/// to a bare `Var(live_source)` before the live/PREVIOUS dispatch -- it then +/// becomes the live ref (`live_shape == Bare`) or (when `live_shape != Bare`, +/// which shouldn't happen for an edge the IR classified `Bare`) a +/// `PREVIOUS(Var(live_source))` (a `Var` arg, which codegen accepts -- vs +/// the `PREVIOUS(Subscript(...))` the pre-fix code produced, which trips the +/// codegen assertion). The model equation itself is untouched -- only the +/// LTM partial's `Expr0` is normalized -- so simulation still evaluates +/// `live_source[d_i]` correctly: in this slot, `live_source[d_i]` and a bare +/// `live_source` reference inside an apply-to-all-over-the-target's-dims +/// equation pick the same element (the bare ref broadcasts/iterates that +/// dimension). +/// +/// `#[cfg(test)]`: production reads the shape straight off the occurrence IR +/// (an iterated-dim subscript is classified `Bare` there); this Expr0 sibling +/// survives only for the wrap unit tests' occurrence builder and the alignment +/// gate. +#[cfg(test)] +pub(crate) fn is_live_source_iterated_dim_subscript( + indices: &[IndexExpr0], + source_dim_elements: &[Vec], + ctx: Option<&IteratedDimCtx<'_>>, + dim_ctx: Option<&crate::dimensions::DimensionsContext>, +) -> bool { + let Some(ctx) = ctx else { return false }; + if indices.is_empty() || indices.len() != ctx.source_dim_names.len() { + return false; + } + for (i, idx) in indices.iter().enumerate() { + let d = match idx { + IndexExpr0::Expr(Expr0::Var(name, _)) => canonicalize(name.as_str()).into_owned(), + _ => return false, + }; + if !ctx.target_iterated_dims.iter().any(|t| t == &d) { + return false; + } + if !expr0_iterated_axis_lines_up(&d, i, source_dim_elements, ctx, dim_ctx) { + return false; + } + } + true +} + +/// Does iterated-dimension index `d` (canonical) line up with the live +/// source's `i`-th axis -- by name, or through a usable positional-mapping +/// remap? The mapped arm consults the SAME +/// [`crate::ltm_agg::iterated_axis_slot_elements`] / +/// `mapped_element_correspondence` gate the Expr2 classifier +/// (`ltm_agg::classify_axis_access`) uses -- BOTH declaration directions +/// (GH #757), positional mappings only (GH #756) -- so the partial +/// builder's live-shape match and the reference-site IR agree by +/// construction. (No mapping context ⇒ no mapped recognition; the by-name +/// check still applies.) +#[cfg(test)] +pub(crate) fn expr0_iterated_axis_lines_up( + d: &str, + i: usize, + source_dim_elements: &[Vec], + ctx: &IteratedDimCtx<'_>, + dim_ctx: Option<&crate::dimensions::DimensionsContext>, +) -> bool { + let src_name = &ctx.source_dim_names[i]; + if d == src_name.as_str() { + return true; + } + let Some(dim_ctx) = dim_ctx else { + return false; + }; + let Some(elems) = source_dim_elements.get(i) else { + return false; + }; + crate::ltm_agg::iterated_axis_slot_elements(d, src_name, elems, dim_ctx).is_some() +} + +/// The per-axis [`crate::ltm_agg::AxisRead`] vector of a subscript whose +/// every index is either an iterated-dimension name lined up with the +/// source's axis at that position or a literal element of that axis +/// (position-STRICT, matching the Expr2 side's per-axis +/// `resolve_literal_axis_index`) -- the Expr0 sibling of the +/// `classify_axis_access`-derived classification +/// `db::ltm_ir::classify_iterated_dim_shape` performs, minus the `Reduced` +/// arm (a wildcard/StarRange index returns `None`; direct references never +/// collapse an axis, and the `Wildcard` precedence check already ran). +/// `None` when any index is neither -- the caller falls through to the +/// legacy literal pass / `DynamicIndex`. +/// +/// `#[cfg(test)]` like the rest of this family. It used to run in PRODUCTION, in +/// the `PerElement` row-pinning lowering, because that lowering was a pass over +/// the ALREADY-WRAPPED `Expr0` -- a tree the wrap mutated by inserting +/// `PREVIOUS(...)` nodes, so its child-index structure no longer matched the +/// target's ORIGINAL `Expr2` and a `SiteId` computed on that original AST could +/// not address it. Folding the pinning INTO the wrap removed that constraint: +/// there the occurrence is still reachable by path, so the per-axis truth comes +/// off `OccurrenceSite::axes` +/// (`post_transform::pin_source_subscript_indices`) and this classifier is no +/// longer part of any production decision. +#[cfg(test)] +pub(crate) fn classify_expr0_per_element_axes( + indices: &[IndexExpr0], + source_dim_elements: &[Vec], + ctx: &IteratedDimCtx<'_>, + dim_ctx: Option<&crate::dimensions::DimensionsContext>, +) -> Option> { + use crate::ltm_agg::AxisRead; + if indices.is_empty() + || indices.len() != ctx.source_dim_names.len() + || indices.len() != source_dim_elements.len() + { + return None; + } + let mut axes = Vec::with_capacity(indices.len()); + for (i, idx) in indices.iter().enumerate() { + if let IndexExpr0::Expr(Expr0::Var(name, _)) = idx { + let d = canonicalize(name.as_str()).into_owned(); + if ctx.target_iterated_dims.iter().any(|t| t == &d) { + if !expr0_iterated_axis_lines_up(&d, i, source_dim_elements, ctx, dim_ctx) { + return None; + } + axes.push(AxisRead::Iterated { + dim: d, + source_dim: ctx.source_dim_names[i].clone(), + }); + continue; + } + } + // Position-strict literal resolution: the Expr2 classifier resolves + // each index against ITS axis only, so the any-dimension fallback + // `resolve_literal_element_index` carries (for the legacy + // FixedIndex match) must not apply here -- a cross-axis literal + // would build a `Pinned` the IR never minted, breaking the + // live-shape equality match. + let candidate = match idx { + IndexExpr0::Expr(Expr0::Var(name, _)) => canonicalize(name.as_str()).into_owned(), + IndexExpr0::Expr(Expr0::Const(s, _, _)) => s.parse::().ok()?.to_string(), + _ => return None, + }; + if !source_dim_elements[i].iter().any(|e| e == &candidate) { + return None; + } + axes.push(AxisRead::Pinned(candidate)); + } + Some(axes) +} + +/// Build the per-axis [`OccurrenceAxis`] classification of an iterated-dimension +/// subscript on a *non-live-source* dependency (e.g. `pop[Region,Age]` in +/// `growth[Region,Age] = row_sum[Region] * c * pop[Region,Age]`), the way +/// `db::ltm_ir::classify_occurrence_axes` classifies it on the `Expr2` AST. +/// +/// This is the Expr0 sibling that lets the `#[cfg(test)]` occurrence builder +/// ([`build_wrap_test_occurrences`]) reconstruct an occurrence's `axes` for the +/// wrap unit tests. PRODUCTION reads the axes straight off the occurrence IR +/// ([`OccurrenceLookup`]) and feeds them to the single-sourced +/// [`derive_other_dep_verdict`] via [`other_dep_verdict`] -- there is no Expr0 +/// re-derivation of the verdict on the live path, so the two families cannot +/// drift. +/// +/// Each index that names (or positionally maps to) the dep's declared axis at +/// that position is `Iterated`, a target-iterated name that does NOT line up is +/// `MismatchedIterated` (the GH #526 case a bare collapse must not silently +/// freeze), an over-dep-arity target-iterated name is `Iterated{d,d}` (the +/// verdict's arity guard dominates it), and any non-target-iterated-Var index is +/// `Dynamic` (which makes `derive_other_dep_verdict` return `NotIterated`, +/// matching the pre-flip recognizer's short-circuit). +#[cfg(test)] +pub(crate) fn other_dep_occurrence_axes( + dep: &Ident, + indices: &[IndexExpr0], + ctx: Option<&IteratedDimCtx<'_>>, + dim_ctx: Option<&crate::dimensions::DimensionsContext>, +) -> Vec { + let Some(ctx) = ctx else { + return Vec::new(); + }; + let dep_dims = ctx.dep_dims.and_then(|m| m.get(dep.as_str())); + indices + .iter() + .enumerate() + .map(|(i, idx)| { + let IndexExpr0::Expr(Expr0::Var(name, _)) = idx else { + return OccurrenceAxis::Dynamic; + }; + let d = canonicalize(name.as_str()).into_owned(); + if !ctx.target_iterated_dims.iter().any(|t| t == &d) { + return OccurrenceAxis::Dynamic; + } + match dep_dims.and_then(|dd| dd.get(i)) { + Some(dep_dim) if other_dep_axis_lines_up(&d, dep_dim, dim_ctx) => { + OccurrenceAxis::Iterated { + dim: d, + source_dim: canonicalize(dep_dim.name()).into_owned(), + } + } + Some(_) => OccurrenceAxis::MismatchedIterated { dim: d }, + None => OccurrenceAxis::Iterated { + dim: d.clone(), + source_dim: d, + }, + } + }) + .collect() +} + +/// Does iterated-dimension index `d` (canonical) line up with a non-live +/// dep's declared axis `dep_dim` -- by name, or through a usable +/// positional-mapping remap? The dep-side sibling of +/// [`expr0_iterated_axis_lines_up`], consulting the SAME +/// [`crate::ltm_agg::iterated_axis_slot_elements`] / +/// `mapped_element_correspondence` gate (both declaration directions, +/// positional mappings only) so the live-source and other-dep recognizers +/// can never disagree about which mapped pairs are usable. +#[cfg(test)] +pub(crate) fn other_dep_axis_lines_up( + d: &str, + dep_dim: &crate::dimensions::Dimension, + dim_ctx: Option<&crate::dimensions::DimensionsContext>, +) -> bool { + let dep_dim_name = canonicalize(dep_dim.name()); + if d == dep_dim_name.as_ref() { + return true; + } + let Some(dim_ctx) = dim_ctx else { + return false; + }; + let elems = dimension_element_names(dep_dim); + crate::ltm_agg::iterated_axis_slot_elements(d, dep_dim_name.as_ref(), &elems, dim_ctx).is_some() +} + +/// Whether a single subscript index is a "literal element" reference -- +/// i.e., a `Var` naming a known dimension element or an integer literal. +/// These are dimension references at runtime, not variable references, +/// and must not be PREVIOUS-wrapped even when their textual form +/// collides with a user-variable name. +/// +/// `position` is the index's 0-based position in the subscript; literal +/// `Var` names are matched against the dimension at that position first +/// and then against any dimension as a fallback (mirroring +/// `classify_expr0_subscript_shape`'s match rules). +/// +/// `#[cfg(test)]`: see [`resolve_literal_element_index`]. +#[cfg(test)] +pub(crate) fn is_literal_element_index( + idx: &IndexExpr0, + position: usize, + source_dim_elements: &[Vec], +) -> bool { + resolve_literal_element_index(idx, position, source_dim_elements).is_some() +} + +/// Resolve a single subscript index to a literal element name, mirroring +/// `db::ltm_ir::resolve_literal_index` (the Expr2 sibling) so both +/// classifiers agree on what counts as a "literal element". +/// +/// `#[cfg(test)]`: this Expr0 classifier no longer drives production. The +/// ceteris-paribus wrap consumes the occurrence IR ([`OccurrenceLookup`]) -- the +/// same `db::ltm_ir` classification the edge emitter uses -- so there is ONE +/// classifier family and the historical Expr0/Expr2 drift (which silently zeroed +/// a link score when the wrap re-derived a different shape than the emitter, e.g. +/// GH #759 / GH #913 / the `pop[01]` canonicalization) is structurally +/// impossible. This sibling survives only for the wrap unit tests' occurrence +/// builder ([`build_wrap_test_occurrences`]) and the alignment gate +/// (`ltm_classifier_agreement_tests`), which proves the reparsed-Expr0 walk and +/// the IR's `Expr2` walk stay path- and shape-isomorphic corpus-wide. +/// +/// Element names appear as `Var` nodes; integer literals appear as +/// `Const` nodes whose text is the integer. Either form is validated +/// by membership in `source_dim_elements`. For an indexed dim of size +/// N, `dimension_element_names` produces `["1", "2", ..., "N"]`, so a +/// `Const("999", ...)` over an indexed dim of size 5 won't match and +/// falls through to `None`. Matching prefers the dim at the index's +/// position, falling back to any dim if not found there. +#[cfg(test)] +pub(crate) fn resolve_literal_element_index( + idx: &IndexExpr0, + position: usize, + source_dim_elements: &[Vec], +) -> Option { + let candidate = match idx { + IndexExpr0::Expr(Expr0::Var(name, _)) => canonicalize(name.as_str()).into_owned(), + IndexExpr0::Expr(Expr0::Const(s, _, _)) => { + // Integer literals (only) could be element references for + // indexed dims. Canonicalize via parse-then-format so + // non-canonical forms like `pop[01]` reduce to `"1"` and + // match `dimension_element_names`'s `"1".."N"` output. The + // Expr2 sibling (`db::ltm_ir::resolve_literal_index`) + // does the same; without canonicalization here we'd + // disagree on `01` (Expr2 -> FixedIndex(["1"]), + // Expr0 -> DynamicIndex), the live-shape match would + // fail, and the partial would silently zero. + let n = s.parse::().ok()?; + n.to_string() + } + _ => return None, + }; + let matches_position = position < source_dim_elements.len() + && source_dim_elements[position] + .iter() + .any(|e| e == &candidate); + let matches_any = !matches_position + && source_dim_elements + .iter() + .any(|dim| dim.iter().any(|e| e == &candidate)); + if matches_position || matches_any { + Some(candidate) + } else { + None + } +} + +/// Classify an `Expr0` subscript's shape based on its indices. +/// +/// Mirrors `db::ltm_ir::resolve_literal_index`'s classification logic but at +/// the `Expr0` (parsed-AST) level. `#[cfg(test)]`: the ceteris-paribus wrap no +/// longer re-derives shape on `Expr0` (it reads the occurrence IR); this sibling +/// survives only for the wrap unit tests' occurrence builder and the alignment +/// gate -- see [`resolve_literal_element_index`]. Each input string in +/// `source_dim_elements` is the canonical lowercase element name for the +/// corresponding source dimension, in source-declared order. +/// +/// Rules: +/// - any `IndexExpr0::Wildcard` → `RefShape::Wildcard` +/// - an iterated-dimension subscript on the live source (all axes iterated) +/// → `RefShape::Bare`; a mixed iterated+literal one → `RefShape::PerElement` +/// - all indices are literal element names that match the source's +/// declared elements (or parseable integer literals for indexed +/// dimensions) → `RefShape::FixedIndex(canonical_names)` +/// - otherwise (StarRange, DimPosition, Range, non-literal Expr, or a +/// literal that doesn't match) → `RefShape::DynamicIndex` +/// +/// The literal pass tries each index against the dimension at that position +/// first, then falls back to scanning all dimensions. This keeps the +/// classifier robust when callers pass dimensions in source-declared +/// order but the subscript indices may not align 1:1 with dimension +/// positions in unusual cases. Defensive `DynamicIndex` for unknown +/// names ensures the worst case is over-conservative wrapping rather +/// than incorrectly matching the live shape. +#[cfg(test)] +pub(crate) fn classify_expr0_subscript_shape( + indices: &[IndexExpr0], + source_dim_elements: &[Vec], + iter_ctx: Option<&IteratedDimCtx<'_>>, + dim_ctx: Option<&crate::dimensions::DimensionsContext>, +) -> RefShape { + if indices + .iter() + .any(|idx| matches!(idx, IndexExpr0::Wildcard(_))) + { + return RefShape::Wildcard; + } + // GH #511: an iterated-dimension subscript on the live source + // (`row_sum[Region]` inside an apply-to-all-over-`Region x Age` equation) + // reads the same source element -- it is `Bare`, mirroring + // `db::ltm_ir::classify_iterated_dim_shape`. Checked before the + // literal-element pass because a dimension name (`Region`) is not a + // literal element, so it would otherwise fall to `DynamicIndex`. + if is_live_source_iterated_dim_subscript(indices, source_dim_elements, iter_ctx, dim_ctx) { + return RefShape::Bare; + } + // GH #525 (T6): a mixed iterated+literal subscript (`pop[Region, young]` + // inside an A2A-over-`Region` equation) is `PerElement`, mirroring + // `classify_iterated_dim_shape`'s `classify_axis_access`-derived mixed + // arm -- the partial builder's live-shape match must agree with the + // reference-site IR (the documented sync requirement). All-`Pinned` + // falls through to the literal pass (`FixedIndex`), and all-`Iterated` + // is the `Bare` case above, so this arm fires only for a genuine mix. + if let Some(ctx) = iter_ctx + && let Some(axes) = + classify_expr0_per_element_axes(indices, source_dim_elements, ctx, dim_ctx) + { + let n_iterated = axes + .iter() + .filter(|a| matches!(a, crate::ltm_agg::AxisRead::Iterated { .. })) + .count(); + if n_iterated > 0 && n_iterated < axes.len() { + return RefShape::PerElement { axes }; + } + } + let mut elems = Vec::with_capacity(indices.len()); + for (i, idx) in indices.iter().enumerate() { + // Use the same resolver as `is_literal_element_index` so this + // classifier and the Expr2 sibling + // (`db::ltm_ir::resolve_literal_index`) agree on what counts + // as a literal element. Integer literals are validated against + // `source_dim_elements` (which contains `["1", ..., "size"]` + // for indexed dims), so out-of-range integers like `pop[999]` + // over a size-5 indexed dim fall through to `DynamicIndex` -- + // matching what the edge emitter sees and avoiding the + // shape-mismatch that would zero out the partial. + match resolve_literal_element_index(idx, i, source_dim_elements) { + Some(elem) => elems.push(elem), + None => return RefShape::DynamicIndex, + } + } + RefShape::FixedIndex(elems) +} diff --git a/src/simlin-engine/src/ltm_classifier_agreement_tests.rs b/src/simlin-engine/src/ltm_classifier_agreement_tests.rs index 2304cff06..2550c4081 100644 --- a/src/simlin-engine/src/ltm_classifier_agreement_tests.rs +++ b/src/simlin-engine/src/ltm_classifier_agreement_tests.rs @@ -20,16 +20,20 @@ //! class -- GH #759 dimension-name indices, GH #913 printer/parser asymmetry, the //! `pop[01]` indexed-literal canonicalization -- is structurally impossible). //! -//! The Expr0 classifier family (`resolve_literal_element_index` / -//! `classify_expr0_subscript_shape` / `classify_expr0_per_element_axes` / -//! `is_live_source_iterated_dim_subscript` in [`super`]) now survives only -//! `#[cfg(test)]`: it reconstructs an occurrence stream on the printed-and-reparsed -//! target text for the focused wrap unit tests (`ltm_augment_wrap_test_support`). -//! Those tests exercise the real production wrap, so the reconstructed stream -//! must carry the same shapes/paths the IR would. This gate is what proves it: -//! it classifies each occurrence with BOTH families over the corpus and asserts -//! they agree, so the `#[cfg(test)]` reconstruction cannot silently diverge from -//! the IR and feed the wrap a shape production never would. +//! The Expr0 classifier family is `#[cfg(test)]` ENTIRELY, and it now lives in +//! `ltm_augment_wrap_test_support` beside its only consumer: it reconstructs an +//! occurrence stream on a parsed `Expr0` so the focused wrap unit tests can drive +//! the real production wrap without a db. Those tests exercise production code, +//! so the reconstructed stream must carry the same shapes and paths the IR would. +//! This gate is what proves it: it classifies each occurrence with BOTH families +//! over the corpus and asserts they agree, so the `#[cfg(test)]` reconstruction +//! cannot silently diverge from the IR and feed the wrap a shape production never +//! would. +//! +//! The gate also carries the property that makes the print->reparse deletion +//! byte-neutral ([`assert_lowering_matches_reparse`]): every fixture slot is +//! checked for structural equality between the direct `Expr2` -> `Expr0` lowering +//! production now feeds the wrap and the printed-then-reparsed form it used to. //! //! # What this gate compares, and why it is drift-complete //! @@ -357,9 +361,9 @@ fn classify_expr0_occurrence( to_var: &Variable, variables: &HashMap, Variable>, dim_ctx: &DimensionsContext, -) -> RefShape { +) -> (RefShape, Vec) { let Some(indices) = &occ.indices else { - return RefShape::Bare; + return (RefShape::Bare, Vec::new()); }; let source_dims = source_dims_of(variables, &occ.source); let source_dim_names: Vec = source_dims.iter().map(|d| d.name().to_string()).collect(); @@ -369,10 +373,31 @@ fn classify_expr0_occurrence( let iter_ctx = IteratedDimCtx { source_dim_names: &source_dim_names, target_iterated_dims: &target_iterated_dims, - dim_ctx: Some(dim_ctx), dep_dims: None, }; - classify_expr0_subscript_shape(indices, &source_dim_elements, Some(&iter_ctx)) + let shape = classify_expr0_subscript_shape( + indices, + &source_dim_elements, + Some(&iter_ctx), + Some(dim_ctx), + ); + // The per-axis vector the `#[cfg(test)]` occurrence builder synthesizes, via + // the SAME function it uses -- so the gate proves what the builder actually + // produces, not a third derivation of it. + let axes = indices + .iter() + .enumerate() + .map(|(i, idx)| { + live_source_occurrence_axis( + idx, + i, + &source_dim_elements, + Some(&iter_ctx), + Some(dim_ctx), + ) + }) + .collect(); + (shape, axes) } /// The reparsed `Expr0` occurrences of every model variable in one target @@ -399,8 +424,12 @@ fn expr0_occurrences_for_target( // `slot` is the first `SiteId` path element (`walk_all_in_expr`: slot 0 for // a scalar/A2A body, the sorted element index for each `Ast::Arrayed` slot, // then the default slot after the last element). - let mut walk_text = |slot: u16, text: &str| { - let parsed = Expr0::new(text, crate::lexer::LexerType::Equation).unwrap_or_else(|e| { + let mut walk_slot = |slot: u16, expr: &crate::ast::Expr2| { + // Every fixture slot doubles as a corpus sample for the round-trip + // fidelity property the print->reparse deletion rests on. + assert_lowering_matches_reparse(&format!("target '{to_name}' slot {slot}"), expr); + let text = crate::patch::expr2_to_string(expr); + let parsed = Expr0::new(&text, crate::lexer::LexerType::Equation).unwrap_or_else(|e| { panic!("target '{to_name}' printed text failed to reparse (GH #913 drift class): text={text:?}, err={e:?}") }); if let Some(expr0) = parsed { @@ -410,7 +439,7 @@ fn expr0_occurrences_for_target( }; match ast { Ast::Scalar(expr) | Ast::ApplyToAll(_, expr) => { - walk_text(0, &crate::patch::expr2_to_string(expr)); + walk_slot(0, expr); } Ast::Arrayed(_, per_elem, default_expr, _) => { // Deterministic slot order (matches the sorted walk in @@ -419,16 +448,111 @@ fn expr0_occurrences_for_target( let mut keys: Vec<_> = per_elem.keys().collect(); keys.sort(); for (slot, k) in keys.iter().enumerate() { - walk_text(slot as u16, &crate::patch::expr2_to_string(&per_elem[*k])); + walk_slot(slot as u16, &per_elem[*k]); } if let Some(default) = default_expr { - walk_text(keys.len() as u16, &crate::patch::expr2_to_string(default)); + walk_slot(keys.len() as u16, default); } } } out } +/// Rebuild `expr` with every `Loc` reset to the default and every raw +/// identifier canonicalized, so two `Expr0` trees can be compared for +/// STRUCTURAL equality with the derived `PartialEq`. +/// +/// Two coordinates are deliberately erased, because they are exactly the two an +/// `Expr0` consumer never reads directly: +/// +/// * **`Loc`** -- a direct `Expr2` -> `Expr0` lowering carries the ORIGINAL +/// model text's spans while a print->reparse carries the printed text's. No +/// LTM consumer reads them. +/// * **Raw identifier SPELLING** -- `RawIdent` is pre-canonical by definition. +/// `Ident::::to_source_repr` renders the module separator `·` back +/// as `.`, and the parser hands back a quoted name WITH its quotes, so a +/// module-output composite is `$⁚m⁚0⁚smth1.output` on the lowering side and +/// `"$⁚m⁚0⁚smth1·output"` on the reparse side. Every consumer reads such a +/// head through `canonicalize` / `Ident::new` and prints it through +/// `print_ident` (which canonicalizes), so the two spellings are the same +/// identifier everywhere it matters -- including in the emitted text. +fn strip_locs(expr: &Expr0) -> Expr0 { + let d = crate::ast::Loc::default(); + match expr { + Expr0::Const(s, v, _) => Expr0::Const(s.clone(), *v, d), + Expr0::Var(id, _) => Expr0::Var(canonical_raw(id), d), + Expr0::App(UntypedBuiltinFn(name, args), _) => Expr0::App( + UntypedBuiltinFn(name.clone(), args.iter().map(strip_locs).collect()), + d, + ), + Expr0::Subscript(id, indices, _) => Expr0::Subscript( + canonical_raw(id), + indices.iter().map(strip_index_locs).collect(), + d, + ), + Expr0::Op1(op, inner, _) => Expr0::Op1(*op, Box::new(strip_locs(inner)), d), + Expr0::Op2(op, l, r, _) => { + Expr0::Op2(*op, Box::new(strip_locs(l)), Box::new(strip_locs(r)), d) + } + Expr0::If(c, t, e, _) => Expr0::If( + Box::new(strip_locs(c)), + Box::new(strip_locs(t)), + Box::new(strip_locs(e)), + d, + ), + } +} + +/// The canonical spelling of a raw identifier, as every `Expr0` consumer reads +/// it. See [`strip_locs`] for why spelling is normalized away. +fn canonical_raw(id: &crate::common::RawIdent) -> crate::common::RawIdent { + crate::common::RawIdent::new_from_str(canonicalize(id.as_str()).as_ref()) +} + +fn strip_index_locs(index: &IndexExpr0) -> IndexExpr0 { + let d = crate::ast::Loc::default(); + match index { + IndexExpr0::Wildcard(_) => IndexExpr0::Wildcard(d), + IndexExpr0::StarRange(id, _) => IndexExpr0::StarRange(canonical_raw(id), d), + IndexExpr0::Range(l, r, _) => IndexExpr0::Range(strip_locs(l), strip_locs(r), d), + IndexExpr0::DimPosition(n, _) => IndexExpr0::DimPosition(*n, d), + IndexExpr0::Expr(e) => IndexExpr0::Expr(strip_locs(e)), + } +} + +/// Assert that lowering `expr` straight to `Expr0` (`patch::expr2_to_expr0`) is +/// STRUCTURALLY identical to printing it and re-parsing the text -- the property +/// that makes the print->reparse round trip deletable without changing a single +/// generated byte. +/// +/// Every LTM equation transform runs on an `Expr0`, and today that `Expr0` comes +/// from `Expr0::new(expr2_to_string(expr))` -- a print followed by a parse of our +/// own output. The direct lowering is the same function minus the two string +/// steps (`expr2_to_string` IS `print_eqn(expr2_to_expr0(..))`), so if the two +/// results are structurally equal then swapping them cannot change any wrap +/// decision, any occurrence path, or any printed byte. +/// +/// Note the asymmetry in what a failure would mean: the DIRECT lowering is +/// structurally isomorphic to the `Expr2` tree by construction, so it is the one +/// that matches `db::ltm_ir::walk_all_in_expr`'s `SiteId` paths. A mismatch here +/// therefore reports a print/reparse infidelity in TODAY's path, not a defect in +/// the replacement. +#[track_caller] +fn assert_lowering_matches_reparse(what: &str, expr: &crate::ast::Expr2) { + let lowered = crate::patch::expr2_to_expr0(expr); + let text = crate::ast::print_eqn(&lowered); + let reparsed = Expr0::new(&text, crate::lexer::LexerType::Equation) + .unwrap_or_else(|e| panic!("{what}: printed text {text:?} failed to reparse: {e:?}")) + .unwrap_or_else(|| panic!("{what}: printed text {text:?} parsed to nothing")); + assert_eq!( + strip_locs(&lowered), + strip_locs(&reparsed), + "{what}: the direct Expr2->Expr0 lowering and the print->reparse round trip \ + disagree structurally on {text:?}; deleting the round trip would change the \ + tree the LTM wrap walks" + ); +} + /// Sort a shape multiset for order-insensitive comparison. `RefShape` derives /// `Ord`, so a stable sort gives a canonical multiset key. fn sorted(shapes: &[RefShape]) -> Vec { @@ -556,12 +680,24 @@ fn assert_occurrence_stream_aligns( "occurrence-stream in_reducer mismatch for {ir_src} -> {to_str}" ); if !ir_occ.in_reducer { - let e0_shape = classify_expr0_occurrence(e0_occ, to_var, variables, dim_ctx); + let (e0_shape, e0_axes) = classify_expr0_occurrence(e0_occ, to_var, variables, dim_ctx); assert_eq!( ir_occ.shape, e0_shape, "occurrence-stream SHAPE mismatch for {ir_src} -> {to_str}: the wrap's IR lookup \ would return a shape the Expr0 classifier disagrees with" ); + // The PER-AXIS classification, not just the coarse shape. Since the + // `PerElement` row pinning moved into the ceteris-paribus wrap it reads + // `OccurrenceSite::axes` to decide which index is a projected + // coordinate and which is a fixed literal, so the reconstruction the + // wrap unit tests feed the real wrap has to agree here too -- a + // divergence would exercise a pinning production never performs. + assert_eq!( + ir_occ.axes, e0_axes, + "occurrence-stream AXES mismatch for {ir_src} -> {to_str}: the row pinning reads \ + these, so the test-side reconstruction would drive a lowering production never \ + would" + ); } } } @@ -629,6 +765,130 @@ fn assert_occurrence_streams_align(tp: &TestProject) { ); } +/// Assert [`assert_lowering_matches_reparse`] for every slot of every variable +/// of `tp`'s `main` model, and return how many slots were checked so a caller +/// can prove the sweep was not vacuous. +fn assert_lowering_matches_reparse_everywhere(tp: &TestProject) -> usize { + let datamodel = tp.build_datamodel(); + let db = SimlinDb::default(); + let sync = sync_from_datamodel(&db, &datamodel); + let model = sync.models["main"].source; + let project = sync.project; + + // A fixture equation that fails to parse leaves its variable with NO AST, so + // it contributes no slot and would silently shrink the sweep. + let errors: Vec<_> = collect_all_diagnostics(&db, project) + .into_iter() + .filter(|d| d.severity == DiagnosticSeverity::Error) + .collect(); + assert!( + errors.is_empty(), + "round-trip fixture has compile errors -- a variable that fails to lower \ + contributes no slot to the sweep: {errors:?}" + ); + + let variables = reconstruct_model_variables(&db, model, project); + let mut names: Vec<&Ident> = variables.keys().collect(); + names.sort(); + let mut slots = 0usize; + for name in names { + let Some(ast) = variables[name].ast() else { + continue; + }; + let mut check = |slot: &str, expr: &crate::ast::Expr2| { + assert_lowering_matches_reparse(&format!("{name} {slot}"), expr); + slots += 1; + }; + match ast { + Ast::Scalar(expr) | Ast::ApplyToAll(_, expr) => check("body", expr), + Ast::Arrayed(_, per_elem, default_expr, _) => { + let mut keys: Vec<_> = per_elem.keys().collect(); + keys.sort(); + for k in keys { + check(k.as_str(), &per_elem[k]); + } + if let Some(default) = default_expr { + check("", default); + } + } + } + } + slots +} + +#[test] +fn direct_lowering_matches_reparse_on_print_sensitive_shapes() { + // The print->reparse deletion is byte-neutral only if the direct + // `Expr2` -> `Expr0` lowering IS the reparse of the printed form. The shapes + // most likely to break that are the ones where `print_eqn` renders something + // the parser could plausibly re-associate differently: a negated constant + // (`-3`, which could re-lex as a single negative literal), the zero-argument + // builtins (printed `time()`, which could re-parse as a bare `Var`), the + // word-spelled operators (`not`, `mod`), the two-character comparisons + // (`<>`, `>=`), right-associative `^`, an `If` nested as an operand, the + // variadic builtins, `LOOKUP`'s table argument, and every subscript index + // form (wildcard, star-range, range, `@N` position, literal, dynamic). + let curve = datamodel::GraphicalFunction { + kind: datamodel::GraphicalFunctionKind::Continuous, + x_points: Some(vec![0.0, 10.0]), + y_points: vec![0.0, 5.0], + x_scale: datamodel::GraphicalFunctionScale { + min: 0.0, + max: 10.0, + }, + y_scale: datamodel::GraphicalFunctionScale { min: 0.0, max: 5.0 }, + }; + let tp = TestProject::new("main") + .with_sim_time(0.0, 3.0, 1.0) + .named_dimension("Region", &["nyc", "boston"]) + .named_dimension("Sub", &["nyc"]) + .indexed_dimension("Slot", 4) + .scalar_aux("a", "2") + .scalar_aux("b", "3") + .scalar_aux("c", "4") + .array_aux("pop[Region]", "10") + .array_aux("wide[Slot]", "1") + .scalar_aux("idx", "2") + .aux_with_gf("curve", "a", curve) + // Unary minus on a constant AND on a compound operand. + .scalar_aux("negs", "-3 + -a * -(b + c)") + // Zero-argument builtins. + .scalar_aux("times", "TIME + TIME_STEP + INITIAL_TIME + FINAL_TIME + PI") + // Word operators, two-character comparisons, right-associative `^`, + // and an `If` in operand position. + .scalar_aux("ops", "a mod b + a ^ b ^ c") + .scalar_aux("cmps", "if (a <> b) AND (a >= c) then 1 else 0") + .scalar_aux("nots", "if NOT (a > b) then 1 else 0") + .scalar_aux( + "nested_if", + "a * (if a > b then (if b > c then 1 else 2) else 3)", + ) + // Variadic / optional-argument builtins and the LOOKUP table slot. + .scalar_aux( + "builtins", + "MAX(a, b) + MIN(a, b) + MEAN(a, b, c) + PULSE(a, b) + PULSE(a, b, c) \ + + SAFEDIV(a, b) + SAFEDIV(a, b, c) + LOOKUP(curve, a) + ABS(-a) + SQRT(a)", + ) + // Every subscript index form. + .scalar_aux("reduce_all", "SUM(pop[*])") + .scalar_aux("reduce_sub", "SUM(pop[*:Sub])") + .scalar_aux("literal_idx", "pop[nyc]") + .scalar_aux("dyn_idx", "wide[idx + 1]") + .scalar_aux("range_idx", "SUM(wide[1:3])") + .scalar_aux("pos_idx", "wide[@2]") + // A name the lexer cannot read bare, so `print_ident` quotes it. + .array_aux("\"1pop\"[Region]", "5") + .array_aux("quoted_reader[Region]", "\"1pop\" * 2"); + + let slots = assert_lowering_matches_reparse_everywhere(&tp); + // Non-vacuity: the sweep must actually have visited the fixture's variables + // (a fixture whose equations all failed to lower would pass trivially). + assert!( + slots >= 20, + "expected the print-sensitive fixture to contribute >= 20 slots, got {slots}" + ); +} + #[test] fn align_already_lagged_and_index_nested_streams() { // Occurrences inside `PREVIOUS(...)`/`INIT(...)` (already-lagged) and inside @@ -782,7 +1042,7 @@ fn assert_classifier_families_agree(tp: &TestProject) -> ComparedShapes { let expr0_occs = expr0_occurrences_for_target(to_str, to_var, &variables); let mut expr0_by_source: HashMap> = HashMap::new(); for occ in &expr0_occs { - let shape = classify_expr0_occurrence(occ, to_var, &variables, dim_ctx); + let (shape, _axes) = classify_expr0_occurrence(occ, to_var, &variables, dim_ctx); expr0_by_source .entry(occ.source.clone()) .or_default() diff --git a/src/simlin-engine/tests/integration/ltm_array_agg.rs b/src/simlin-engine/tests/integration/ltm_array_agg.rs index 278eb3617..41856d326 100644 --- a/src/simlin-engine/tests/integration/ltm_array_agg.rs +++ b/src/simlin-engine/tests/integration/ltm_array_agg.rs @@ -5639,6 +5639,434 @@ fn gh525_two_reference_partially_iterated_row_sum_scores() { ); } +/// A monotone two-point GF over the model's whole operating range, so a +/// per-element table neither saturates nor flattens the loop it sits in. +/// `slope` scales the identity so each element's table is distinguishable. +fn identity_gf(slope: f64) -> datamodel::GraphicalFunction { + datamodel::GraphicalFunction { + kind: datamodel::GraphicalFunctionKind::Continuous, + x_points: Some(vec![0.0, 400.0]), + y_points: vec![0.0, 400.0 * slope], + x_scale: datamodel::GraphicalFunctionScale { + min: 0.0, + max: 400.0, + }, + y_scale: datamodel::GraphicalFunctionScale { + min: 0.0, + max: 400.0 * slope, + }, + } +} + +/// Replace `effect`'s placeholder equation with the per-element TABLE-bearing form +/// the three `LOOKUP`-table-argument tests share: one graphical function per +/// `(Region, Age)` element, each element's equation naming its own `pop` cell. +/// +/// `effect` must carry ONE TABLE PER ELEMENT. A single whole-variable gf on an +/// arrayed variable has `table_count == 1`, so every element past the first reads +/// NaN (the same reason `ltm_augment_with_lookup` pins a shared table as +/// `to[1,...]`); a per-element gf rides an `Equation::Arrayed`. Being both +/// value-bearing AND table-bearing is what makes `effect` simultaneously a causal +/// source and a legal table reference, which is the whole premise of these tests. +fn give_effect_per_element_tables(project: &mut datamodel::Project) { + let effect = project.models[0] + .variables + .iter_mut() + .find(|v| v.get_ident() == "effect") + .expect("the fixture declares effect"); + let datamodel::Variable::Aux(effect) = effect else { + panic!("effect is declared as an aux"); + }; + let elements: Vec<( + String, + String, + Option, + Option, + )> = [ + ("a", "young", 1.0), + ("a", "old", 1.1), + ("b", "young", 0.9), + ("b", "old", 1.2), + ] + .into_iter() + .map(|(r, age, slope)| { + ( + format!("{r},{age}"), + format!("pop[{r},{age}]"), + None, + Some(identity_gf(slope)), + ) + }) + .collect(); + effect.equation = datamodel::Equation::Arrayed( + vec!["Region".to_string(), "Age".to_string()], + elements, + None, + false, + ); +} + +/// A `PerElement` source read BOTH as a value and as a `LOOKUP` **table** +/// argument in the same target keeps its per-element scores. +/// +/// `target[Region] = effect[Region,young] + LOOKUP(effect[Region,old], time)`, +/// where `effect[Region,Age]` is value-bearing AND carries a per-element +/// graphical function (a real equation plus tables -- an implicit WITH LOOKUP, +/// which is exactly what makes it both a causal source and a legal table +/// reference; `compiler::codegen::extract_table_info` resolves such an argument +/// by offset -> owning ident and has a deliberate `StaticSubscript` arm for +/// `LOOKUP(g[], x)`). +/// +/// The FIRST term is an ordinary, perfectly scoreable `PerElement` site. The +/// SECOND is a table argument, which `db::ltm_ir` deliberately records no +/// occurrence for ("static data, not a causal edge") -- correctly: it carries no +/// causal edge and earns no score. What it DOES need is to compile, and a +/// dimension-name subscript (`effect[region, old]`) cannot resolve in a scalar +/// link-score fragment. +/// +/// Conflating those two obligations is the regression this pins. `391bc3c1` moved +/// the row pinning inside the wrap, where it is IR-driven, so the table argument +/// came out un-pinned; `8d3ee4ab` then made the whole partial decline loudly, +/// which threw away the FIRST term's scores too -- every per-element score of a +/// valid edge dropped because the same target also mentions the source in a table +/// argument. The structural (name-directed) pin discharges the lowering without +/// inventing an occurrence, so both the compile and the scores survive. +/// +/// No char golden reaches this shape (none has a `LOOKUP` inside a partial at +/// all), which is why nothing else catches it. +#[test] +fn per_element_source_also_read_as_a_lookup_table_keeps_its_scores() { + let mut project = TestProject::new("per_element_lookup_table_arg") + .with_sim_time(0.0, 8.0, 1.0) + .named_dimension("Region", &["a", "b"]) + .named_dimension("Age", &["young", "old"]) + // Placeholder: replaced below by the per-element table-bearing form. + .array_aux("effect[Region, Age]", "pop[Region, Age]") + .array_aux( + "target[Region]", + "effect[Region, young] + LOOKUP(effect[Region, old], time)", + ) + .array_flow("growth[Region, Age]", "target[Region] * 0.0001", None) + .array_stock("pop[Region, Age]", "100", &["growth"], &[], None) + .build_datamodel(); + + give_effect_per_element_tables(&mut project); + + let mut db = SimlinDb::default(); + let sync = sync_from_datamodel_incremental(&mut db, &project, None); + set_project_ltm_enabled(&mut db, sync.project, true); + let compiled = compile_project_incremental(&db, sync.project, "main") + .expect("a per-element-table source read in a LOOKUP must compile with LTM"); + + // The direct silent-zero guard: a fragment that fails to compile is dropped + // and its variable reads a constant 0. + let warnings = assembly_warnings(&db, sync.project); + assert!( + warnings.is_empty(), + "every LTM fragment must compile -- an un-pinned dimension-name subscript \ + in the table argument is exactly what does not; got: {warnings:?}" + ); + + let source_model = sync.models["main"].source_model; + let ltm = model_ltm_variables(&db, source_model, sync.project); + let mut vm = Vm::new(compiled).expect("VM construction should succeed"); + vm.run_to_end() + .expect("VM simulation should run to completion"); + let results = vm.into_results(); + + // The edge's per-element scores exist AT ALL -- this is what the interim loud + // decline dropped -- and each pins the table argument to ITS OWN region. + for region in ["a", "b"] { + let name = format!("{LINK_SCORE_PREFIX}effect[{region},young]\u{2192}target[{region}]"); + let var = ltm.vars.iter().find(|v| v.name == name).unwrap_or_else(|| { + panic!( + "the direct PerElement site must still be scored; emitted: {:?}", + ltm.vars.iter().map(|v| v.name.as_str()).collect::>() + ) + }); + let text = match &var.equation { + LtmEquation::Scalar(arm) => arm.text.clone(), + other => panic!("{name} must be a scalar score; got {other:?}"), + }; + assert!( + text.contains(&format!("effect[region\u{B7}{region}, age\u{B7}old]")), + "the table argument must be pinned to THIS target element's own row \ + (region\u{B7}{region}), qualified; got: {text}" + ); + assert!( + !text.contains("effect[region,"), + "no dimension-name subscript may survive into the scalar fragment; \ + got: {text}" + ); + assert!( + text.contains(&format!("effect[{region}, young]")), + "the live site must keep its bare row spelling; got: {text}" + ); + + let series = series_at(&results, offset_of(&results, &name)); + assert!( + series.iter().all(|v| v.is_finite()), + "{name} must stay finite; got {series:?}" + ); + assert!( + series.iter().skip(STARTUP_STEPS).any(|&v| v != 0.0), + "{name} must carry a real non-zero value -- an identically-zero series \ + is the silent zero this shape regressed into; got {series:?}" + ); + } + + // ...and the loops the edge closes are scored too (one per region: only the + // `young` cells carry an `effect -> target` edge, since the table argument + // deliberately contributes none). + let loop_scores: Vec<&LtmSyntheticVar> = ltm + .vars + .iter() + .filter(|v| v.name.starts_with(LOOP_SCORE_PREFIX)) + .collect(); + assert!( + !loop_scores.is_empty(), + "the pop -> effect -> target -> growth -> pop loops must be scored" + ); + for lv in &loop_scores { + let series = series_at(&results, offset_of(&results, &lv.name)); + assert!( + series.iter().all(|v| v.is_finite()), + "loop score {} must stay finite; got {series:?}", + lv.name + ); + assert!( + series.iter().skip(STARTUP_STEPS).any(|&v| v != 0.0), + "loop score {} must carry real non-zero values; got {series:?}", + lv.name + ); + } +} + +/// The shared body of the STATIC-SELECTOR twins of +/// [`per_element_source_also_read_as_a_lookup_table_keeps_its_scores`]: the same +/// model, but the `LOOKUP` table argument picks its `Age` element with something +/// other than a name -- `table_index` as written, expected to survive VERBATIM in +/// the emitted fragment beside the pinned `region·{r}`. +/// +/// Both selectors reached the pin rule's catch-all and scored a RUNTIME read, so a +/// bare table argument declined and the edge's per-element scores were dropped. +/// Neither reads model state at the current step, so neither is a ceteris-paribus +/// hazard, and neither needs a pin. These tests are what make that a MEASURED claim +/// rather than a read of the compiler: the `warnings.is_empty()` assertion is the +/// compile, and the score-existence assertion is the dropped edge. +fn assert_static_table_index_keeps_its_scores( + fixture: &str, + table_index: &str, + expect_verbatim: &str, +) { + let mut project = TestProject::new(fixture) + .with_sim_time(0.0, 8.0, 1.0) + .named_dimension("Region", &["a", "b"]) + .named_dimension("Age", &["young", "old"]) + // Placeholder: replaced below by the per-element table-bearing form. + .array_aux("effect[Region, Age]", "pop[Region, Age]") + .array_aux( + "target[Region]", + &format!("effect[Region, young] + LOOKUP(effect[Region, {table_index}], time)"), + ) + .array_flow("growth[Region, Age]", "target[Region] * 0.0001", None) + .array_stock("pop[Region, Age]", "100", &["growth"], &[], None) + .build_datamodel(); + + give_effect_per_element_tables(&mut project); + + let mut db = SimlinDb::default(); + let sync = sync_from_datamodel_incremental(&mut db, &project, None); + set_project_ltm_enabled(&mut db, sync.project, true); + let compiled = compile_project_incremental(&db, sync.project, "main").unwrap_or_else(|e| { + panic!("a `{table_index}`-indexed table argument must compile with LTM: {e:?}") + }); + + let warnings = assembly_warnings(&db, sync.project); + assert!( + warnings.is_empty(), + "every LTM fragment must compile -- a `{table_index}` table index the pin \ + refuses makes the whole partial a warned skip, and one it MIS-pins would \ + fail to compile here; got: {warnings:?}" + ); + + let source_model = sync.models["main"].source_model; + let ltm = model_ltm_variables(&db, source_model, sync.project); + let mut vm = Vm::new(compiled).expect("VM construction should succeed"); + vm.run_to_end() + .expect("VM simulation should run to completion"); + let results = vm.into_results(); + + for region in ["a", "b"] { + let name = format!("{LINK_SCORE_PREFIX}effect[{region},young]\u{2192}target[{region}]"); + let var = ltm.vars.iter().find(|v| v.name == name).unwrap_or_else(|| { + panic!( + "the PerElement site must be scored -- declining the `{table_index}` \ + table index drops this whole edge; emitted: {:?}", + ltm.vars.iter().map(|v| v.name.as_str()).collect::>() + ) + }); + let text = match &var.equation { + LtmEquation::Scalar(arm) => arm.text.clone(), + other => panic!("{name} must be a scalar score; got {other:?}"), + }; + assert!( + text.contains(&format!("effect[region\u{B7}{region}, {expect_verbatim}]")), + "the static index must survive VERBATIM beside the pinned row -- it \ + already selects a fixed element, and re-spelling it here would be a \ + second implementation of what the compiler resolves; got: {text}" + ); + assert!( + !text.contains("effect[region,"), + "no dimension-name subscript may survive into the scalar fragment; \ + got: {text}" + ); + + let series = series_at(&results, offset_of(&results, &name)); + assert!( + series.iter().all(|v| v.is_finite()), + "{name} must stay finite; got {series:?}" + ); + assert!( + series.iter().skip(STARTUP_STEPS).any(|&v| v != 0.0), + "{name} must carry a real non-zero value -- an identically-zero series \ + is the silent zero this shape regressed into; got {series:?}" + ); + } +} + +/// `@N` POSITION syntax as the table argument's element selector +/// (`effect[Region, @2]` -- `@2` is `Age`'s second element, `old`). +/// `compiler::context`'s subscript lowering resolves `DimPosition` to a concrete +/// element offset in scalar context, which a link-score fragment is. +#[test] +fn per_element_source_read_as_a_lookup_table_by_position_keeps_its_scores() { + assert_static_table_index_keeps_its_scores( + "per_element_lookup_table_arg_by_position", + "@2", + "@2", + ); +} + +/// A CONSTANT ARITHMETIC expression as the table argument's element selector +/// (`effect[Region, 1 + 1]`). It reads no variable at any step, so it is exactly as +/// static as the bare literal `2` the rule already left alone -- the catch-all just +/// never looked inside the expression. +#[test] +fn per_element_source_read_as_a_lookup_table_by_constant_expr_keeps_its_scores() { + assert_static_table_index_keeps_its_scores( + "per_element_lookup_table_arg_const_expr", + "1 + 1", + "1 + 1", + ); +} + +/// The MAPPED twin of [`per_element_source_also_read_as_a_lookup_table_keeps_its_scores`]: +/// the target iterates `State` while the source is declared over `Region`, joined +/// by a positional `State -> Region` mapping. +/// +/// `target[State] = effect[State,young] + LOOKUP(effect[State,old], time)` over an +/// `effect[Region,Age]` source is a valid model -- the executed simulation reads +/// `Region`'s element at the same position as the `State` element being computed. +/// The name-directed table-argument pin nonetheless judged `State` unspellable +/// SOLELY because the name differed from the source axis's `Region`, so the whole +/// partial declined and every `effect -> target` per-element score on the edge was +/// dropped: exactly the same silent-zero outcome the un-mapped twin regressed into, +/// reached by a different route. +/// +/// The fix is that the pin asks `per_element_row_for_target` -- hence +/// `DimensionsContext::mapped_element_correspondence` -- which source element an +/// axis reads, instead of comparing names. That is the same derivation the score's +/// NAME comes from, which is why the assertions below can pair `s1` with `a` and +/// `s2` with `b` and expect the name and the pinned row to agree. +#[test] +fn mapped_per_element_source_read_as_a_lookup_table_keeps_its_scores() { + let mut project = TestProject::new("mapped_per_element_lookup_table_arg") + .with_sim_time(0.0, 8.0, 1.0) + .named_dimension("Region", &["a", "b"]) + .named_dimension("Age", &["young", "old"]) + .named_dimension_with_mapping("State", &["s1", "s2"], "Region") + // Placeholder: replaced below by the per-element table-bearing form. + .array_aux("effect[Region, Age]", "pop[Region, Age]") + .array_aux( + "target[State]", + "effect[State, young] + LOOKUP(effect[State, old], time)", + ) + // The loop closes through a whole-extent reduce so it needs no second + // mapped reference: the mapped axis under test is the one in `target`. + .array_flow("growth[Region, Age]", "SUM(target[*]) * 0.0001", None) + .array_stock("pop[Region, Age]", "100", &["growth"], &[], None) + .build_datamodel(); + + give_effect_per_element_tables(&mut project); + + let mut db = SimlinDb::default(); + let sync = sync_from_datamodel_incremental(&mut db, &project, None); + set_project_ltm_enabled(&mut db, sync.project, true); + let compiled = compile_project_incremental(&db, sync.project, "main") + .expect("a mapped per-element-table source read in a LOOKUP must compile with LTM"); + + let warnings = assembly_warnings(&db, sync.project); + assert!( + warnings.is_empty(), + "every LTM fragment must compile -- a mapped dimension-name index the pin \ + declines makes the whole partial a warned skip; got: {warnings:?}" + ); + + let source_model = sync.models["main"].source_model; + let ltm = model_ltm_variables(&db, source_model, sync.project); + let mut vm = Vm::new(compiled).expect("VM construction should succeed"); + vm.run_to_end() + .expect("VM simulation should run to completion"); + let results = vm.into_results(); + + // `s1` is `State`'s first element and `a` is `Region`'s, so the positional + // correspondence pairs them -- in the score's NAME and in the pinned table row + // alike, since both come from the one row derivation. + for (state, region) in [("s1", "a"), ("s2", "b")] { + let name = format!("{LINK_SCORE_PREFIX}effect[{region},young]\u{2192}target[{state}]"); + let var = ltm.vars.iter().find(|v| v.name == name).unwrap_or_else(|| { + panic!( + "the mapped PerElement site must be scored -- declining the table \ + argument's mapped index drops this whole edge; emitted: {:?}", + ltm.vars.iter().map(|v| v.name.as_str()).collect::>() + ) + }); + let text = match &var.equation { + LtmEquation::Scalar(arm) => arm.text.clone(), + other => panic!("{name} must be a scalar score; got {other:?}"), + }; + assert!( + text.contains(&format!("effect[region\u{B7}{region}, age\u{B7}old]")), + "the table argument's `State` index must be pinned to the SOURCE element \ + the mapping corresponds it to (region\u{B7}{region}), not to the target's \ + own {state}; got: {text}" + ); + for dim in ["region", "state"] { + assert!( + !text.contains(&format!("effect[{dim},")), + "no dimension-name subscript may survive into the scalar fragment; \ + got: {text}" + ); + } + assert!( + text.contains(&format!("effect[{region}, young]")), + "the live site must keep its bare row spelling; got: {text}" + ); + + let series = series_at(&results, offset_of(&results, &name)); + assert!( + series.iter().all(|v| v.is_finite()), + "{name} must stay finite; got {series:?}" + ); + assert!( + series.iter().skip(STARTUP_STEPS).any(|&v| v != 0.0), + "{name} must carry a real non-zero value -- an identically-zero series \ + is the silent zero this shape regressed into; got {series:?}" + ); + } +} + /// GH #525 (T6, BROADCAST): a `PerElement` reference whose Iterated dims /// are a strict SUBSET of the target's -- `mid[D1,D2] = pop[D1, young] * /// 0.05` (`D1` iterated, `Age` pinned, `D2` broadcast). One row feeds every