Check NodeId uniqueness at every pipeline boundary, and fix what that surfaces by groundlar · Pull Request #98 · cambra-dev/cambra

groundlar · 2026-08-14T16:39:39Z

Clone was derived on Expr, so it copied node_id: a subtree reaching the output at two positions left both nodes sharing one identity, which makes the pane projection ambiguous and collapses the pair into a single entry in every NodeId-keyed walk. Only one pipeline boundary — post-lowering — checked for that. This adds the other seven (post-inline, post-transact, post-letrec-run, post-desugar, post-live-read, post-lambda-elim, post-planning), fixes the two violations they surface, and then removes the class of defect entirely by making Clone freshen.

The two violations the new boundaries surfaced

simplify's zip-distribute placed the left operand on both legs with bare clones, so the legs shared an identity. It survived because the follow-on product-beta consumes one copy per leg whenever the arms read different slots (⟨.0, .1⟩); only same-slot arms (⟨.0, .0⟩) leave both live. The first leg is now the survivor, the second a freshened sibling.

transact_phase::subst_var_with freshened the whole replacement, satisfying uniqueness by deleting the occurrence's id — and the read site is user-written, so its span went too. It now root-carries, like the two subst_envs below it.

planning::groupby's key lift was flagged by the same audit and is already safe: it leaves the predicate domain through lambda_elim::run, which re-mints every node. groupby_recognition_lifts_the_key_without_aliasing pins that, so an elim that started preserving ids fails there rather than at a pane boundary.

`Clone` freshens, and the opt-out is named

A clone is a sibling, not the same node. The hand-written Clone mints a new NodeId for every node it copies and reports each (origin, fresh) pair through on_copy. The recursion is the derive's — node.clone() clones the children, each reaching the same impl — so the freshen is deep by construction and fused into the copy rather than being the second walk fresh_copy made. fresh_copy, freshen_node_ids_deep, freshen_interior_node_ids, freshen_node_id and monomorphization's freshen_clone_node_ids are all deleted; 34 call sites become a plain clone().

The decision inverts rather than disappearing. The safe default is now automatic, and a copy that is not a new node says so through clone_preserving_ids. Two shapes qualify — a snapshot taken for rollback or comparison, and a test comparing trees across a pass — plus the moves-out-of-a-borrow, where Rust forces a clone, the source is dropped, and the copy takes its position. Those last were already id-preserving here; only the spelling flips, since clone() used to preserve and fresh_copy() opted into freshening.

What it is not for is silencing a leak. An Unexplained or ParentUnknown means a copy was made with no frame open, or against an origin the table never recorded — a recording gap, whose honest fix is a bracket.

Measured, because the objection was about cost

Release, min-of-5, against the unchanged parent, on the instrumented top of the stack (so the pane gates were live):

program	compile time	peak RSS	ids
`stress_10_10_3_10`	1.067 → 1.030 s (0.97×)	+2.2%	2.09×
`stress_10_10_5_12`	3.702 → 3.472 s (0.94×)	+1.3%	1.74×
`stress_10_10_8_12`	21.631 → 20.569 s (0.95×)	+0.5%	1.36×

Compile time improves, consistently and with a mechanism: one fused walk replaces copy-then-walk. Isolating capture with CAMBRA_LINEAGE=0/1, the lineage table goes 5.2 MB → 10.3 MB on a 1 GB compile; with capture off the two arms agree to 0.05%, so the freshening itself costs no memory. Even the unmitigated arm — freshening everywhere, no opt-outs — was faster than baseline at 2–3× the ids.

Lowering records its predicates

collect_tree_ids now reaches refinement predicates, so the lowering fold has to explain them. It could not: lowering builds a predicate from already-lowered sub-expressions plus the nodes minted and copied to join them up, seals it into a Refinement via refined_data_fun, and those assembly nodes live in a type slot outside the walk_children domain. 1331 of them across 80 pipeline tests. Nearly half are copies that exist only because Clone freshens — they used to alias already-tagged main-tree ids — which is why this rides in the same commit.

