Repository navigation
LTM shape expressiveness: one per-axis access model (epic #488 phase 1) - #784
Merged
Merged
Conversation
Design document for the next epic-#488 burn-down phase: one coherent generalization of the LTM aggregate data model -- AxisRead becomes the single per-axis access vocabulary (gaining a subset-bearing Reduced and serving direct references via a new RefShape::PerElement family), AggNode carries per-source read slices plus a value-shape hoisting precondition, and read_slice_rows becomes the only co-reduced-row derivation. Ten member issues (#765 #766 #767 #764 #771 #751 #757 #525 #526 #769) fall out as consequences, with #526 called out honestly as a small adjunct fix rather than a consequence. Sequenced into seven independently-landable tasks with named RED tests, pinned-test flips, byte-identity blast-radius gates, and a C-LEARN perf protocol; the #756 element-map gate and dynamic indices stay conservatively declined.
Close the two critical holes: PerElement emission is now defined as per-(row, full-target-element) scores (the broadcast case mirrors the agg_name_for_target projection; pin_body_to_row is named as the live-ref index-substitution mechanism), and the #752 gate generalizes to any statically-describable variable-backed slice so scalar-result Pinned/ subset slices move edges and scores together in T3 instead of minting warned-phantom circuits. Also: precise gh525 flip note (all circuits go per-circuit scalar, ~0.5 per-site attribution, bare-name absence check), mixed Bare+PerElement resolver precedence pinned by a new fixture, #769 reframed as a FixedIndex-only adjunct gate widening, invariant I1 tightened (canonical slice, differing-subset and duplicate-var declines, sorted AggSource ordering), the T2->T5 feeder-pin vacuity split, the T3 atomicity requirement, and the minor sweep (stale #766 breadcrumb, non-subdimension StarRange declines, subset-inert agg aux equation, #526 unresolvable-dep fallback, SUM-only proptest note).
The design-review confirmation pass conditionally approved e5006af on one residual: section 6's scalar-result admission silently assumed the slice's owner is scalar, but an ARRAYED owner with a Pinned/subset broadcast slice (share[Region] = SUM(pop[nyc,*])) met the letter of the gate while the bare-to-node instruction would emit edges into a nonexistent node. The admission is now scoped to to_dims.is_empty(); the arrayed-owner shape is declined by both the gate and try_cross's partial-reduce branch into the #758 loud skip (it is silently wrong today -- full-cartesian per-row-slot garbage, an unfiled #765 sibling that becomes loud), pinned by a new T3 RED fixture, with the full broadcast fan-out noted as later T6-style work. Also takes the two cosmetic items: the dead scalar-to sentence in the PerElement name grammar and the garbled flip-note substring parenthetical.
Task T1 of docs/design-plans/2026-06-11-ltm-shape-expressiveness.md (Architecture sections 1-2; GH #766, GH #771). AxisRead::Reduced gains an optional element subset: a StarRange over a PROPER subdimension (SUM(arr[*:Sub])) resolves the subdimension's elements via SubdimensionRelation at enumeration time, a StarRange over the axis's own dimension stays the full extent (byte-identical), and a StarRange naming a non-subdimension now DECLINES the hoist rather than silently widening to the full extent. read_slice_rows and emit_agg_routed_edges iterate the subset, fixing MEAN/STDDEV divisors (subset size, not parent extent) and dropping out-of-subset element edges for inline/synthetic aggs. The per-axis decision logic moves out of compute_read_slice into classify_axis_access, the single per-axis classifier the design later rewires classify_iterated_dim_shape onto (T6); the extraction is behavior-faithful, including the one-directional has_mapping_to gate (GH #757 widens it in T6, not here). Hoisting now additionally requires reducer_collapses_to_scalar (invariant I5): RANK is array-valued, so the scalar agg node minted for it could never compile and every agg-routed loop score was a warned 0-stub. De-hoisted, a RANK argument classifies by its syntactic shape (Bare -> diagonal edges) and scores through the GH #742 arrayed-capture path; loops through the rank ORDERING are a documented residual on reducer_collapses_to_scalar. The variable-backed gate (variable_backed_partial_reduce_agg) excludes subset-bearing slices exactly like the Pinned exclusion -- admitting them would pair subset edges with try_cross_dimensional_link_scores' full-cartesian divisors (silently wrong numbers); both exclusions go together in T3 when that derivation consumes the read slice. Pin flips: rank_frozen_subtree_link_score_scores_correctly's exactly-1-warning assertion (the RANK-agg warning disappears) becomes zero-warnings with both Region loops asserted; libsimlin's fragment-failure harvest fixture rotates from the retired RANK shape to the Pinned-bearing mixed reduce (loud until T3). Byte-identity for non-subrange, non-RANK shapes verified by golden-diffing model_ltm_variables + element edges against the parent commit on the variable-backed partial-reduce, inline whole-extent MEAN, pinned-slice, and full-extent StarRange fixtures.
T1 review follow-up for the shape-expressiveness phase (docs/design-plans/2026-06-11-ltm-shape-expressiveness.md; GH #766, GH #771). The GH #771 de-hoist created a new Direct-Wildcard path that four load-bearing inventories falsely denied: RANK(pop[*], 1) -- the wildcard-ARG spelling -- never sets in_reducer, so the #514 DynamicIndex reclassification deliberately does not fire and the site reaches emit_edges_for_reference's conservative cross-product arm (coarse but sound; scores ~0 under constant ranks). Update the reclassification comment in ltm_ir.rs (the AC4.5 invariant narrowed to hoistable reducers' arguments), the Wildcard-arm inventory in analysis.rs, link_score_var_name's Direct-shape inventory in ltm_augment.rs, and the two matching CLAUDE.md sentences -- these inventories are load-bearing for T6's implementor. Also pin two implemented-but-untested classify_axis_access branches (the same-cardinality permuted-alias StarRange normalizing to the unique full-extent form per invariant I3, and the indexed-subdimension declared-parent subset resolution) and add the composed end-to-end fixture out[D1] = 1 + SUM(matrix[D1,*:SubD2]) closed in a loop (zero warnings, subset-only per-(row, slot) scores at exactly 0.5, no loop through the unread rows, finite non-zero loop scores).
T2 of the LTM shape-expressiveness design (docs/design-plans/2026-06-11-ltm-shape-expressiveness.md, "Task 2: per-source AggNode representation"): AggNode.sources: Vec<AggSource> -- one entry per source variable, sorted by canonical name, each carrying its own read slice -- replaces the source_vars list plus the single shared read_slice. Acceptance stays identical-slices-only (combined_read_slice is unchanged), so every arrayed source carries the same canonical slice and behavior is byte-identical; the new shape exists so T3/T4/T5 can widen acceptance per source (GH #767). Consumers are rewired to per-source reads: the reference-site IR's routing filter and the GH #752 gate key on reads_var, the gate's axis checks on canonical_read_slice (its decision is about the reducer's shape -- 'from' may be a scalar feeder), and emit_agg_routed_edges / emit_source_to_agg_link_scores enumerate rows from source_read_slice(from) (a scalar feeder's or non-source's empty slice trips the existing arity guards into the same conservative fallbacks). Byte-identity verified by a 14-fixture golden diff of emitted LTM vars, element edges, and diagnostics against the parent commit, plus the unmodified full suite. New unit tests pin the T2-scoped invariants (sorted/deduped sources, scalar-feeder empty slice, duplicate-var and differing-subset declines -- the latter two GREEN characterizations of the agreement check); I1's feeder clause is deliberately NOT pinned here (unreachable until T5 -- the GH #739 vacuity trap).
The T2 review approved the per-source refactor outright but flagged that canonical_read_slice's first-non-empty rule silently embeds the identical-only acceptance: under T5's projection feeders an alphabetically-first feeder slice would satisfy the gate's axis checks for the wrong shape. State the required T5 redefinition (first slice with a Reduced axis) on the accessor itself so the T5 implementor cannot miss it.
T3 of the shape-expressiveness design (docs/design-plans/ 2026-06-11-ltm-shape-expressiveness.md, Architecture section 6): make read_slice_rows the single derivation (invariant I4) for variable-backed reduce edges. try_cross_dimensional_link_scores now resolves the edge's variable-backed AggNode and enumerates rows/slots/co-reduced sets from the per-source read slice -- Pinned axes fixed to their literal element, subset-Reduced axes enumerated over the subset -- so MEAN/STDDEV divisors are the true read count and unread rows get no score. The full-cartesian derivation remains only for edges with no accepted agg (the dynamic-index conservative family and the GH #764 broadcast/permuted shapes), all byte-identical. The #752 gate generalizes from variable_backed_partial_reduce_agg to variable_backed_reduce_agg per section 6: any statically-describable non-trivial slice with an aligned Iterated result is admitted (Pinned and subset axes included), and scalar-result Pinned/subset slices are admitted for SCALAR owners only (the slot is the bare `to` node). Pure full-extent slices stay outside the gate (the reference walker's edges already are the read rows -- inert skip). The T1-era Pinned/subset exclusions are deleted ATOMICALLY with the derivation swap, per the design's I4 atomicity constraint: deleting them first would re-admit Pinned slices to a derivation that still divides by the full cartesian (the 0.25-vs-0.5 silent-wrong-divisor hazard, guarded by the new fixtures' 0.5 assertions). The element-graph dispatch widens its shape condition to Wildcard|DynamicIndex (the latter is the partial-StarRange coarse classifier shape, e.g. SUM(matrix[D1,*:Sub]); no DynamicIndex site could pass the old gate, so old shapes are unaffected), and the loop builder's routing shares the same predicate by construction. The one inexpressible residual -- the ARRAYED-owner scalar-result Pinned/subset broadcast slice (share[Region] = SUM(pop[nyc,*])) -- was silently wrong pre-T3 (full-cartesian per-(row, slot) garbage: constant delta-ratio +1 scores for rows the reducer never reads) and now takes the GH #758 loud skip: one Warning, no link-score variable, loop scores through the edge dropped. The libsimlin fragment-failure fixture rotates from the now-clean Pinned-mixed shape to the GH #743 co-source closure (a real organic failure that survives T3; the LtmFragmentFailureGuard hook is engine-pub(crate) and unavailable cross-crate). Byte-identity vs the parent commit was verified by diffing model_ltm_variables output, element edges, and detected loops over 13 non-flipped golden families in a temp worktree (identical), plus a new golden equation-text pin on the aligned SUM(matrix[D1,*]) emission.
T3 review follow-up. The prior commit justified widening the element-graph
dispatch's shape condition to Wildcard|DynamicIndex with the claim that no
DynamicIndex site could pass the old gate; that claim was false. The
own-dimension mixed StarRange family (out[D1] = SUM(matrix[D1,*:D2]) with
*:D2 naming the axis's own full dimension) resolves to an
all-Iterated/Reduced{None} slice with no bare `*` -- it PASSED the old gate
-- while classify_subscript_shape calls the subscript DynamicIndex, so the
old Wildcard-only dispatch refused to route it even though the loop
builder's routing (gate-only) flagged the hop: internally inconsistent
treatment, conservative cross-product element edges feeding per-circuit
loop routing, i.e. warned phantom loops (4 fail-warned 0-stubs alongside
the 4 real circuits at the parent commit). The widening makes the dispatch
consistent with the gate and the family now gets first-class read-slice
treatment; the comment at the dispatch states this corrected rationale and
the new own_dim_star_range_mixed_reduce_scores_read_slice fixture pins the
flip (zero warnings, diagonal edges, 0.5 divisor-correct scores, exactly
the 4 real loops) -- verified failing at the parent in a temp worktree.
Also pins the all-Pinned scalar-owner flip (total = SUM(pop[nyc,p]) in a
loop): in-scope intended churn that was previously unfixtured. Pre-T3 the
full-cartesian derivation emitted four scores -- three constant +1.0
delta-ratio garbage series for rows the reducer never reads alongside the
true one (verified failing at the parent); now only the single true
pop[nyc,p] score (+1) is emitted, matching the FixedIndex element edges
that were already correct.
And scopes the section-6 loud-skip rustdoc precisely: the newly-loud
decline covers arrayed owners whose dims are a SUBSET of the source's
(reaching the partial-reduce branch); the disjoint-dims sibling
(share[D9] = SUM(pop[nyc,*])) early-returns before that branch and keeps
its pre-existing loud degradation through emit_per_shape_link_scores' GH
#758 gate, unchanged. The arrayed-owner residual is tracked as GH #777.
A whole-RHS variable-backed reducer whose result dims are a BROADCAST (strict subset of the owner's dims, out[D1,D3] = SUM(matrix[D1,*])) or a PERMUTATION (same dims, different order) of the owner's dims previously kept the conservative cross-product with no matching link scores: the broadcast shape took the GH #758 loud skip (every loop through the edge dropped) and the permuted/Pinned-mix shapes flooded warned 0-stub phantom loops off the old cartesian derivation -- the Pinned-bearing mix even scored unread rows. GH #764, T4 of the shape-expressiveness design. The fix generalizes the GH #534 mapped carve-out into ONE minting condition (variable_backed_shape_is_expressible): a whole-RHS reducer is variable-backed only when its slice has no mapped Iterated axis AND its Iterated target dims equal the owner's declared dims in order (or it has no Iterated axis at all). Everything else falls through to walk_subexpr_for_aggs and mints a synthetic agg arrayed over result_dims, riding the existing two-half emitters (whose read_slice_rows derivation is Pinned-correct) and the GH #528 agg-to-target projection (which handles the broadcast fan-out; permutation is free because slots are keyed by result_dims order). The mapped case is deliberately a separate clause of the same predicate -- its Iterated axis carries the TARGET dim, so its result dims are aligned; the remap, not the shape, is what the name-keyed variable-backed path cannot express. The GH #777 arrayed-owner Pinned/subset slice (no Iterated axis) is NOT widened into: it keeps the loud-skip decline, byte-identical. The variable-backed gate's alignment check becomes defense-in-depth (no non-aligned vb agg is registered anymore). Byte-identity vs the parent commit was verified over 11 non-flipped fixture families (LTM vars, element edges, diagnostics) via a temp-worktree golden diff, and the mapped whole-RHS emission text is pinned byte-for-byte in whole_rhs_mapped_reduce_emissions_stay_byte_identical.
T4 review follow-ups. The minting predicate's rustdoc claimed the mapped (GH #534) case always has aligned result dims -- falsified: mapped and non-aligned co-occur (out[State,D3] = SUM(matrix[State,*]) is mapped AND broadcast); only the CANONICAL #534 shape is aligned. The code was already right (the mapped clause fires first, and the synthetic machinery composes the remapped source half with the GH #528 broadcast projection), but the doc would have licensed a future clause reorder trusting mapped implies aligned, which would break the canonical case. The rustdoc now states the intersection explicitly, and a new end-to-end fixture pins it (remapped 0.5 source halves, 1.0 broadcast agg halves, exactly 8 loops at the derived 0.125 product). Verified in a temp worktree that the fixture also passes at the parent commit (148a17d): the pre-T4 mapped-only condition already routed the intersection synthetic, so this is a regression pin of pre-existing behavior, not a flip. Also: the pinned-mix fixture's loop census tightens from >= 4 to exactly 6 with the derivation spelled out (4 diagonals + 2 causally real cross-D2 petal-stitched loops), pinning the no-phantom invariant hard; and the design doc's T4 section now states the emitted-var-count perf guardrail is deferred wholly to T6 (T4 adds no emissions for C-LEARN-class models), so the T6 implementor inherits it explicitly.
T5 of the shape-expressiveness design (docs/design-plans/ 2026-06-11-ltm-shape-expressiveness.md), GH #767. combined_read_slice's identical-slices requirement widens to invariant I1: co-sources (Reduced-bearing slices) must still carry one identical canonical slice, and 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])) -- is accepted as an AggSource with ITS OWN slice. The ordered-equality reading of the design's "drawn from the set" wording is deliberate: a dim-subset feeder's rows are not 1:1 with agg slots and a permuted feeder's read_slice_rows slots would mis-name result_dims-ordered slots, so both decline, as do Pinned-axis mixes, mapped Iterated axes in feeder combinations, differing co-sources, and one variable read with two slices (I3b) -- all staying on the pre-T5 loud conservative path. canonical_read_slice is redefined from "first non-empty" to "first Reduced-bearing slice" (falling back to first non-empty for the degenerate no-co-source agg) so an alphabetically-first feeder can never satisfy the variable-backed gate's axis checks for the wrong shape -- the contract flag T2's reviewer left on the rustdoc. The feeder half emits per-(row, slot) CHANGED-LAST scores (the reducer text pinned to the slot, 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, replacing the GH #743 Bare changed-last conservative score for this shape; the changed-last chooser stays for un-hoistable shapes). 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 wrong-magnitude delta-ratio fallback. Feeder edges are flagged into agg_routed_edges (off the fast A2A path) and routed per-circuit by the shared variable_backed_reduce_agg gate, so the co-source-closure loops the GH #767 body names flip from warned zero-stubs to real sustained scores (0.5 per (row, cell) circuit on the pinned fixture; per-slot additivity to exactly +1 is asserted numerically). Byte-identity vs the parent commit was verified over 14 non-flipped fixture families (LTM vars, element edges, detected loops, diagnostics) via a temp-worktree golden diff -- identical -- with the only flips the enumerated ones: the two GH #743 end-to-end tests (rewritten on the hoisted shape plus a new Pinned-axis stays-loud boundary twin) and the two pin_body_to_row unpinnable-bail unit pins (rewritten as by-name-pin assertions with genuinely-unpinnable disjoint-dim replacements). The libsimlin fragment-failure fixture rotates from the now-clean feeder closure to the Pinned-axis non-projection mix, a real organic failure -- no LtmFragmentFailureGuard re-export needed. (The design's named non-projection example, SUM(matrix[D1,*] * other[D2]), is not expressible -- the engine rejects the free reduced-axis index outright.)
T5 review fixes for the GH #767 projection-feeder acceptance. The by-name pin the co-source row partial used for a mismatched-arity feeder dep first-matched the dim name in row_dim_names, which mis-pins a REPEATED-dim co-source: matrix[D1,D1] read as SUM(matrix[*, D1] * frac[D1]) is ACCEPTED by the feeder clause (canonical [Reduced, Iterated], iterated dims [d1]), but the name "d1" is ambiguous across the axes and the first match is the REDUCED position -- the partial froze PREVIOUS(frac[d1-r1]) for slot r2 with zero warnings, a silently wrong score (reviewer-measured 3x off, per-slot additivity broken). ReducerBodyCtx now carries the live source's accepted read slice and resolve_mismatched_index_position resolves the index at the slice's ITERATED axis position -- the executed A2A coordinate the index reads, correct for repeated dims (the pinned fixture's per-slot additivity to exactly Dgrowth is the numeric guard). Without a slice (the un-hoisted cartesian families) the by-name lookup now requires UNIQUENESS, bailing to the delta-ratio fallback on ambiguity -- the pre-#767 behavior. The design doc gains the two as-landed amendments this branch established: the I1 feeder clause's "drawn from the ... set" wording is ordered EQUALITY (the set reading is internally inconsistent with the design's own 1:1-rows consequence -- a subset feeder's per-(row, slot) names are under-subscripted and its changed-last equation leaves an iterated index free; the subset shape's home is T6's broadcast machinery), and the "feeders that are not projections" boundary example SUM(matrix[D1,*] * other[D2]) is not expressible (a hard dimension error) -- the Pinned-axis mix is the real boundary inhabitant. A Pinned-bearing CANONICAL slice is explicitly in feeder scope (the Iterated-only requirement is on the feeder's slice), pinned by a new acceptance unit test and an end-to-end additivity fixture over SUM(cube[D1, c1, *] * frac[D1]). Stale T2-era test-section comments updated to name accept_source_slices. Perf (the design's T5 protocol): warmed interleaved A/B vs e872918, perf stat -r 5 on 3x LTM-enabled C-LEARN compiles (fresh salsa db each, release), two rounds: instructions +0.44%/+0.43% (213.76->214.71B, 214.38->215.29B, +-0.2%), branch-misses +0.1%, task-clock +1.8%/+0.15% (inside the +-5% noise floor; parent itself moved +0.6% between rounds) -- under the >2%-instructions / >5%-wall gates. Byte-identity vs the parent commit re-verified over the review's 6-family golden dump (identical).
T6 of the shape-expressiveness phase (docs/design-plans/2026-06-11-ltm-shape-expressiveness.md; GH #525, GH #757, GH #769). RefShape gains PerElement { axes: Vec<AxisRead> } for the iterated+literal mixed subscript (pop[Region, young] inside an A2A-over-Region equation). classify_iterated_dim_shape is rewired onto ltm_agg::classify_axis_access -- the single per-axis classifier the reducer path already uses -- with a Reduced post-filter (a non-reducer reference never collapses an axis): all-Iterated stays Bare, all-Pinned falls through to FixedIndex, and only the mixed case mints PerElement, so every existing Bare/FixedIndex link-score name is untouched. The element graph expands PerElement sites as the diagonal-with-pinned-axes rows from the shared read_slice_rows derivation (invariant I4); the pre-T6 DynamicIndex cross-product's phantom circuits -- silent confident ~0.245 loop scores in the GH #525 repro -- die at enumeration. Emission is one scalar per (row, FULL-target-element), $:ltm:link_score:{from}[{row}]->{to}[{e}] with the row a function of e (project e onto the Iterated axes, slot-remapped for mapped pairs; fill Pinned with literals), covering the broadcast case where the Iterated dims are a strict subset of the target's -- the existing per-(row, slot) grammar, so loop_link_score_ref and discovery's parse_link_offsets resolve the names unchanged. The equation builder (generate_per_element_link_equation) rewrites the live occurrence to the concrete row subscript and pins-and-freezes every other source occurrence -- the pin_body_to_row index-substitution mechanism lifted to target-equation bodies -- so a mixed Bare+PerElement edge emits BOTH forms and a hop both sites produce resolves to the PerElement scalar (the pinned resolver precedence). The loop builder routes every circuit through a PerElement edge per-circuit via is_per_element_edge, keyed on the same model_edge_shapes IR projection (the #752 single-gate pattern); the mixed scalar-bearing branch keeps both endpoint subscripts for the hop. The Expr0 sibling (classify_expr0_subscript_shape) gains the same mixed recognition so the partial builder's live-shape match agrees with the IR. GH #757: classify_axis_access's mapped arm drops the forward-declared has_mapping_to pre-gate and relies on iterated_axis_slot_elements / mapped_element_correspondence (both declaration directions, the #756 positional-only gate inherited), so reverse-declared positional pairs classify Bare (subscripted references get the diagonal the bare form already had) and reverse-declared sliced reducers hoist. GH #769 (classifier-untouched adjunct): try_disjoint_dim_arrayed_link_scores accepts ApplyToAll targets for FixedIndex-ONLY edges (one shared slot body holding from[elem] live); any other site shape returns None so those edges keep the GH #758 loud skip byte-identically. Flips, per the design's notes: the gh525 repro's merged Bare +1 score is replaced by four per-(row, element) scalars at exactly 0.5; its ApplyToAll row_sum loop becomes four per-circuit element-subscripted scalar loops; the phantom block is replaced by the precise no-plain-substring absence check; detected/scored ids still biject. element_graph_mapped_reverse_declared_subscripted and the reverse-declared sliced-reducer pins flip to the diagonal/hoisted forms, and the GH #758-era reverse-declared loud-skip e2e pin flips to a genuinely-scored diagonal. Byte-identity vs parent ad6bdeb verified over a 12-family golden dump (scalar/A2A/whole-extent/sliced/subset/mapped-forward/element-mapped/ dynamic-index/FixedIndex-arrayed/feeder/scalar-arrayed/RANK): identical vars, element edges, detected loops, and diagnostics. C-LEARN guardrail (pinned by clearn_ltm_var_count_guardrail): emitted LTM vars 6605 -> 6713 (+1.64%; nine real GH #525-family edges flip from one merged score to per-element scalars) while the #654-relevant layout row width SHRINKS 31132 -> 30928 slots (-0.66%); perf under the T5 substitute protocol reads instructions -0.3%/-0.6% across two interleaved rounds, inside all gates. The design doc gains the as-landed guardrail numbers and the correction that the cited "C-LEARN LTM compile benchmark" never existed.
T6 review follow-up for the shape-expressiveness phase (GH #525, GH #757, GH #769; docs/design-plans/2026-06-11-ltm-shape-expressiveness.md). The emit_per_shape_link_scores comment claimed a ThroughAgg-routed PerElement site's pre-T6 DynamicIndex spelling "minted an extra Bare-named conservative score alongside the agg halves" -- reviewer verification (reproduced here with parent/current probe dumps on the reviewer's aliased-routing fixture, out[R] = SUM(pop[R,*]) + MEAN(w[R,*] * pop[R,young])) shows EXHAUSTIVE mode is byte-identical pre/post: the loop-link caller emits an agg-routed hop's scores via its agg branches and never reaches the per-shape pass for the edge, so no Bare score existed there to retire. The flip the comment described is real only in DISCOVERY mode (causal-edge iteration does reach the pass; the parent emitted a duplicate Bare-named pop->out pathway alongside the agg halves there). The comment now states both halves, and aliased_through_agg_per_element_site_emits_only_agg_halves pins the family on both surfaces (exhaustive = GREEN boundary pin verified byte-identical at parent ad6bdeb; discovery = the one real flip, parent-RED on exactly that assertion). Two reviewer-probed-but-unpinned corners gain exact-value fixtures: per_element_body_with_iterated_other_dep_scores (a PerElement body referencing another arrayed dep by iterated subscript -- asserts the dimension-name pinning lands other[region-elem] in the equation text and the scalars read exactly +1; parent-RED: the merged Bare pop->row_sum existed instead) and mapped_per_element_subscript_scores_positional_diagonal (the mapped PerElement family, exercising per_element_row_for_target's mapped_element_correspondence arm that no committed test reached -- asserts the positional s1<->r1 diagonal names with SOURCE-dim rows at exactly +1, no cross/old-row names, zero warnings; parent-RED: the GH #758 loud-skip Warning fired and no pop->mid score existed). Parent-RED/GREEN evidence was gathered by grafting the three tests onto an ad6bdeb temp worktree and running them there.
When a target equation hoists two distinct arrayed reducers (to[D1] = SUM(m1[D1,*]) + SUM(m2[D1,*])), agg A's per-target-element partial froze co-agg B as a bare PREVIOUS(B) -- a multi-slot reference in a scalar equation that cannot compile, so every agg-to-target fragment failed (4 Assembly warnings on the 2x2 repro) and all agg-half link scores plus every loop score through the target silently read 0. Generalize the GH #528-era single source_pin_element on generate_scalar_to_element_equation into a per-ident pin slice: each arrayed agg referenced by the substituted equation -- the live one AND every frozen co-agg -- is pinned to the target element's projection onto ITS OWN result_dims (the same agg_pin_for_target projection the live agg already got, applied per ident). Scalar co-aggs get no entry by construction, so the GH #737-era scalar twin stays byte-identical (pinned by gh751_scalar_co_aggs_keep_bare_previous_freeze), and golden dumps of single-agg broadcast / scalar-feeder / whole-extent sentinels are byte-identical to the parent commit.
The partial builder collapsed ANY iterated-dim subscript on a non-live array dep to a bare PREVIOUS(dep) without checking index-vs-declared-dim correspondence. A transposed reference (arr[D2,D1] for arr declared [D1,D2] -- a genuine positional transposition in the executed simulation, confirmed empirically) was therefore frozen at the WRONG element: on the 2x2 repro the off-diagonal pop-to-growth scores read 0.9940/1.0059 instead of exactly 1.0, silently and warning-free. Thread the target's array deps' declared dims into IteratedDimCtx (from the per-shape salsa path, which has db access) and replace the boolean recognizer with a three-way verdict: exact position-and-mapping correspondence (or un-threadable dims -- implicit/synthetic deps keep the historical permissive collapse, per the design's GH #526 fallback clause) still collapses; a KNOWN mismatch sets a doom flag on the new WrapOutcome out-channel instead. shaped_guard_form_text then routes the mismatch to the changed-last convention -- the transposed dep stays live and verbatim, so the numerator attributes exactly the live source's change (the repro now scores exactly 1.0 in every slot) -- and the changed-first-only builders fail with the loud UnfreezablePartial rather than a silent magnitude error. The mapped lineup check reuses iterated_axis_slot_elements, so the live-source and other-dep recognizers share one positional-mapping gate; natural-position and positionally-mapped deps are pinned byte-identical, and golden dumps of the agg sentinel fixtures match the parent commit byte-for-byte.
The T7 adversarial review approved commits 3779f38/fa8fa774 but flagged three stale-doc findings: the engine CLAUDE.md still described the single source_pin_element parameter (now the per-ident source_pins map covering frozen co-aggs, GH #751); the LTM reference's changed-last note claimed exactly two trigger shapes (GH #526 adds the known position-mismatched non-live array dep as a third); and the design doc specified a loud skip for known mismatches where the implementation lands the strictly-better changed-last scoring (skip only when both legs are doomed), plus the #751 live-not-latent discovery, both now recorded as as-landed amendments.
The design promised follow-up issues at landing time in three places: the de-hoisted RANK ordering residual (filed as GH #776), the arrayed-owner broadcast reduce slice that survived the phase (GH #777), and the every-emitted-fragment-compiles prevention harness (GH #782). Record the numbers so the doc's open ends resolve to tracked work.
Extend element_graph_proptest's spec strategy with the two Phase-1 shape families the 2026-06-11 shape-expressiveness design calls out as its proptest close-out: iterated+literal MIXED subscripts (the GH #525 PerElement family, via an appended wide[Dim,Age]/mixed pair covering both the 1:1 and broadcast target shapes) and SUBSET reducers (SUM(v[*:Sub]) over a proper subdimension, GH #766, as both an arrayed pattern and a scalar-target form including the scalar-feeder interaction). Expected edges derive from read_slice_rows -- the design's invariant-I4 single row-derivation source of truth -- while new deterministic companions hand-pin literal edge names as the independent oracle so a regression in the shared derivation cannot mask itself inside the derived property expectations. Each family gets its own forced strategy + property per the GH #739 lesson (a generated pattern that never occurs in the sampled corpus must fail the test): both spec-level guards were demonstrated RED by stubbing the injection, and the invariant assertions were demonstrated sensitive by simulating a subset-widened-to-full-extent regression (caught by the new forbidden-unread-rows half of the agg routing check) and a wrong-pinned-element regression (caught by the exact pinned-diagonal edge-set equality). The projection invariant and the existing per-reducer agg-hop expectations generalize unchanged; specs without the new shapes still build byte-identical projects.
Address the adversarial-review findings on d26dfc3. The mixed-subscript block previously generated only one axis order (wide[Dim,Age] with the literal invariably on the second axis), leaving the GH #525 Pinned-first order with zero repo-wide coverage; production is per-axis with no ordering assumption, but this file guards the Phase 2 AST-walking refactor where exactly such an assumption could creep in. MixedRefTarget gains an age_first flip (wide[Age,Dim] referenced as wide[lit, Dim]), the read_slice_rows oracle swaps its axes and dim lists with it, the deterministic companion pins both flipped shapes with literal edge names, and a fixed-seed distribution guard samples the forced strategy directly so neither axis order (nor target shape) can silently stop being drawn -- the per-case forced property can only see a missing block, not a missing variant. Also corrects the oracle-strength comments: the subset expectations are genuinely DIFFERENTIAL (production agg-routed edges come from emit_agg_routed_edges' own AxisPlan enumeration, not read_slice_rows -- the invariant-I4 deviation tracked as GH #783), while the mixed family shares read_slice_rows with production, making the deterministic companion its sole independent oracle. The module doc now also names the two generative scope boundaries (inlined-only subset reducers; feeder-x-subset only against scalar targets).
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #784 +/- ##
==========================================
+ Coverage 90.79% 90.86% +0.07%
==========================================
Files 224 224
Lines 138701 140812 +2111
==========================================
+ Hits 125932 127954 +2022
- Misses 12769 12858 +89 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Review of #784 (LTM shape expressiveness, epic #488 phase 1)I ran four parallel reviewers over the largest changed surfaces (ltm_agg.rs, ltm_augment.rs, db/ltm/link_scores.rs + db/ltm/loops.rs, db/analysis.rs, db/ltm_ir.rs) and verified the most concrete findings directly. The shape-expressiveness refactor is well-architected — the single [P3] pin_iterated_dim_indices lacks the uniqueness defense its sibling resolve_mismatched_index_position carries
[P3] agg_sources silent canonical-slice fallback contradicts its debug_assert in release builds
debug_assert!(slices.per_var.contains_key(&var), ...);
slices.per_var.get(&var).cloned().unwrap_or_else(|| slices.canonical.clone())The Overall correctnessCorrect. No P0/P1/P2 issues identified. The two P3 items above are silent-fallback paths that could mask invariant breaks; they do not manifest as wrong scores under the inputs the PR tests cover. The exhaustive |
This was referenced Jun 11, 2026
bpowers
added a commit
that referenced
this pull request
Jun 12, 2026
Follow-up to #784 (supersedes #786, which conflicted because #784 was squash-merged and the old branch carried the pre-merge history). Fixes the two P3 findings #784's automated review surfaced after merge. Both harden the phase's no-silent-fallback discipline, and the first turned out to be a real reachable defect rather than defense-in-depth. **Feeder slot pinning on square-source aggs**: a degenerate square-source reducer (`x[D1] = 1 + SUM(cube[D1,D1,*] * frac[D1,D1])`) mints an agg with `result_dims == [D1, D1]`, and `pin_iterated_dim_indices` resolved indices by first-match dim name -- every duplicated-dim index pinned to the first slot part, silently freezing the wrong source row on off-diagonal feeder scores (confident garbage, zero diagnostics). It now mirrors `resolve_mismatched_index_position`'s uniqueness defense: an ambiguous index bails to the existing `UnfreezablePartial` loud-skip channel (per-row warning, no score emitted). RED-verified pre-fix at both fixture and unit level; byte-identical for every non-repeated-dim shape by construction. **`agg_sources` release-build fallback**: the helper carried a `debug_assert` that every arrayed reducer source has a per-var slice, then silently substituted the canonical slice in release if the invariant ever broke -- which would mislabel a projection feeder as a co-source and corrupt per-(row, slot) derivations. It now returns `Option` and declines the hoist entirely on a missing slice (the same inert conservative degradation as the dynamic-index carve-outs). The reviewer-suggested empty-slice fallback was rejected because an arrayed source wearing the scalar-feeder encoding would flow into the scalar-agg name grammar -- plausibly wrong rather than inert. The two AST walkers were audited as structurally symmetric, so the path is unreachable today; the defense now matches the asserted invariant. The remaining square-source hazards (co-source and agg-to-target halves still pin by dim name; phantom off-diagonal link scores reach link-level surfaces unwarned) are pre-existing and tracked as #785/#778, with the loop-score cascade gap as #780. Adversarial review of this commit: APPROVE, with the empirical co-source findings appended to #785.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 of the epic #488 root-cause burn-down: ten open ltm issues were all symptoms of one representational gap -- LTM described how an arrayed variable is accessed with several partial, locally-derived vocabularies, so each surface (classification, element-graph expansion, link-score emission, loop assembly, naming) re-derived or approximated the access independently. This PR makes
AxisRead { Pinned, Iterated, Reduced{subset} }the single per-axis access vocabulary for both reducer reads and direct references, derived by one per-axis classifier (classify_axis_access) and consumed by every surface through one row/slot derivation (read_slice_rows). Design doc (committed in this PR, with as-landed amendments per task):docs/design-plans/2026-06-11-ltm-shape-expressiveness.md.The work landed as seven independently-reviewed tasks (T1-T7), each with TDD RED fixtures first and a fresh adversarial review iterated to zero material findings, plus a phase-close proptest extension (mixed-subscript and subset-reducer generative coverage with vacuity guards, mutation-validated by its reviewer):
Reduced{subset}for StarRange subdimensions; RANK de-hoisted via thereducer_collapses_to_scalarvalue-shape precondition.AggNode.sources: Vec<AggSource>representation (byte-identical golden-pin refactor).try_cross_dimensional_link_scoresconsumesread_slice_rows; the variable-backed gate generalizes to any statically-describable non-trivial slice, with edges and scores moving together so unread rows get neither.RefShape::PerElement{axes}for iterated+literal mixed subscripts -- diagonal-with-pinned-axes edges (killing the ltm: iterated+literal mixed subscript keeps the cross-product element expansion -- phantom cross-element loops carry silent nonzero scores (rescoped: hard compile failure fixed) #525 phantom-loop pathology at enumeration), per-(row, full-target-element) scores, mapped classification gated onmapped_element_correspondencein both declaration directions, and the ltm: FixedIndex reference into a disjoint-dim ApplyToAll target is loud-skipped though recoverable via the #510 per-element construction #769 ApplyToAll FixedIndex-only widening.Standing invariants enforced throughout: no silent wrong numbers (inexpressible shapes keep one loud warning + dropped loop scores), every two-surface decision shares one predicate, byte-identity for unchanged classifications with enumerated flip-lists for intended changes. Perf held within the design's guardrails (instructions-adjudicated interleaved A/B on C-LEARN LTM compile); the C-LEARN per-step layout width shrank 0.66% while the LTM var count grew 1.64% from nine genuine #525-family edges gaining truthful per-element scores (adjudicated in the design doc's T6 amendment).
Conservative boundaries kept deliberately (element maps #756, dynamic indices, RANK ordering coupling) are documented in the design doc; residuals discovered during the phase are tracked as #776, #777, #778, #779, #780, #783, and the prevention harness as #782.
Fixes #525
Fixes #526
Fixes #751
Fixes #757
Fixes #764
Fixes #765
Fixes #766
Fixes #767
Fixes #769
Fixes #771