compute: Re-encode LetRec read-edges to the columnar edge - #37794

CPU-51

QA LLM Review

1. MEDIUM -- Rec-binding Variable re-encode is dead work for bindings nothing reads before they are bound

src/compute/src/render.rs:964

The columnar re-encode of a rec binding's feedback Variable is built unconditionally, but a Variable only has readers while it is unbound: once the rec loop calls insert_id with the rendered value, later bindings and the body resolve Gets to that bundle instead. For any rec binding that no earlier-or-same binding reads, the VecToColumnar operator this creates ends up with no consumers on its output, yet still repacks that binding's entire collection into columnar buffers on every iteration and drops it.

// `Get`s resolve to the `Variable` only until the binding is bound, so the
// re-encode has readers exactly when some binding up to and including this
// one depends on it.
let mut variable_read = BTreeSet::new();
let mut seen = BTreeSet::new();
for rec in recs.iter() {
    seen.extend(rec.value.depends());
    if seen.contains(&Id::Local(rec.id)) {
        variable_read.insert(rec.id);
    }
}

Confirmed and fixed in compute: skip the LetRec re-encode for unread variables.

The reasoning holds as written. Variables are installed before the rec loop and each binding's real bundle replaces its variable bundle at the end of its own iteration, so while recs[j] renders, ids < j already resolve to the rendered value and only ids >= j still resolve to a Variable. Readers of v_i are therefore exactly the values of recs[0..=i]. insert_id is a plain map insert rather than an assert-absent, so dropping the pre-insert costs nothing downstream, and the extraction loop reads the bundle the rec loop stored.

Two details worth recording. RenderPlan::depends walks nested binds and the body, so it also reports ids bound inside the value, which over-approximates. That is the safe direction: at worst a re-encode is built where none is needed, which is today's behavior. And a same-stage let cannot read a rec id, since the lets loop runs before the variables exist, so restricting the scan to recs misses nothing.

Went with the check as suggested, naming the accumulator read_so_far, and guarded the insert_id rather than leaving the id out of a separate list. Both variables are still created and set, so the feedback loop and fixpoint are untouched.

with_mutually_recursive.slt (61), recursion_limit.slt (2), recursive_type_unioning.slt (8), session-window-wmr.slt (33) and introspection/relations.slt (12) pass, on this branch and again at the top of the stack after the enum collapse, along with the 26 render unit tests.

Left the outer extraction site at render.rs:1037 alone. Its re-encode runs once per outer time rather than once per iteration, and a binding the body never reads would not survive the optimizer.

_{Posted by Claude Code}