LoweringContext::tag_predicate sweeps the finished predicate at the three sites that build one. A copy-frame cannot do this job: flush_into_lowering deliberately asserts a lowering frame captures no mints (a frame-flushed step carries no span, so a mint routed through one would be silently unresolvable), and a predicate is a mixed mint/copy region.

The sweep skips nodes that are already recorded, and that is the load-bearing part. The fold is last-write-wins, so a blanket sweep leaves every node perfectly explained while silently replacing its real span and label with the sweep's coarse ones — 318 nodes on the corpus, with every gate green. the_predicate_sweep_skips_already_recorded_nodes pins it, because no leak class can. assert_unique_node_ids stays narrow: it answers uniqueness, not explanation, and predicates legitimately alias main-tree ids at inline's blind spot.

What this PR cannot prove on its own

This commit gates on the lowering fold and uniquify's id-stability tripwire, and that is all — the NodeId-keyed table and the pane boundaries arrive in #107. The pane snapshots, check()'s scratch copy, the register-init stash and the type-domain discharges carry no local test; they are verified on the instrumented stack and land here on that evidence. Two markers record what the next commit must do:

TODO(mono-frame) in specialize_use: when the recorder arrives, its frame must open before the clone, since the clone is what fires on_copy — a frame entered after it leaves the whole specialization Unexplained.
TODO(predicate-domain) on lineage::preserving_ids: one legitimate caller (PredMemo's rebuild), slated for removal once predicate mints are recorded. Inference's predicate producers are deliberately out of scope here — there is no pass recorder over inference yet, so a bracket would be a no-op with no test that could fail.

Full reading, including the eight distinct intents that were hiding under one clone and the exhaustive compiler-oracle sweep of all 118 production clone sites: freshening-clone-report in the vault.

Rebased onto `main` (`7a9d1201`)

Restacked onto current main, which brought #91 (commit stores split by connected
components of transaction blocks) and #92 (await_final terminal reads).

One new commit rides here: ccl(transact): a dropped as-of read moves its body rather than freshening it. rewrite_as_of_reads' dead-binding sweep
(drop_dead_as_of_reads) does *e = (**body).clone() to drop a let whose as-of
read nothing reads. That was a move when clone() preserved ids; with the
inversion above it re-mints the whole continuation instead, stranding every id
below the dropped binding — and on a reply chain that subtree is the rest of the
program. It now mem::takes the body, which copies nothing at all.

The site arrived upstream after the clone audit in this PR ran, so it was never
one of the 118 sites that sweep classified. Read that claim as "118 as of the
pre-rebase tree"; this is the 119th, and it is the one case the sweep's own
argument predicts — a clone() that was a move, at a call site nobody re-read
when the default flipped.

The second placement of a register init is gone. The trailing
final_or_default(reg_x, init) read that placed a key's seed a second time is now
an as_of_read with no seed operand (#92), so the freshening this PR added there
had nothing left to freshen and was dropped rather than ported.

Boundary rename: assert_unique_node_ids(&expr, "post-live-read") is now
"post-as-of-read", following upstream's rename of rewrite_live_reads →
rewrite_as_of_reads.

Stale numbers. The compile-time / peak-RSS / id-ratio table above was measured
before the restack and has not been re-run. The mechanism it reports — one
fused walk replacing copy-then-walk — is unchanged by the rebase.

`simplify`'s zip-distribute placed the left operand on both legs of the distributed zip with two bare clones, so the two legs shared one identity. The pipeline boundaries never caught it because the follow-on product-beta consumes one copy per leg whenever the arms read different slots (`⟨.0, .1⟩`) — arms reading the same slot (`⟨.0, .0⟩`) beta-reduce to `⟨f0, f0⟩` and leave both copies live. The first leg is now the survivor and the second a freshened sibling. `transact_phase::subst_var_with` freshened the whole replacement, which satisfies uniqueness by deleting the occurrence's id from the output — the read site is a user-written register read, so its span went with it. It now root-carries like both `subst_env`s eight lines below: the occurrence keeps its id, the interior is freshened per occurrence. `planning::groupby`'s key lift is the third site the audit flagged, and it is already safe: it crosses out of the predicate domain, but through `lambda_elim::run`, which rebuilds the term and re-mints every node. The comment now records that the laundering is what makes the crossing safe rather than leaving the next reader to re-derive it, and a test pins the property so an elim that started preserving ids fails there rather than at a pane boundary. `design/provenance.md` gains the discipline itself under "Duplication discipline": the rule, the seven boundaries and their gating, `fresh_copy` versus root-carry, why freshen-all rather than keep-first, why at placement rather than at construction, and what a predicate-domain crossing owes. `subst_var_with` is repaired here rather than deleted. The collapse that subsumes it — one engine for all three hand-rolled root-carries, `Subst` — is its own change on top of this one, so the repair is not work a later commit reverses: it is what establishes that the three shapes agreed on root-carry *before* they were unified, which is the evidence the collapse rests on and cannot produce for itself.

…rolled copies Prototype pass over the smaller findings from the hygiene audit. `fresh_copy` now has the two call sites that predate it: lowering's chained-comparison operand and `fan_out_copy`. Both were clone-then-deep-freshen written out longhand, and grep-ability is most of what naming the shape buys. The remaining hand-rolled walks are deliberately left: `lineage.rs`'s two are tests *of* `freshen_node_ids_deep`, and `infer::solve`'s wrapper freshens an owned clone in place rather than producing one. `build_value_case_cform` keeps a copy of the last branch's body only, instead of overwriting `default_body` on every iteration — the default *is* the last branch, and the index says so. `rewrite_live_reads` gets its own boundary assert. It was covered transitively by post-lambda-elim, which reports a violation against the wrong pass; a check is only evidence about the boundary that runs it. The `mem::take` slot in channelize's feed re-binding is a throwaway, so it takes `NodeId::PLACEHOLDER` rather than minting an id the recorder would log a birth for. (The other unframed mints — `flatten_spine`, `simplify::take` — are the declare-unrecorded-deaths commit's, not this one's.) `key_init` reads back through `get(..).expect("key init present")`; indexing a `HashMap` panics without saying which invariant broke. Finally, the two eager per-branch environment freshens record what they cost: an accumulator the branch overwrites has its copy killed by the `env.insert`, so each such pair is a death the pass manufactures. They stay eager because the terminal reads the environment with a bare clone, but the next reader should know the discipline is at-placement everywhere else.

…icates A clone is a *sibling*, not the same node. The hand-written `Clone` mints a new `NodeId` for every node it copies and reports each `(origin, fresh)` pair through `on_copy`; the recursion is the derive's, so the freshen is deep by construction and **fused** into the copy rather than being the second walk `fresh_copy` used to make. `fresh_copy`, `freshen_node_ids_deep`, `freshen_interior_node_ids` and `freshen_node_id` are deleted, along with monomorphization's `freshen_clone_node_ids`; the root-carry sites collapse to `clone().re_root(id)`, which mints a root id the re-root then discards -- a stranded copy that folds as a death, not a defect. # The decision inverts rather than disappearing The safe default is now automatic, and a copy that is **not a new node** says so through `clone_preserving_ids`. Two shapes qualify: a snapshot taken for rollback or comparison, and a test comparing trees across a pass. The moves-out-of-a- borrow -- where Rust forces a clone, the source is dropped, and the copy takes its position -- are the same shape, and were *already* id-preserving here; only the spelling flips, since `clone()` used to preserve and `fresh_copy()` opted into freshening. What it is **not** for is silencing a leak. An `Unexplained` or `ParentUnknown` means a copy was made with no frame open, or against an origin the table never recorded -- a *recording* gap, whose honest fix is a bracket. Measured on the instrumented top of the stack: freshening everywhere costs no compile time and no meaningful memory, so there is no argument for suppressing a mint to quiet a gate. # Lowering records its predicates `collect_tree_ids` now reaches refinement predicates, so the lowering fold has to explain them. It could not: lowering builds a predicate from already-lowered sub-expressions plus the nodes minted and copied to join them up, seals it into a `Refinement` via `ccl_utils::refined_data_fun`, and those assembly nodes live in a type slot outside the `walk_children` domain. 1331 of them across 80 pipeline tests. Nearly half are *copies* that only exist because `Clone` freshens -- they used to alias already-tagged main-tree ids -- which is why this rides in the same commit. `LoweringContext::tag_predicate` sweeps the finished predicate at the three sites that build one. A copy-frame cannot do this job: `flush_into_lowering` deliberately asserts a lowering frame captures no mints, and a predicate is a mixed mint/copy region. **The sweep skips nodes that are already recorded, and that is the load-bearing part.** The fold is last-write-wins, so a blanket sweep leaves every node perfectly *explained* while silently replacing its real span and label with the sweep's coarse ones -- 318 nodes on the corpus, with every gate green. `the_predicate_sweep_skips_already_recorded_nodes` pins it, because no leak class can. `assert_unique_node_ids` stays narrow: it answers uniqueness, not explanation, and predicates legitimately alias main-tree ids at inline's blind spot. Inference's predicate producers are **not** covered -- there is no pass recorder over inference until the `NodeId`-keyed table lands, so a bracket there would be a no-op with no test that could fail. Tracked in the vault's `predicate-lineage-report`, SS8. # What this commit cannot prove `01-hygiene` gates on the lowering fold and uniquify's id-stability tripwire, and that is all. The pane snapshots, the scratch copy, the register-init stash and the type-domain discharges carry no local test; they are verified on the instrumented stack and land here on that evidence. `specialize_use` carries a `TODO(mono-frame)`: when the recorder arrives, its frame must open **before** the clone, since the clone is what fires `on_copy`. Measured, release, min-of-5 against the unchanged parent of the instrumented top: compile time 0.95-0.96x (faster -- the fused walk), peak RSS +0.5% to +2.2%, ids 1.4-2.1x. Full reading: vault `freshening-clone-report`.

…ning it `Clone` freshens as of this bookmark, which changes what `drop_dead_as_of_reads` means: `*e = (**body).clone()` no longer moves the continuation into the dropped `let`'s position, it re-mints the whole subtree. Every id below the dropped binding is replaced by one nothing recorded, and on a reply chain that subtree is the entire rest of the program. The body genuinely takes the position — the `let` above it is gone and the original is dropped — so this is a move, and `mem::take` says so without copying the tree at all. A preserving clone would also be correct and still pay for the copy. This site is `rewrite_as_of_reads`' dead-binding sweep, which arrived with the `await_final` work upstream after the clone audit ran, so it was never one of the 118 sites that sweep classified — the inversion reached it silently.

Replace `re_root` with `clone_at`, which builds the copy's root directly at the carried id instead of minting one and overwriting it, so a substituted occurrence no longer spends an id per read. `attach_feed_fields`' rebuilt `Let` reaches the same result through `Expr::let_in_preserving`. Drop the three `clone_preserving_ids` sites that were standing in for an ordinary duplication: lowering's comprehension fan-out and its chained-comparison operands now freshen every placement inside a copy-frame, which removes the keep-first flag threaded through both. `clone_preserving_ids`' second documented shape is a throwaway copy, not "a copy that replaces or shadows its source" — the wider wording is what licensed those sites. Rewrite provenance.md's identity sections around the properties of a `NodeId`, how uniqueness is kept, and the walks that read it. `Pass` moves to the lineage model, which is where it lives in the data. Prune the commentary that narrated ordinary clones. A clone freshens, every pass is built that way, and only the preservation sites are worth a note.

`try_lift_defer` decided its shape by destructuring: it walked the `ExprStmt` spine taking `current.node` apart and only then discovered, at the inner `Let`, whether the lift applied. Having no way to hand `bound_expr` back on that path it worked on a copy, and the copy had to keep its ids because what it yields replaces the original rather than standing beside it. Split the decision from the rewrite. `is_lift_shape` walks the spine through a borrow and answers yes or no; `lift_defer` consumes its input and is total, with the pattern it relies on stated as a `debug_assert!`. The spine loop puts the node back on the expression it moved it out of rather than rebuilding one at a carried id, which retires the hand-rolled preserve and its TODO. `lift_defer` also returns the inner handle's channel domain, read off the binding it replaces. That is what `find_defer_chan_dom` re-walked the tree to recover, so the function goes with it — the lift's call site was its only caller.

`lineage.rs`'s recorder-test preamble named `freshen_node_ids_deep`, which the freshening `Clone` replaced. The tests reach the copy hook through `Clone` itself. `fan_out_copy` and `float_comp_source_case` shared one doc comment above `fan_out_copy`, so the float's description documented the copy helper and `float_comp_source_case` had none. Each paragraph now sits above the function it describes.

) The mutability-elimination phases each carried their own substitution: a `subst_env` in `mut_elim`, another in `transact_phase`, and a single-name `subst_var_with` beside it. `Subst` already is a simultaneous map, so these were a second engine reimplementing it — and drifting, since a property fixed in one copy did not reach the others. This collapses all three onto `Subst::discharge_env_in_place` / `discharge_env`, and fixes two properties only one caller had. ## Two properties the copies got wrong **Simultaneous, not a fold.** The env form is a constructor over `Subst`, not a loop of single-name discharges: a sequential fold re-substitutes into a replacement that mentions another key, sending `{a ↦ b, b ↦ 0}` to `0` instead of `b`. Caller ranges are key-free today, but that is a property of the callers, not of the operation. **Root-carry.** Each replaced occurrence keeps its own `NodeId` — the replacement's root is built at the occurrence's id, inheriting the read site's span, and only the interior is freshened. A whole-subtree freshen discards both. ## The arrow species `Compose` has its ends recomputed from the rewritten elements: substituting a `Var` whose type was an unresolved placeholder can concretize them, and the `Compose.ty == Fun(first_domain, last_codomain)` invariant must follow. `FunKind` and the Pi binder belong to the composition rather than to its elements, so they survive — a data arrow (`⤇`) is no longer downgraded to a compute one (`⇒`). `ccl_utils::free_among` answers the free-variable question for a whole candidate set in one traversal; the per-name `is_free` made selecting a live subset of a wide environment cost `|candidates| × |expr|`. ## Rebased onto #98's freshening `Clone` #98 now makes `Clone` mint a fresh `NodeId` for every node it copies, which changes how the root-carry above is *achieved* without changing what it guarantees. Previously the replacement was cloned (sharing ids), then its interior was freshened explicitly via `freshen_interior_node_ids`, leaving the root free to take the occurrence's id. Now the clone freshens root and interior alike, and `re_root` overwrites the root with the occurrence's id immediately after. So `freshen_interior_node_ids` is gone from these sites — it is what `Clone` does — and the sequence is simply `rep.clone().re_root(occurrence_id)`. The guarantee is unchanged: each replaced occurrence keeps its own `NodeId` and its read site's span, and N reads still give N distinct roots. The one cost is that the clone mints a root id which the re-root then discards — one stranded id per substituted occurrence, which folds as a death rather than a defect, and which the occurrence itself never leaves the live set for. That trade was taken deliberately over adding a `clone_at(node_id)` constructor, to keep #98's diff minimal. --- ## Rebased onto `main` (`7a9d1201`) **The arrow-species fix landed upstream independently**, as #113 — the identical `Type::fun` → `Type::fun_like` one-liner in `subst.rs`. So this commit no longer carries that change. What it still carries on that point is the part upstream did *not* land: the two tests that pin the property (`a_compose_keeps_its_fun_kind_and_binder_across_a_discharge`, `a_data_compose_is_not_downgraded_to_a_compute_arrow` — #113's whole `subst.rs` diff is 5 insertions and no test), the failure mode spelled out at the call site, and the `rewrite_expr` doc. "The arrow species" section above should be read as what the commit now *asserts* rather than what it fixes. The claim was re-verified against upstream's `ty.rs` rather than assumed, because #113 is titled *"Relate the two function kinds by equality instead of ordering `Data` below `Compute`"* and could have moved it: `Type::fun` still stamps `None`/`Compute`, `Type::pi` still stamps `Some(n)`/`Compute`, `data_fun` still stamps `Data`, and `fun_like` copies `name` and `kind` off its exemplar. What #113 changed is how the two kinds *relate in the solver* — they are now incomparable and pinned rather than ordered, and `FunKind::Var` joined the enum — none of which touches what these constructors stamp. **The substitution collapse itself is unchanged**, and upstream's rewritten `transact_phase` (#91/#92) brought new call sites for the engines this PR deletes; they are converted to `Subst::discharge_env` the same way. `free_among`, `discharge_env_in_place`'s one-traversal selection, and root-carry are as described above.

groundlar force-pushed the skylar/provenance/01-hygiene branch 2 times, most recently from ef5fa09 to 53f1ce3 Compare August 14, 2026 18:22

groundlar marked this pull request as ready for review August 14, 2026 18:29

groundlar requested a review from a team as a code owner August 14, 2026 18:29

dpmills reviewed Aug 14, 2026

View reviewed changes

Comment thread src/ccl/design/provenance.md Outdated

Comment thread src/ccl/design/provenance.md Outdated

Comment thread src/ccl/design/provenance.md Outdated

groundlar force-pushed the skylar/provenance/01-hygiene branch from 53f1ce3 to 6643817 Compare August 18, 2026 21:01

groundlar changed the title ~~ccl: every pass yields unique NodeIds~~ Check NodeId uniqueness at every pipeline boundary, and fix what that surfaces Aug 18, 2026

groundlar force-pushed the skylar/provenance/01-hygiene branch from 6643817 to 9ec243b Compare August 19, 2026 15:44

groundlar requested a review from dpmills August 19, 2026 15:46

This was referenced Aug 19, 2026

Collapse the phase-local substitution copies onto one Subst engine #101

Merged

Record a NodeId-keyed lineage table as a byproduct of each rewrite #107

Open

groundlar added 3 commits August 20, 2026 10:36

ccl: freshen at every duplication site — a pass yields unique NodeIds

12b4c27

groundlar force-pushed the skylar/provenance/01-hygiene branch from 795dca3 to aefcbea Compare August 20, 2026 17:56

groundlar commented Aug 20, 2026

View reviewed changes

Comment thread src/ccl/expr.rs Outdated

groundlar force-pushed the skylar/provenance/01-hygiene branch from aefcbea to 6c97168 Compare August 20, 2026 18:20

dpmills reviewed Aug 20, 2026

View reviewed changes

dpmills approved these changes Aug 20, 2026

View reviewed changes

groundlar force-pushed the skylar/provenance/01-hygiene branch from 66d6c2a to 023a1b5 Compare August 21, 2026 01:23

groundlar added 5 commits August 20, 2026 19:42

groundlar force-pushed the skylar/provenance/01-hygiene branch from 023a1b5 to cee02be Compare August 21, 2026 01:43

groundlar merged commit f12ccd1 into main Aug 21, 2026
1 check passed

groundlar deleted the skylar/provenance/01-hygiene branch August 21, 2026 01:47

Navigation Menu

Uh oh!

Check NodeId uniqueness at every pipeline boundary, and fix what that surfaces - #98

Check NodeId uniqueness at every pipeline boundary, and fix what that surfaces#98
groundlar merged 8 commits into
mainfrom
skylar/provenance/01-hygiene

groundlar commented Aug 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

groundlar commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The two violations the new boundaries surfaced

Clone freshens, and the opt-out is named

Measured, because the objection was about cost

Lowering records its predicates

What this PR cannot prove on its own

Rebased onto main (7a9d1201)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

groundlar commented Aug 14, 2026 •

edited

Loading

`Clone` freshens, and the opt-out is named

Rebased onto `main` (`7a9d1201`)