From 0939336f1bd5fad81a45a274ba0605e6e83dfd8a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 08:13:27 +0000 Subject: [PATCH 1/6] report: fold two ordinal coordinates in one pass via Quack's GroupAddr::Pair lance-graph-report planned a two-dimensional report as one population pass per partition member and called the composite-key group fold a named substrate gap. Quack already has GroupAddr::Pair, so the physical planner now picks a fold major: the widest other ordinal whose product with the fold key's domain fits domain_buffer_budget. Both coordinates then fold in ONE pass (20x30: 20 passes -> 1, 80 population scans -> 4). - Buckets, mask sets and a third ordinal stay partitions (no value-derived or multi-plane group key in Quack). - A pair-keyed SUM lowers to the NULL-preserving GroupReduce{SumI32}; its raw sink goes through Quack's normalize_group_sink (empty -> 0). Count/Min/Max already seed the report's own identities. - PhysicalPlan gains fold_major; explain labels it. Tests: new pair == per-member test over a mostly-empty space (cells, totals, hidden-axis merges; budget threshold 600 pairs / 599 does not); a disable run with the normalization removed goes red. t9 pins the pair programs plus a per-member twin; the pass-budget test moves to three ordinals. lance-graph-report 33/33, report-ogar 4/4, z8run-lance 10/10 (pivot_flow handle sizes and plan equality unchanged), clippy -D warnings clean. Board: one entry with the stack-convergence recon and the five decisions that scoped this cut. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DCEP2fZdHYdMCtcEVcTpS2 --- ...6-10-05-report-pair-key-and-stack-recon.md | 60 ++++++ .claude/board/entries/README.md | 3 +- crates/lance-graph-report/src/exec.rs | 178 ++++++++++++++---- crates/lance-graph-report/src/explain.rs | 2 + crates/lance-graph-report/tests/agnostic.rs | 121 +++++++++++- 5 files changed, 317 insertions(+), 47 deletions(-) create mode 100644 .claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md diff --git a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md new file mode 100644 index 000000000..a39261589 --- /dev/null +++ b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md @@ -0,0 +1,60 @@ +# Report folds two ordinals in one pass; stack-convergence recon (2026-10-05) + +**Status:** MEASURED — the code change and its tests are in this PR. The recon findings are VERIFIED-IN-CODE against z8run `3a8a758`, lance-graph `97a3610d`, OGAR `e5de84e`, rs-graph-llm `e824977` and rig `d165343`. It ratifies the five decisions below and nothing more. + +## The cut + +`lance-graph-report` planned two-dimensional reports as **one population pass per partition member**. Its module doc called the composite-key group fold "a named substrate gap". That gap had already closed: Quack has `GroupAddr::Pair { hi, lo, stride }`. + +The physical planner now picks a **fold major**: the widest remaining ordinal whose product with the fold key's domain fits `domain_buffer_budget`. It folds both coordinates in ONE pass, through the pair key (`exec.rs` `fold_addr`). + +| case | before | after | +|---|---|---| +| 20×30 plan | 20 passes | 1 pass | +| `population_scans` | 80 | 4 | + +- Bucket coordinates, mask-set coordinates and a third ordinal stay partitions. Quack has no value-derived or multi-plane group key. +- Quack has no pair-keyed SUM terminal, so a pair-keyed SUM lowers to the NULL-preserving `GroupReduce { SumI32 }`. Its raw sink is normalized with Quack's own `normalize_group_sink` (empty → 0, the SUM identity). Count, Min and Max seed exactly the report's identities (0, `i64::MAX`, `i64::MIN`). Honest limit: the NULL-preserving sum is defined up to `GROUP_SUM_SYM_MAX_ROWS` = 2^32 − 1 rows, one fewer than `GroupSumI32`, so a pair-keyed SUM over a plane of exactly 2^32 rows is refused, not wrapped. Values (including `i32::MIN`) are unaffected. + +**Tests:** +- New `pair_key_folds_two_ordinals_in_one_pass_and_equals_per_member_passes`: pair ≡ per-member on every cell, total and hidden-axis merge, over a mostly-empty 600-cell space. + - Budget threshold: 600 pairs, 599 does not (can-fire / can-stay-silent). + - **Disable run:** with the normalization removed, it goes red (`agnostic.rs:564`), then green when restored. +- `t9` now pins the pair programs, plus a per-member twin under a tight budget. +- The pass-budget test moved to three ordinals so it still fires at 20. + +**Other runs:** +- z8run-lance (`pivot_flow`, handle size under 128 bytes, `z8run_plan_is_the_native_plan`, zero-refold pivot): 10/10. +- report-ogar: 4/4. +- Report suite: 33/33. +- clippy `-D warnings` clean. + +## Recon findings that set the scope + +- **`pivot_flow()` already runs on Quack:** `lance-execute` → `ReportPlan::execute` → `quack::lower` → mask-risc. ReportPlan is a Quack frontend that duplicates some Quack semantics. Remaining duplicates: IN as an OR of equalities (Quack has `Filter::in_u32`), MEAN/NULL finalize (Quack has `lower_avg` + `GroupAgg::SumI32` + normalize), and its own Selection/Scalar membrane. +- **Quack has no execute and no result handle.** It stops at `Query` → `lower` → `Program`. +- **z8run-lance is not wired into z8run.** It is a workspace exclude, and `register_lance_nodes` is never called. +- **z8run-core runs a JSON row engine:** `database` (up to 1,000 rows into `FlowMessage`) → `aggregator` (group-by over JSON); `batch`/`loop` stream a population as messages. +- **graph-flow infers Kanban lifecycle from workflow status** (`storage_kanban.rs:158-173`: `Completed → Commit`, `Error → Prune`, `Commit → CognitiveWork`). This bypasses `can_transition_to`. graph-flow-kanban also no longer matches the contract (`KanbanMove.libet_offset_us`, `GateDecision {reason}`). +- **Session status is never persisted**, and the engine never builds `ExecutionStatus::Error`. +- **No carrier holds (coordinate space × version):** mask-risc `Planes`, `AbiBatch` and Quack `Query` identify populations by length. The z8run Envelope and the report generation use a free `u32`. +- **No dependency edge** between z8run, graph-flow and ogar-loco in any direction. + +## DECISION (operator, 2026-10-05) — the provenance of these five rows is that choice; the evidence is the recon above + +1. **First PR** = the two-ordinal partition → `GroupAddr::Pair`, one pass. *(This PR.)* +2. **The numeric Quack `Query` is the canonical boundary.** `bind::Draft` and `ReportPlan` are SIBLING frontends; so are a SQL frontend, an IAM frontend, a z8run island compiler and a Rig tool frontend. `Draft` stays small. A maximal pure z8run island lowers to one bound Query/program, not to "one Draft". +3. **Retire the graph-flow → KanbanColumn inference.** Keep generic session replay and resume. graph-flow emits facts and results (task completed, waiting for input, result ref, action receipt); the canonical Kanban + Revision owns lifecycle. graph-flow is not wired to the cycle-seal driver either. +4. **Enforce coordinate space × version at the population execution and result boundaries** (execute(Program, World), ResultRef, workspace). Do not put the dataset version into the `Query` IR to satisfy the invariant. Binder provenance (version-dependent CAM ordinals) is a separate question. +5. **No `Quack::execute` and no `Quack::ResultHandle` yet.** First determine who owns the execution membrane between a numeric Program and a versioned World. It is probably a backend adapter above mask-risc (#1330: "Quack owns query semantics; storage supplies capabilities"). + +**REVISIT WHEN:** a second frontend needs a capability that only `Draft` growth could give; or the execution membrane's owner is settled. + +## OPEN + +- Owner of the execution membrane (Program + World → ResultRef). It is needed by both z8run and graph-flow. +- Whether the merge-law re-roll of totals is a Quack fold or presentation. +- Whether `with_roles` may hide dimensions, since it merges over them: a GROUP BY done in the view. +- `ReportPlan::pivot`/`axis` docs say "pure metadata", but adding a coordinate changes the physical key (a data pivot). +- OGAR `handle_submit` runs `CapabilityExecutor` with no gate; the gate exists only in rs-graph-llm's `dispatch_via`. +- The `graph-flow-action-ogar` production path mints V1 (`NodeGuid::new`), reads the classid canon-low, and hardwires `guard_field_value = None`. diff --git a/.claude/board/entries/README.md b/.claude/board/entries/README.md index 663abe3ff..34c7272f8 100644 --- a/.claude/board/entries/README.md +++ b/.claude/board/entries/README.md @@ -25,11 +25,12 @@ index row, (3) no duplicate entry id. Checks 1 and 2 are deliberately opposite directions; the stranding this convention prevents shows up in exactly one of them, never both. -216 entries, 2026-08-06 .. 2026-10-05. +217 entries, 2026-08-06 .. 2026-10-05. | date | entry id | finding | file | |---|---|---|---| | 2026-10-05 | `text-to-numeric-boundary-inventory` | | [2026-10-05-text-to-numeric-boundary-inventory.md](2026-10-05-text-to-numeric-boundary-inventory.md) | +| 2026-10-05 | `report-pair-key-and-stack-recon` | | [2026-10-05-report-pair-key-and-stack-recon.md](2026-10-05-report-pair-key-and-stack-recon.md) | | 2026-10-05 | `quack-two-world-frontend` | | [2026-10-05-quack-two-world-frontend.md](2026-10-05-quack-two-world-frontend.md) | | 2026-10-05 | `quack-storage-portability` | | [2026-10-05-quack-storage-portability.md](2026-10-05-quack-storage-portability.md) | | 2026-10-04 | `D-LXC-22` | | [2026-10-04-wordnet-clam-chaoda-scope-and-grammar-read-params.md](2026-10-04-wordnet-clam-chaoda-scope-and-grammar-read-params.md) | diff --git a/crates/lance-graph-report/src/exec.rs b/crates/lance-graph-report/src/exec.rs index bf6fa9575..9f32211e6 100644 --- a/crates/lance-graph-report/src/exec.rs +++ b/crates/lance-graph-report/src/exec.rs @@ -25,6 +25,11 @@ //! * **Fold key** — one ordinal coordinate the substrate scatters into //! directly (`GroupSumI32` / `GroupReduce`). Chosen as the widest //! field-backed coordinate whose domain fits the domain-buffer budget. +//! * **Fold major** — a second ordinal coordinate folded in the SAME pass +//! through Quack's composite key [`GroupAddr::Pair`] +//! (`major × minor_domain + minor`): the widest remaining ordinal whose +//! product with the fold key's domain still fits the domain-buffer budget. +//! No composite key lane is materialized; the fused key is the substrate's. //! * **Partitions** — every other coordinate. A partition member is a //! selection conjunct (`field = m`, or a derived bucket's two range //! compares) evaluated tile by tile during the pass; nothing is @@ -42,9 +47,11 @@ //! The honest limit: a partition costs one pass per member tuple, so a plan //! whose partition side is high-cardinality AND densely observed exceeds the //! pass budget and is REFUSED with [`ReportError::PassBudget`] rather than -//! run slowly or allocated densely. The primitive that would lift it — a -//! composite-key (multi-lane) group fold in `ndarray::simd` / mask-risc — is -//! a named substrate gap, not something to hand-roll here. +//! run slowly or allocated densely. Two ordinal coordinates no longer pay it: +//! the composite-key fold this note used to name as a substrate gap now +//! exists ([`GroupAddr::Pair`]) and is used as the fold major. What still +//! costs a pass per member is a third ordinal, a derived bucket, and a mask +//! set — Quack has no value-derived or multi-plane group key. use std::collections::HashMap; use std::sync::Arc; @@ -53,7 +60,9 @@ use lance_graph_mask_risc::{ execute_extent, scratch_words_for, tile_words_for, words_for, Foreign, Out, Planes, Program, Scratch, Value, }; -use lance_graph_quack::{lower, Agg, Cmp, Col, Filter, GroupAddr, GroupAgg, Mask, Query}; +use lance_graph_quack::{ + lower, normalize_group_sink, Agg, Cmp, Col, Filter, GroupAddr, GroupAgg, Mask, Query, +}; use crate::batch::{AbiBatch, LaneData}; use crate::plan::{CoordSpec, FoldState, PhysicalKey, ReportPlan, SourceRef}; @@ -162,8 +171,13 @@ pub struct PhysicalPlan { pub extent: Option, /// Canonical dimensions. pub dims: Vec, - /// Canonical index of the fold-key dimension, if any. + /// Canonical index of the fold-key dimension (the minor, innermost key), + /// if any. pub fold_key: Option, + /// Canonical index of the fold-major dimension: a second ordinal folded + /// in the same pass through [`GroupAddr::Pair`] with the fold key as its + /// minor. `None` when no second ordinal fits the domain-buffer budget. + pub fold_major: Option, /// Canonical indices of the partition dimensions, in pass order. pub partitions: Vec, /// Fold states (index 0 is COUNT). @@ -204,6 +218,7 @@ pub struct ExecStats { struct Resolved { dims: Vec, fold_key: Option, + fold_major: Option, partitions: Vec, states: Vec, state_lanes: Vec>, @@ -314,7 +329,26 @@ fn resolve( }) .max_by_key(|(i, d)| (d.domain, std::cmp::Reverse(*i))) .map(|(i, _)| i); - let partitions = (0..dims.len()).filter(|&i| Some(i) != fold_key).collect(); + // The fold major: the widest other ordinal whose composite universe with + // the fold key still fits the same domain-buffer budget. An empty + // universe is never paired (a GroupReduce sink must be non-empty). + let fold_major = fold_key.and_then(|k| { + let minor = u64::from(dims[k].domain); + dims.iter() + .enumerate() + .filter(|&(i, d)| { + let product = minor * u64::from(d.domain); + i != k + && matches!(d.provider, Provider::OrdinalLane { .. }) + && product > 0 + && product <= u64::from(policy.domain_buffer_budget) + }) + .max_by_key(|(i, d)| (d.domain, std::cmp::Reverse(*i))) + .map(|(i, _)| i) + }); + let partitions = (0..dims.len()) + .filter(|&i| Some(i) != fold_key && Some(i) != fold_major) + .collect(); let mut states = vec![FoldState::Count]; for m in &key.measures { @@ -340,30 +374,55 @@ fn resolve( Ok(Resolved { dims, fold_key, + fold_major, partitions, states, state_lanes, }) } +/// The group address of the fold: the fold key alone, or the fold major and +/// the fold key fused into Quack's composite [`GroupAddr::Pair`]. +fn fold_addr(r: &Resolved) -> Option { + let k = r.fold_key?; + let lo = Col(fold_key_lane(&r.dims[k])); + Some(match r.fold_major { + None => GroupAddr::Local(lo), + Some(m) => GroupAddr::Pair { + hi: Col(fold_key_lane(&r.dims[m])), + lo, + stride: r.dims[k].domain, + }, + }) +} + /// The aggregate a fold state lowers to, keyed or scalar. -fn agg_for(state: &FoldState, lane: Option, key: Option) -> Agg { +/// +/// A SUM keyed by a single lane is the coalescing `GroupSumI32`. Quack has no +/// pair-keyed SUM terminal, so a pair-keyed SUM is the NULL-preserving +/// `GroupReduce { SumI32 }` — whose sink must be normalized (see +/// [`needs_normalize`]) before its slots are this crate's fold states. +fn agg_for(state: &FoldState, lane: Option, key: Option) -> Agg { let v = lane.map(Col); - match (key.map(Col), state) { - (Some(k), FoldState::Count) => Agg::GroupReduce { - key: GroupAddr::Local(k), + match (key, state) { + (Some(key), FoldState::Count) => Agg::GroupReduce { + key, agg: GroupAgg::Count, }, - (Some(k), FoldState::Sum(_)) => Agg::GroupSumI32 { + (Some(GroupAddr::Local(k)), FoldState::Sum(_)) => Agg::GroupSumI32 { key: k, val: v.expect("lane"), }, - (Some(k), FoldState::Min(_)) => Agg::GroupReduce { - key: GroupAddr::Local(k), + (Some(key), FoldState::Sum(_)) => Agg::GroupReduce { + key, + agg: GroupAgg::SumI32(v.expect("lane")), + }, + (Some(key), FoldState::Min(_)) => Agg::GroupReduce { + key, agg: GroupAgg::MinI32(v.expect("lane")), }, - (Some(k), FoldState::Max(_)) => Agg::GroupReduce { - key: GroupAddr::Local(k), + (Some(key), FoldState::Max(_)) => Agg::GroupReduce { + key, agg: GroupAgg::MaxI32(v.expect("lane")), }, (None, FoldState::Count) => Agg::Count, @@ -373,6 +432,20 @@ fn agg_for(state: &FoldState, lane: Option, key: Option) -> Agg { } } +/// The grouped fold whose raw sink is NOT already a fold state of this +/// crate: the NULL-preserving SUM leaves a group no row reached at the +/// substrate's empty marker. Every other keyed fold seeds exactly +/// [`FoldState::identity`] (0 / `i64::MAX` / `i64::MIN`). +fn needs_normalize(agg: &Agg) -> Option { + match agg { + Agg::GroupReduce { + agg: g @ GroupAgg::SumI32(_), + .. + } => Some(*g), + _ => None, + } +} + fn scalar_of(state: &FoldState, v: Value) -> i64 { match v { Value::Count(n) => n as i64, @@ -501,7 +574,7 @@ impl ReportPlan { } else { base }; - let key_lane = r.fold_key.map(|i| fold_key_lane(&r.dims[i])); + let key_addr = fold_addr(&r); let first_pass = r .states .iter() @@ -509,7 +582,7 @@ impl ReportPlan { .map(|(s, &l)| { lower(&Query { filter: conj(&fbase, &first_members), - agg: agg_for(s, l, key_lane), + agg: agg_for(s, l, key_addr), }) }) .collect::, _>>()?; @@ -520,6 +593,7 @@ impl ReportPlan { extent, dims: r.dims, fold_key: r.fold_key, + fold_major: r.fold_major, partitions: r.partitions, states: r.states, accumulator, @@ -591,8 +665,11 @@ impl ReportPlan { lanes: &lanes, }; - let key_lane = r.fold_key.map(|i| fold_key_lane(&r.dims[i])); - let key_domain = r.fold_key.map_or(1, |i| r.dims[i].domain as usize); + let key_addr = fold_addr(&r); + // The fold's group universe: the fold key's domain, times the fold + // major's when the pair key is in use (bounded by the domain budget). + let minor_domain = r.fold_key.map_or(1, |i| r.dims[i].domain as usize); + let key_domain = minor_domain * r.fold_major.map_or(1, |i| r.dims[i].domain as usize); // One program run: lowers, sizes branch-private scratch, executes. let run = |filter: Filter, @@ -600,10 +677,9 @@ impl ReportPlan { out: Option<&mut [i64]>, stats: &mut ExecStats| -> Result { - let prog = lower(&Query { - filter, - agg: agg_for(&r.states[s], r.state_lanes[s], key_lane), - })?; + let agg = agg_for(&r.states[s], r.state_lanes[s], key_addr); + let normalize = needs_normalize(&agg); + let prog = lower(&Query { filter, agg })?; let mut scratch = Scratch::for_program(&prog, n)?; if prog.requires_scratch() { let b = scratch_words_for(tile_words_for(n), prog.scratch_slots as usize) @@ -613,18 +689,32 @@ impl ReportPlan { } stats.population_scans += 1; stats.tile_mask_ops += prog.ops.len() as u64; - let out = match out { - Some(o) => Out::I64(o), - None => Out::None, - }; - Ok(execute_extent( - &prog, - &planes, - &Foreign::NONE, - &mut scratch, - out, - extent.clone(), - )?) + match out { + Some(o) => { + let v = execute_extent( + &prog, + &planes, + &Foreign::NONE, + &mut scratch, + Out::I64(&mut *o), + extent.clone(), + )?; + if let Some(g) = normalize { + // Empty group -> 0, the SUM identity; the presence + // mask is not needed — COUNT already defines EMPTY. + normalize_group_sink(g.fold(), o); + } + Ok(v) + } + None => Ok(execute_extent( + &prog, + &planes, + &Foreign::NONE, + &mut scratch, + Out::None, + extent.clone(), + )?), + } }; let dims_meta: Vec = pp @@ -659,12 +749,17 @@ impl ReportPlan { budget: policy.pass_budget, }); } - // Storage order: partitions (pass order), then the fold key - // innermost so each pass writes one contiguous run. + // Storage order: partitions (pass order), then the fold major, + // then the fold key innermost, so each pass writes one + // contiguous run — `major × minor_domain + minor` is exactly + // the pair key's group index. let mut strides = vec![0usize; r.dims.len()]; let mut stride = 1usize; if let Some(k) = r.fold_key { strides[k] = 1; + if let Some(m) = r.fold_major { + strides[m] = minor_domain; + } stride = key_domain; } for &p in r.partitions.iter().rev() { @@ -790,7 +885,14 @@ impl ReportPlan { coords[p].push(m); } if let (Some(kd), Some(k)) = (r.fold_key, k) { - coords[kd].push(k); + match r.fold_major { + None => coords[kd].push(k), + Some(md) => { + let minor = minor_domain as u32; + coords[md].push(k / minor); + coords[kd].push(k % minor); + } + } } for (s, v) in vals.iter_mut().enumerate() { v.push(st(s)); diff --git a/crates/lance-graph-report/src/explain.rs b/crates/lance-graph-report/src/explain.rs index dcec09c39..6c1ed6c95 100644 --- a/crates/lance-graph-report/src/explain.rs +++ b/crates/lance-graph-report/src/explain.rs @@ -57,6 +57,8 @@ impl fmt::Display for PhysicalPlan { }; let role = if self.fold_key == Some(i) { "fold key" + } else if self.fold_major == Some(i) { + "fold major (pair key, same pass)" } else { "partition" }; diff --git a/crates/lance-graph-report/tests/agnostic.rs b/crates/lance-graph-report/tests/agnostic.rs index b5aa30d1c..89e7a8b27 100644 --- a/crates/lance-graph-report/tests/agnostic.rs +++ b/crates/lance-graph-report/tests/agnostic.rs @@ -403,8 +403,10 @@ fn t8_json_and_csv_differ_only_at_the_terminal() { fn t9_native_plan_lowers_to_exactly_the_programs_quack_would_write() { let fx = synthetic(2_000, &[6, 9], 19); let batch = fx.batch(); - // "WHERE V >= 0 GROUP BY A, B SUM(V)" — A (6) is a partition, B (9) - // the fold key (wider). Pass 0 is A = 0. + // "WHERE V >= 0 GROUP BY A, B SUM(V)" — B (9) is the fold key (wider), + // A (6) the fold major: ONE pass over Quack's composite key + // `A × 9 + B`. Quack has no pair-keyed SUM terminal, so the SUM is the + // NULL-preserving `GroupReduce { SumI32 }`. let plan = ReportPlan::over(src()) .filter(Selection::cmp(V, CmpOp::Ge, Scalar::Int(0))) .pivot(&[fa()], &[fb()]) @@ -420,15 +422,21 @@ fn t9_native_plan_lowers_to_exactly_the_programs_quack_would_write() { .unwrap(); // Lanes: F0 → 0, F1 → 1, F100 → 2 (fixture order). let where_ = Filter::Cmp(Col(2), Cmp::GeI32(0)); - let pass0 = Filter::And(vec![where_, Filter::Cmp(Col(0), Cmp::EqU32(0))]); + let pair = lance_graph_quack::GroupAddr::Pair { + hi: Col(0), + lo: Col(1), + stride: 9, + }; + assert_eq!(pp.passes, 1, "two ordinals fold in one pass"); + let pass0 = where_.clone(); let quack: Vec = [ Agg::GroupReduce { - key: lance_graph_quack::GroupAddr::Local(Col(1)), + key: pair, agg: lance_graph_quack::GroupAgg::Count, }, - Agg::GroupSumI32 { - key: Col(1), - val: Col(2), + Agg::GroupReduce { + key: pair, + agg: lance_graph_quack::GroupAgg::SumI32(Col(2)), }, ] .into_iter() @@ -462,15 +470,112 @@ fn t9_native_plan_lowers_to_exactly_the_programs_quack_would_write() { .first_pass, quack ); + + // Twin: a budget too small for the 54-group pair universe (but wide + // enough for B alone) keeps the per-member shape — A is a partition, B + // the single-lane key, and pass 0 is A = 0. Same native == Quack law. + let narrow = PlannerPolicy { + reuse_mask_min_programs: u64::MAX, + domain_buffer_budget: 53, + ..PlannerPolicy::default() + }; + let pp = plan.explain(&batch, &narrow).unwrap(); + assert_eq!(pp.fold_major, None); + assert_eq!(pp.passes, 6); + let pass0 = Filter::And(vec![where_, Filter::Cmp(Col(0), Cmp::EqU32(0))]); + let per_member: Vec = [ + Agg::GroupReduce { + key: lance_graph_quack::GroupAddr::Local(Col(1)), + agg: lance_graph_quack::GroupAgg::Count, + }, + Agg::GroupSumI32 { + key: Col(1), + val: Col(2), + }, + ] + .into_iter() + .map(|agg| { + lower(&Query { + filter: pass0.clone(), + agg, + }) + .unwrap() + }) + .collect(); + assert_eq!(pp.first_pass, per_member); +} + +// ── PAIR KEY ≡ PER-MEMBER PASSES ──────────────────────────────────────── +// Two ordinal coordinates fold in ONE pass through Quack's composite key. +// Every cell, every total, and every hidden-axis merge must equal the +// per-member plan's — including empty cells, where the pair-keyed SUM's raw +// sink holds the substrate's NULL marker until it is normalized to 0. +#[test] +fn pair_key_folds_two_ordinals_in_one_pass_and_equals_per_member_passes() { + // 300 rows over 20 × 30 = 600 cells: most cells are EMPTY (anti-vacuity + // for the SUM normalization), with negative values present. + let fx = synthetic(300, &[20, 30], 22); + let batch = fx.batch(); + let plan = with_four(ReportPlan::over(src()).pivot(&[fa()], &[fb()]), V); + let pair_policy = PlannerPolicy { + domain_buffer_budget: 600, + ..PlannerPolicy::default() + }; + let split_policy = PlannerPolicy { + domain_buffer_budget: 599, + ..PlannerPolicy::default() + }; + // The budget knob decides the shape and nothing else: 600 pairs, 599 + // does not (can-fire / can-stay-silent at the exact threshold). + let pp = plan.explain(&batch, &pair_policy).unwrap(); + assert_eq!(pp.dims[pp.fold_key.unwrap()].coord, fb()); + assert_eq!(pp.dims[pp.fold_major.unwrap()].coord, fa()); + assert!(pp.partitions.is_empty()); + assert_eq!(pp.passes, 1); + assert!(pp.to_string().contains("fold major")); + let sp = plan.explain(&batch, &split_policy).unwrap(); + assert_eq!(sp.fold_major, None); + assert_eq!(sp.passes, 20); + + let (pair, ps) = plan.execute(&batch, &pair_policy).unwrap(); + let (split, ss) = plan.execute(&batch, &split_policy).unwrap(); + assert_eq!(ps.population_scans, 4, "one pass × four fold states"); + assert_eq!(ss.population_scans, 20 * 4); + assert_matches_oracle(&fx, &plan, &pair, V); + + let empty = fx.n < 600 && fx.oracle(&plan, Some(V)).len() < 600; + assert!(empty, "the fixture must leave cells empty"); + for m in pair.measures() { + for a in 0..20 { + for b in 0..30 { + assert_eq!( + pair.value(m, &[], &[a], &[b]), + split.value(m, &[], &[a], &[b]), + "{m:?} at ({a},{b})" + ); + } + } + // Totals merge stored slots, so an un-normalized empty SUM slot would + // corrupt them even though each empty cell reads NULL. + assert_eq!(pair.grand_total(m), split.grand_total(m), "{m:?} total"); + let pr = pair.with_roles(&[fa()], &[], &[]).unwrap(); + let sr = split.with_roles(&[fa()], &[], &[]).unwrap(); + for a in 0..20 { + assert_eq!(pr.value(m, &[], &[a], &[]), sr.value(m, &[], &[a], &[])); + } + } } // ── knob inertness: pass and domain budgets can fire, and stay silent ─── #[test] fn pass_budget_refuses_instead_of_running_slowly_and_is_silent_when_met() { - let fx = synthetic(1_000, &[20, 30], 20); + // Three ordinals: C (40) keys the fold, B (30) is the fold major, and + // A (20) is the one partition left — 20 passes. + let fx = synthetic(1_000, &[20, 30, 40], 20); let batch = fx.batch(); let plan = ReportPlan::over(src()) .pivot(&[fa()], &[fb()]) + .axis(fc(), AxisRole::Page) .measure(Measure::count()); let tight = PlannerPolicy { pass_budget: 19, From ae1d633b7b38457417ddaafb2dd44f7ed1737b06 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 08:41:20 +0000 Subject: [PATCH 2/6] report: domain_buffer_budget doc names the pair universe; record greedy pair choice as OPEN Review nit: the knob now also bounds the product of the fold key's and the fold major's domains, not only a single coordinate's domain. Doc-only. The entry gains the review's non-blocking finding: the pair choice is greedy (widest key first), so 100x60x60 at budget 3600 runs 3600 passes where a joint (key, major) choice would run 100. REVISIT WHEN the planner is next touched; kept out of this PR to keep the fold key unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DCEP2fZdHYdMCtcEVcTpS2 --- .../entries/2026-10-05-report-pair-key-and-stack-recon.md | 2 ++ crates/lance-graph-report/src/exec.rs | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md index a39261589..e6bf4633d 100644 --- a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md +++ b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md @@ -52,6 +52,8 @@ The physical planner now picks a **fold major**: the widest remaining ordinal wh ## OPEN +- **Pair choice is greedy, not pass-minimal** (found in review). Today the planner takes the widest ordinal as the fold key, then the widest ordinal that fits with it. With domains 100 × 60 × 60 and a 3600 budget, 100 × 60 does not fit, so there is no pair, and 60 × 60 = 3600 partition passes run. Folding 60 × 60 and partitioning over 100 would need only 100 passes. **REVISIT WHEN** the planner is next touched: choose `(fold key, fold major)` jointly to minimise passes. That is "the best exact fold universe" — still Quack's Pair, no new abstraction. Kept out of #1331 on purpose, so the fold key stays exactly as it was. + - Owner of the execution membrane (Program + World → ResultRef). It is needed by both z8run and graph-flow. - Whether the merge-law re-roll of totals is a Quack fold or presentation. - Whether `with_roles` may hide dimensions, since it merges over them: a GROUP BY done in the view. diff --git a/crates/lance-graph-report/src/exec.rs b/crates/lance-graph-report/src/exec.rs index 9f32211e6..f1b59a41a 100644 --- a/crates/lance-graph-report/src/exec.rs +++ b/crates/lance-graph-report/src/exec.rs @@ -76,7 +76,10 @@ use crate::ReportError; pub struct PlannerPolicy { /// Largest coordinate product stored densely. pub dense_cell_budget: u64, - /// Largest single-coordinate domain a fold-key / discovery buffer may span. + /// Largest fold/group universe or discovery buffer the planner may + /// allocate: one fold key's domain, the PRODUCT of the fold key's and the + /// fold major's domains when they fold through [`GroupAddr::Pair`], or one + /// partition's discovery domain. pub domain_buffer_budget: u32, /// Most passes over the population one report may make. pub pass_budget: u64, From f83da40fce0268392d7ccaa5e33fcf0c555e75be Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 09:19:38 +0000 Subject: [PATCH 3/6] report: revert the Pair execution path; name the real gap (destination-resolving keyed fold) Correction before merge. GroupAddr::Pair addresses the dense product of two dimension domains; lowering every two-ordinal report to it made the Cartesian product the execution geometry because the primitive existed, not because a dense cube was demanded. The intended fold model resolves each row's accumulator destination directly, with accumulator state scaling with observed destinations (PowerShell hashtable / Excel pivot), not with |A| x |B|. - Reverted fold_major, fold_addr, Pair SUM normalization, the product budget, Pair strides/decode and the Pair-pinning tests: Report's planner is main's again. - exec.rs's gap doc now names the real missing primitive: a destination-resolving keyed fold (resolved key tuple -> compact slot in the same pass), substrate-first in ndarray's keyed-reduction family. Pair stays in Quack/mask-risc/ndarray for explicitly dense problems. - The board entry keeps the Pair cut as a superseded record and states why; decision 1 is struck; decisions 2-5 stand. lance-graph-report 32/32, fmt, clippy -D warnings clean. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DCEP2fZdHYdMCtcEVcTpS2 --- ...6-10-05-report-pair-key-and-stack-recon.md | 23 ++- crates/lance-graph-report/src/exec.rs | 190 +++++------------- crates/lance-graph-report/src/explain.rs | 2 - crates/lance-graph-report/tests/agnostic.rs | 121 +---------- 4 files changed, 73 insertions(+), 263 deletions(-) diff --git a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md index e6bf4633d..6d05dc37e 100644 --- a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md +++ b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md @@ -1,8 +1,21 @@ # Report folds two ordinals in one pass; stack-convergence recon (2026-10-05) -**Status:** MEASURED — the code change and its tests are in this PR. The recon findings are VERIFIED-IN-CODE against z8run `3a8a758`, lance-graph `97a3610d`, OGAR `e5de84e`, rs-graph-llm `e824977` and rig `d165343`. It ratifies the five decisions below and nothing more. +**Status:** ⊘ SUPERSEDED IN PART (2026-10-05, same PR, before merge) — the Pair execution path described under "The cut" was REVERTED; this PR now ships only the corrected gap doc in `exec.rs` and this record. See "Correction" below. Recon findings and decisions 2–5 stand. ~~MEASURED — the code change and its tests are in this PR.~~ The recon findings are VERIFIED-IN-CODE against z8run `3a8a758`, lance-graph `97a3610d`, OGAR `e5de84e`, rs-graph-llm `e824977` and rig `d165343`. It ratifies the five decisions below and nothing more. -## The cut +## Correction (2026-10-05) — why the cut below was superseded + +`GroupAddr::Pair` addresses `hi · stride + lo`: the **dense product** of two dimension domains. Lowering every two-ordinal report to it made the Cartesian product the execution geometry because the primitive existed, not because any consumer demanded a dense cube. The intended fold model is the PowerShell-Hashtable / Excel-Pivot one: a row resolves its accumulator **destination** directly, and accumulator state scales with **observed/demanded destinations**. For IAM-sized dimensions (64k × 64k) a Pair sink is 4·10⁹ slots; a destination-resolving fold touches only the observed pairs. + +- **What stays true:** Pair was useful *evidence* that Report re-implemented what Quack can fold, and the old doc's "substrate gap" claim was stale. +- **What was wrong:** Pair is not the general Pivot/Join execution model. A destination-resolving keyed fold is. +- **What the source shows (VERIFIED-IN-CODE):** + - no compact tuple → slot resolver exists on any fold path. ndarray/mask-risc/Quack have ONE keyed-reduction walker with three addresses — `Lane`, `Via` (`table[fk[i]]`), `Pair` — all writing a dense K-slot sink (ndarray `simd_masking_ops.rs` `masked_group_*`, mask-risc `ir.rs` `GroupKey`, Quack `GroupAddr`, lgj `plan_lower.rs` Local/Via). + - The only compact structure is Report's `Layout::Sparse` RESULT index, reached by rescanning once per observed partition member. +- **Reverted:** `fold_major`, `fold_addr`, the Pair SUM normalization, the product budget, the Pair strides and decode, the Pair-pinning tests. Report is back to main's planner. +- **Kept:** `GroupAddr::Pair` in Quack/mask-risc/ndarray (correct for an explicitly dense mixed-radix problem; untouched here). +- **Shipped:** `exec.rs`'s gap doc now names the real gap: a destination-resolving keyed fold, substrate-first in ndarray's keyed-reduction family. + +## The cut (⊘ SUPERSEDED — reverted before merge; kept as the record of what was tried) `lance-graph-report` planned two-dimensional reports as **one population pass per partition member**. Its module doc called the composite-key group fold "a named substrate gap". That gap had already closed: Quack has `GroupAddr::Pair { hi, lo, stride }`. @@ -42,7 +55,7 @@ The physical planner now picks a **fold major**: the widest remaining ordinal wh ## DECISION (operator, 2026-10-05) — the provenance of these five rows is that choice; the evidence is the recon above -1. **First PR** = the two-ordinal partition → `GroupAddr::Pair`, one pass. *(This PR.)* +1. ~~**First PR** = the two-ordinal partition → `GroupAddr::Pair`, one pass.~~ ⊘ SUPERSEDED (operator, same day): Pair is not the fold model; this PR is reduced to revert + corrected record. See "Correction". 2. **The numeric Quack `Query` is the canonical boundary.** `bind::Draft` and `ReportPlan` are SIBLING frontends; so are a SQL frontend, an IAM frontend, a z8run island compiler and a Rig tool frontend. `Draft` stays small. A maximal pure z8run island lowers to one bound Query/program, not to "one Draft". 3. **Retire the graph-flow → KanbanColumn inference.** Keep generic session replay and resume. graph-flow emits facts and results (task completed, waiting for input, result ref, action receipt); the canonical Kanban + Revision owns lifecycle. graph-flow is not wired to the cycle-seal driver either. 4. **Enforce coordinate space × version at the population execution and result boundaries** (execute(Program, World), ResultRef, workspace). Do not put the dataset version into the `Query` IR to satisfy the invariant. Binder provenance (version-dependent CAM ordinals) is a separate question. @@ -52,8 +65,10 @@ The physical planner now picks a **fold major**: the widest remaining ordinal wh ## OPEN -- **Pair choice is greedy, not pass-minimal** (found in review). Today the planner takes the widest ordinal as the fold key, then the widest ordinal that fits with it. With domains 100 × 60 × 60 and a 3600 budget, 100 × 60 does not fit, so there is no pair, and 60 × 60 = 3600 partition passes run. Folding 60 × 60 and partitioning over 100 would need only 100 passes. **REVISIT WHEN** the planner is next touched: choose `(fold key, fold major)` jointly to minimise passes. That is "the best exact fold universe" — still Quack's Pair, no new abstraction. Kept out of #1331 on purpose, so the fold key stays exactly as it was. +- ⊘ SUPERSEDED with the Pair path — **Pair choice is greedy, not pass-minimal** (found in review). Today the planner takes the widest ordinal as the fold key, then the widest ordinal that fits with it. With domains 100 × 60 × 60 and a 3600 budget, 100 × 60 does not fit, so there is no pair, and 60 × 60 = 3600 partition passes run. Folding 60 × 60 and partitioning over 100 would need only 100 passes. **REVISIT WHEN** the planner is next touched: choose `(fold key, fold major)` jointly to minimise passes. That is "the best exact fold universe" — still Quack's Pair, no new abstraction. Kept out of #1331 on purpose, so the fold key stays exactly as it was. +- **The missing primitive:** a destination-resolving keyed fold (resolved key tuple → compact slot in the same pass; keys SoA + value columns ∝ observed destinations). Layer order: ndarray keyed-reduction family → mask-risc `GroupKey` + an `Out` carrying keys and values → Quack `GroupAddr`; Report and lgj consume it. Contract first — see the fold-contract recon. +- **`CoordSpec` has no functional-reference coordinate:** Report cannot group by `user → department` although `GroupAddr::Via` / `Filter::EqU32Via` already exist. A separable small step. - Owner of the execution membrane (Program + World → ResultRef). It is needed by both z8run and graph-flow. - Whether the merge-law re-roll of totals is a Quack fold or presentation. - Whether `with_roles` may hide dimensions, since it merges over them: a GROUP BY done in the view. diff --git a/crates/lance-graph-report/src/exec.rs b/crates/lance-graph-report/src/exec.rs index f1b59a41a..1f36cb2d6 100644 --- a/crates/lance-graph-report/src/exec.rs +++ b/crates/lance-graph-report/src/exec.rs @@ -25,11 +25,6 @@ //! * **Fold key** — one ordinal coordinate the substrate scatters into //! directly (`GroupSumI32` / `GroupReduce`). Chosen as the widest //! field-backed coordinate whose domain fits the domain-buffer budget. -//! * **Fold major** — a second ordinal coordinate folded in the SAME pass -//! through Quack's composite key [`GroupAddr::Pair`] -//! (`major × minor_domain + minor`): the widest remaining ordinal whose -//! product with the fold key's domain still fits the domain-buffer budget. -//! No composite key lane is materialized; the fused key is the substrate's. //! * **Partitions** — every other coordinate. A partition member is a //! selection conjunct (`field = m`, or a derived bucket's two range //! compares) evaluated tile by tile during the pass; nothing is @@ -47,11 +42,16 @@ //! The honest limit: a partition costs one pass per member tuple, so a plan //! whose partition side is high-cardinality AND densely observed exceeds the //! pass budget and is REFUSED with [`ReportError::PassBudget`] rather than -//! run slowly or allocated densely. Two ordinal coordinates no longer pay it: -//! the composite-key fold this note used to name as a substrate gap now -//! exists ([`GroupAddr::Pair`]) and is used as the fold major. What still -//! costs a pass per member is a third ordinal, a derived bucket, and a mask -//! set — Quack has no value-derived or multi-plane group key. +//! run slowly or allocated densely. The primitive that would lift it is a +//! named substrate gap, not something to hand-roll here: a +//! **destination-resolving keyed fold** — a keyed-reduction address in +//! `ndarray::simd` / mask-risc that maps a row's resolved key tuple to a +//! COMPACT accumulator slot in the same pass, so accumulator state scales with +//! the observed destinations, never with the product of the dimension domains. +//! (A composite mixed-radix key — Quack's `GroupAddr::Pair`, `hi · stride + +//! lo` — exists, but it addresses the dense product and is therefore NOT that +//! primitive; this planner does not lower to it. It belongs only to a +//! problem that explicitly demands the dense cube.) use std::collections::HashMap; use std::sync::Arc; @@ -60,9 +60,7 @@ use lance_graph_mask_risc::{ execute_extent, scratch_words_for, tile_words_for, words_for, Foreign, Out, Planes, Program, Scratch, Value, }; -use lance_graph_quack::{ - lower, normalize_group_sink, Agg, Cmp, Col, Filter, GroupAddr, GroupAgg, Mask, Query, -}; +use lance_graph_quack::{lower, Agg, Cmp, Col, Filter, GroupAddr, GroupAgg, Mask, Query}; use crate::batch::{AbiBatch, LaneData}; use crate::plan::{CoordSpec, FoldState, PhysicalKey, ReportPlan, SourceRef}; @@ -76,10 +74,7 @@ use crate::ReportError; pub struct PlannerPolicy { /// Largest coordinate product stored densely. pub dense_cell_budget: u64, - /// Largest fold/group universe or discovery buffer the planner may - /// allocate: one fold key's domain, the PRODUCT of the fold key's and the - /// fold major's domains when they fold through [`GroupAddr::Pair`], or one - /// partition's discovery domain. + /// Largest single-coordinate domain a fold-key / discovery buffer may span. pub domain_buffer_budget: u32, /// Most passes over the population one report may make. pub pass_budget: u64, @@ -174,13 +169,8 @@ pub struct PhysicalPlan { pub extent: Option, /// Canonical dimensions. pub dims: Vec, - /// Canonical index of the fold-key dimension (the minor, innermost key), - /// if any. + /// Canonical index of the fold-key dimension, if any. pub fold_key: Option, - /// Canonical index of the fold-major dimension: a second ordinal folded - /// in the same pass through [`GroupAddr::Pair`] with the fold key as its - /// minor. `None` when no second ordinal fits the domain-buffer budget. - pub fold_major: Option, /// Canonical indices of the partition dimensions, in pass order. pub partitions: Vec, /// Fold states (index 0 is COUNT). @@ -221,7 +211,6 @@ pub struct ExecStats { struct Resolved { dims: Vec, fold_key: Option, - fold_major: Option, partitions: Vec, states: Vec, state_lanes: Vec>, @@ -332,26 +321,7 @@ fn resolve( }) .max_by_key(|(i, d)| (d.domain, std::cmp::Reverse(*i))) .map(|(i, _)| i); - // The fold major: the widest other ordinal whose composite universe with - // the fold key still fits the same domain-buffer budget. An empty - // universe is never paired (a GroupReduce sink must be non-empty). - let fold_major = fold_key.and_then(|k| { - let minor = u64::from(dims[k].domain); - dims.iter() - .enumerate() - .filter(|&(i, d)| { - let product = minor * u64::from(d.domain); - i != k - && matches!(d.provider, Provider::OrdinalLane { .. }) - && product > 0 - && product <= u64::from(policy.domain_buffer_budget) - }) - .max_by_key(|(i, d)| (d.domain, std::cmp::Reverse(*i))) - .map(|(i, _)| i) - }); - let partitions = (0..dims.len()) - .filter(|&i| Some(i) != fold_key && Some(i) != fold_major) - .collect(); + let partitions = (0..dims.len()).filter(|&i| Some(i) != fold_key).collect(); let mut states = vec![FoldState::Count]; for m in &key.measures { @@ -377,55 +347,30 @@ fn resolve( Ok(Resolved { dims, fold_key, - fold_major, partitions, states, state_lanes, }) } -/// The group address of the fold: the fold key alone, or the fold major and -/// the fold key fused into Quack's composite [`GroupAddr::Pair`]. -fn fold_addr(r: &Resolved) -> Option { - let k = r.fold_key?; - let lo = Col(fold_key_lane(&r.dims[k])); - Some(match r.fold_major { - None => GroupAddr::Local(lo), - Some(m) => GroupAddr::Pair { - hi: Col(fold_key_lane(&r.dims[m])), - lo, - stride: r.dims[k].domain, - }, - }) -} - /// The aggregate a fold state lowers to, keyed or scalar. -/// -/// A SUM keyed by a single lane is the coalescing `GroupSumI32`. Quack has no -/// pair-keyed SUM terminal, so a pair-keyed SUM is the NULL-preserving -/// `GroupReduce { SumI32 }` — whose sink must be normalized (see -/// [`needs_normalize`]) before its slots are this crate's fold states. -fn agg_for(state: &FoldState, lane: Option, key: Option) -> Agg { +fn agg_for(state: &FoldState, lane: Option, key: Option) -> Agg { let v = lane.map(Col); - match (key, state) { - (Some(key), FoldState::Count) => Agg::GroupReduce { - key, + match (key.map(Col), state) { + (Some(k), FoldState::Count) => Agg::GroupReduce { + key: GroupAddr::Local(k), agg: GroupAgg::Count, }, - (Some(GroupAddr::Local(k)), FoldState::Sum(_)) => Agg::GroupSumI32 { + (Some(k), FoldState::Sum(_)) => Agg::GroupSumI32 { key: k, val: v.expect("lane"), }, - (Some(key), FoldState::Sum(_)) => Agg::GroupReduce { - key, - agg: GroupAgg::SumI32(v.expect("lane")), - }, - (Some(key), FoldState::Min(_)) => Agg::GroupReduce { - key, + (Some(k), FoldState::Min(_)) => Agg::GroupReduce { + key: GroupAddr::Local(k), agg: GroupAgg::MinI32(v.expect("lane")), }, - (Some(key), FoldState::Max(_)) => Agg::GroupReduce { - key, + (Some(k), FoldState::Max(_)) => Agg::GroupReduce { + key: GroupAddr::Local(k), agg: GroupAgg::MaxI32(v.expect("lane")), }, (None, FoldState::Count) => Agg::Count, @@ -435,20 +380,6 @@ fn agg_for(state: &FoldState, lane: Option, key: Option) -> Agg } } -/// The grouped fold whose raw sink is NOT already a fold state of this -/// crate: the NULL-preserving SUM leaves a group no row reached at the -/// substrate's empty marker. Every other keyed fold seeds exactly -/// [`FoldState::identity`] (0 / `i64::MAX` / `i64::MIN`). -fn needs_normalize(agg: &Agg) -> Option { - match agg { - Agg::GroupReduce { - agg: g @ GroupAgg::SumI32(_), - .. - } => Some(*g), - _ => None, - } -} - fn scalar_of(state: &FoldState, v: Value) -> i64 { match v { Value::Count(n) => n as i64, @@ -577,7 +508,7 @@ impl ReportPlan { } else { base }; - let key_addr = fold_addr(&r); + let key_lane = r.fold_key.map(|i| fold_key_lane(&r.dims[i])); let first_pass = r .states .iter() @@ -585,7 +516,7 @@ impl ReportPlan { .map(|(s, &l)| { lower(&Query { filter: conj(&fbase, &first_members), - agg: agg_for(s, l, key_addr), + agg: agg_for(s, l, key_lane), }) }) .collect::, _>>()?; @@ -596,7 +527,6 @@ impl ReportPlan { extent, dims: r.dims, fold_key: r.fold_key, - fold_major: r.fold_major, partitions: r.partitions, states: r.states, accumulator, @@ -668,11 +598,8 @@ impl ReportPlan { lanes: &lanes, }; - let key_addr = fold_addr(&r); - // The fold's group universe: the fold key's domain, times the fold - // major's when the pair key is in use (bounded by the domain budget). - let minor_domain = r.fold_key.map_or(1, |i| r.dims[i].domain as usize); - let key_domain = minor_domain * r.fold_major.map_or(1, |i| r.dims[i].domain as usize); + let key_lane = r.fold_key.map(|i| fold_key_lane(&r.dims[i])); + let key_domain = r.fold_key.map_or(1, |i| r.dims[i].domain as usize); // One program run: lowers, sizes branch-private scratch, executes. let run = |filter: Filter, @@ -680,9 +607,10 @@ impl ReportPlan { out: Option<&mut [i64]>, stats: &mut ExecStats| -> Result { - let agg = agg_for(&r.states[s], r.state_lanes[s], key_addr); - let normalize = needs_normalize(&agg); - let prog = lower(&Query { filter, agg })?; + let prog = lower(&Query { + filter, + agg: agg_for(&r.states[s], r.state_lanes[s], key_lane), + })?; let mut scratch = Scratch::for_program(&prog, n)?; if prog.requires_scratch() { let b = scratch_words_for(tile_words_for(n), prog.scratch_slots as usize) @@ -692,32 +620,18 @@ impl ReportPlan { } stats.population_scans += 1; stats.tile_mask_ops += prog.ops.len() as u64; - match out { - Some(o) => { - let v = execute_extent( - &prog, - &planes, - &Foreign::NONE, - &mut scratch, - Out::I64(&mut *o), - extent.clone(), - )?; - if let Some(g) = normalize { - // Empty group -> 0, the SUM identity; the presence - // mask is not needed — COUNT already defines EMPTY. - normalize_group_sink(g.fold(), o); - } - Ok(v) - } - None => Ok(execute_extent( - &prog, - &planes, - &Foreign::NONE, - &mut scratch, - Out::None, - extent.clone(), - )?), - } + let out = match out { + Some(o) => Out::I64(o), + None => Out::None, + }; + Ok(execute_extent( + &prog, + &planes, + &Foreign::NONE, + &mut scratch, + out, + extent.clone(), + )?) }; let dims_meta: Vec = pp @@ -752,17 +666,12 @@ impl ReportPlan { budget: policy.pass_budget, }); } - // Storage order: partitions (pass order), then the fold major, - // then the fold key innermost, so each pass writes one - // contiguous run — `major × minor_domain + minor` is exactly - // the pair key's group index. + // Storage order: partitions (pass order), then the fold key + // innermost so each pass writes one contiguous run. let mut strides = vec![0usize; r.dims.len()]; let mut stride = 1usize; if let Some(k) = r.fold_key { strides[k] = 1; - if let Some(m) = r.fold_major { - strides[m] = minor_domain; - } stride = key_domain; } for &p in r.partitions.iter().rev() { @@ -888,14 +797,7 @@ impl ReportPlan { coords[p].push(m); } if let (Some(kd), Some(k)) = (r.fold_key, k) { - match r.fold_major { - None => coords[kd].push(k), - Some(md) => { - let minor = minor_domain as u32; - coords[md].push(k / minor); - coords[kd].push(k % minor); - } - } + coords[kd].push(k); } for (s, v) in vals.iter_mut().enumerate() { v.push(st(s)); diff --git a/crates/lance-graph-report/src/explain.rs b/crates/lance-graph-report/src/explain.rs index 6c1ed6c95..dcec09c39 100644 --- a/crates/lance-graph-report/src/explain.rs +++ b/crates/lance-graph-report/src/explain.rs @@ -57,8 +57,6 @@ impl fmt::Display for PhysicalPlan { }; let role = if self.fold_key == Some(i) { "fold key" - } else if self.fold_major == Some(i) { - "fold major (pair key, same pass)" } else { "partition" }; diff --git a/crates/lance-graph-report/tests/agnostic.rs b/crates/lance-graph-report/tests/agnostic.rs index 89e7a8b27..b5aa30d1c 100644 --- a/crates/lance-graph-report/tests/agnostic.rs +++ b/crates/lance-graph-report/tests/agnostic.rs @@ -403,10 +403,8 @@ fn t8_json_and_csv_differ_only_at_the_terminal() { fn t9_native_plan_lowers_to_exactly_the_programs_quack_would_write() { let fx = synthetic(2_000, &[6, 9], 19); let batch = fx.batch(); - // "WHERE V >= 0 GROUP BY A, B SUM(V)" — B (9) is the fold key (wider), - // A (6) the fold major: ONE pass over Quack's composite key - // `A × 9 + B`. Quack has no pair-keyed SUM terminal, so the SUM is the - // NULL-preserving `GroupReduce { SumI32 }`. + // "WHERE V >= 0 GROUP BY A, B SUM(V)" — A (6) is a partition, B (9) + // the fold key (wider). Pass 0 is A = 0. let plan = ReportPlan::over(src()) .filter(Selection::cmp(V, CmpOp::Ge, Scalar::Int(0))) .pivot(&[fa()], &[fb()]) @@ -422,21 +420,15 @@ fn t9_native_plan_lowers_to_exactly_the_programs_quack_would_write() { .unwrap(); // Lanes: F0 → 0, F1 → 1, F100 → 2 (fixture order). let where_ = Filter::Cmp(Col(2), Cmp::GeI32(0)); - let pair = lance_graph_quack::GroupAddr::Pair { - hi: Col(0), - lo: Col(1), - stride: 9, - }; - assert_eq!(pp.passes, 1, "two ordinals fold in one pass"); - let pass0 = where_.clone(); + let pass0 = Filter::And(vec![where_, Filter::Cmp(Col(0), Cmp::EqU32(0))]); let quack: Vec = [ Agg::GroupReduce { - key: pair, + key: lance_graph_quack::GroupAddr::Local(Col(1)), agg: lance_graph_quack::GroupAgg::Count, }, - Agg::GroupReduce { - key: pair, - agg: lance_graph_quack::GroupAgg::SumI32(Col(2)), + Agg::GroupSumI32 { + key: Col(1), + val: Col(2), }, ] .into_iter() @@ -470,112 +462,15 @@ fn t9_native_plan_lowers_to_exactly_the_programs_quack_would_write() { .first_pass, quack ); - - // Twin: a budget too small for the 54-group pair universe (but wide - // enough for B alone) keeps the per-member shape — A is a partition, B - // the single-lane key, and pass 0 is A = 0. Same native == Quack law. - let narrow = PlannerPolicy { - reuse_mask_min_programs: u64::MAX, - domain_buffer_budget: 53, - ..PlannerPolicy::default() - }; - let pp = plan.explain(&batch, &narrow).unwrap(); - assert_eq!(pp.fold_major, None); - assert_eq!(pp.passes, 6); - let pass0 = Filter::And(vec![where_, Filter::Cmp(Col(0), Cmp::EqU32(0))]); - let per_member: Vec = [ - Agg::GroupReduce { - key: lance_graph_quack::GroupAddr::Local(Col(1)), - agg: lance_graph_quack::GroupAgg::Count, - }, - Agg::GroupSumI32 { - key: Col(1), - val: Col(2), - }, - ] - .into_iter() - .map(|agg| { - lower(&Query { - filter: pass0.clone(), - agg, - }) - .unwrap() - }) - .collect(); - assert_eq!(pp.first_pass, per_member); -} - -// ── PAIR KEY ≡ PER-MEMBER PASSES ──────────────────────────────────────── -// Two ordinal coordinates fold in ONE pass through Quack's composite key. -// Every cell, every total, and every hidden-axis merge must equal the -// per-member plan's — including empty cells, where the pair-keyed SUM's raw -// sink holds the substrate's NULL marker until it is normalized to 0. -#[test] -fn pair_key_folds_two_ordinals_in_one_pass_and_equals_per_member_passes() { - // 300 rows over 20 × 30 = 600 cells: most cells are EMPTY (anti-vacuity - // for the SUM normalization), with negative values present. - let fx = synthetic(300, &[20, 30], 22); - let batch = fx.batch(); - let plan = with_four(ReportPlan::over(src()).pivot(&[fa()], &[fb()]), V); - let pair_policy = PlannerPolicy { - domain_buffer_budget: 600, - ..PlannerPolicy::default() - }; - let split_policy = PlannerPolicy { - domain_buffer_budget: 599, - ..PlannerPolicy::default() - }; - // The budget knob decides the shape and nothing else: 600 pairs, 599 - // does not (can-fire / can-stay-silent at the exact threshold). - let pp = plan.explain(&batch, &pair_policy).unwrap(); - assert_eq!(pp.dims[pp.fold_key.unwrap()].coord, fb()); - assert_eq!(pp.dims[pp.fold_major.unwrap()].coord, fa()); - assert!(pp.partitions.is_empty()); - assert_eq!(pp.passes, 1); - assert!(pp.to_string().contains("fold major")); - let sp = plan.explain(&batch, &split_policy).unwrap(); - assert_eq!(sp.fold_major, None); - assert_eq!(sp.passes, 20); - - let (pair, ps) = plan.execute(&batch, &pair_policy).unwrap(); - let (split, ss) = plan.execute(&batch, &split_policy).unwrap(); - assert_eq!(ps.population_scans, 4, "one pass × four fold states"); - assert_eq!(ss.population_scans, 20 * 4); - assert_matches_oracle(&fx, &plan, &pair, V); - - let empty = fx.n < 600 && fx.oracle(&plan, Some(V)).len() < 600; - assert!(empty, "the fixture must leave cells empty"); - for m in pair.measures() { - for a in 0..20 { - for b in 0..30 { - assert_eq!( - pair.value(m, &[], &[a], &[b]), - split.value(m, &[], &[a], &[b]), - "{m:?} at ({a},{b})" - ); - } - } - // Totals merge stored slots, so an un-normalized empty SUM slot would - // corrupt them even though each empty cell reads NULL. - assert_eq!(pair.grand_total(m), split.grand_total(m), "{m:?} total"); - let pr = pair.with_roles(&[fa()], &[], &[]).unwrap(); - let sr = split.with_roles(&[fa()], &[], &[]).unwrap(); - for a in 0..20 { - assert_eq!(pr.value(m, &[], &[a], &[]), sr.value(m, &[], &[a], &[])); - } - } } // ── knob inertness: pass and domain budgets can fire, and stay silent ─── #[test] fn pass_budget_refuses_instead_of_running_slowly_and_is_silent_when_met() { - // Three ordinals: C (40) keys the fold, B (30) is the fold major, and - // A (20) is the one partition left — 20 passes. - let fx = synthetic(1_000, &[20, 30, 40], 20); + let fx = synthetic(1_000, &[20, 30], 20); let batch = fx.batch(); let plan = ReportPlan::over(src()) .pivot(&[fa()], &[fb()]) - .axis(fc(), AxisRole::Page) .measure(Measure::count()); let tight = PlannerPolicy { pass_budget: 19, From 100156b7ea53f76e0b5ca01c40253189b7773256 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 10:12:54 +0000 Subject: [PATCH 4/6] report: state the destination-resolving fold gap semantically; retire the greedy-Pair OPEN item Wording only. exec.rs gap doc: bind/resolve -> destination -> fold, with the law that a join/pivot used only to find an aggregate destination compiles to destination resolution + fold, never to an intermediate relation and never automatically to a dense product; the normal path binds destinations beforehand (ordinal, CAM/codebook, Via / composed reference) and only an unbindable universe uses a query-local compact resolver. No physical representation is chosen. Board entry: heading names the superseded Pair experiment; the greedy-Pair optimization leaves OPEN and is kept only as a historical observation under the superseded cut; the missing-primitive OPEN line no longer prescribes keys-SoA / observed-destination storage. Decisions 2-5 unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DCEP2fZdHYdMCtcEVcTpS2 --- ...6-10-05-report-pair-key-and-stack-recon.md | 14 +++++----- crates/lance-graph-report/src/exec.rs | 26 ++++++++++++------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md index 6d05dc37e..73e3b6dbc 100644 --- a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md +++ b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md @@ -1,19 +1,19 @@ -# Report folds two ordinals in one pass; stack-convergence recon (2026-10-05) +# Report Pair experiment superseded; destination-resolving fold gap and stack recon (2026-10-05) **Status:** ⊘ SUPERSEDED IN PART (2026-10-05, same PR, before merge) — the Pair execution path described under "The cut" was REVERTED; this PR now ships only the corrected gap doc in `exec.rs` and this record. See "Correction" below. Recon findings and decisions 2–5 stand. ~~MEASURED — the code change and its tests are in this PR.~~ The recon findings are VERIFIED-IN-CODE against z8run `3a8a758`, lance-graph `97a3610d`, OGAR `e5de84e`, rs-graph-llm `e824977` and rig `d165343`. It ratifies the five decisions below and nothing more. ## Correction (2026-10-05) — why the cut below was superseded -`GroupAddr::Pair` addresses `hi · stride + lo`: the **dense product** of two dimension domains. Lowering every two-ordinal report to it made the Cartesian product the execution geometry because the primitive existed, not because any consumer demanded a dense cube. The intended fold model is the PowerShell-Hashtable / Excel-Pivot one: a row resolves its accumulator **destination** directly, and accumulator state scales with **observed/demanded destinations**. For IAM-sized dimensions (64k × 64k) a Pair sink is 4·10⁹ slots; a destination-resolving fold touches only the observed pairs. +`GroupAddr::Pair` addresses `hi · stride + lo`: the **dense product** of two dimension domains. Lowering every two-ordinal report to it made the Cartesian product the execution geometry because the primitive existed, not because any consumer demanded a dense cube. The intended fold model is the PowerShell-Hashtable / Excel-Pivot one: a row resolves its accumulator **destination** directly, and accumulator state scales with the **demanded / resolved destination universe**, not with an accidental Cartesian product. For IAM-sized dimensions (64k × 64k) a Pair sink is 4·10⁹ slots whatever the data demands. - **What stays true:** Pair was useful *evidence* that Report re-implemented what Quack can fold, and the old doc's "substrate gap" claim was stale. - **What was wrong:** Pair is not the general Pivot/Join execution model. A destination-resolving keyed fold is. - **What the source shows (VERIFIED-IN-CODE):** - - no compact tuple → slot resolver exists on any fold path. ndarray/mask-risc/Quack have ONE keyed-reduction walker with three addresses — `Lane`, `Via` (`table[fk[i]]`), `Pair` — all writing a dense K-slot sink (ndarray `simd_masking_ops.rs` `masked_group_*`, mask-risc `ir.rs` `GroupKey`, Quack `GroupAddr`, lgj `plan_lower.rs` Local/Via). + - no fold path resolves a row to a destination outside a dense K-slot universe. ndarray/mask-risc/Quack have ONE keyed-reduction walker with three addresses — `Lane`, `Via` (`table[fk[i]]`), `Pair` — all writing a dense K-slot sink (ndarray `simd_masking_ops.rs` `masked_group_*`, mask-risc `ir.rs` `GroupKey`, Quack `GroupAddr`, lgj `plan_lower.rs` Local/Via). - The only compact structure is Report's `Layout::Sparse` RESULT index, reached by rescanning once per observed partition member. - **Reverted:** `fold_major`, `fold_addr`, the Pair SUM normalization, the product budget, the Pair strides and decode, the Pair-pinning tests. Report is back to main's planner. - **Kept:** `GroupAddr::Pair` in Quack/mask-risc/ndarray (correct for an explicitly dense mixed-radix problem; untouched here). -- **Shipped:** `exec.rs`'s gap doc now names the real gap: a destination-resolving keyed fold, substrate-first in ndarray's keyed-reduction family. +- **Shipped:** `exec.rs`'s gap doc now names the real gap: a destination-resolving keyed fold, stated semantically (bind/resolve → destination → fold), substrate-first. ## The cut (⊘ SUPERSEDED — reverted before merge; kept as the record of what was tried) @@ -36,6 +36,8 @@ The physical planner now picks a **fold major**: the widest remaining ordinal wh - `t9` now pins the pair programs, plus a per-member twin under a tight budget. - The pass-budget test moved to three ordinals so it still fires at 20. +**Historical observation from review (not active work):** the Pair choice was greedy, not pass-minimal — with domains 100 × 60 × 60 and a 3600 budget it ran 3600 partition passes where folding 60 × 60 and partitioning over 100 would need 100. This is NOT an open optimization for Report: optimizing which dense product to fold is the superseded model. `GroupAddr::Pair` remains valid only where a consumer explicitly demands a dense mixed-radix destination universe; it is not the future implementation of sparse Pivot/Join/grouping. + **Other runs:** - z8run-lance (`pivot_flow`, handle size under 128 bytes, `z8run_plan_is_the_native_plan`, zero-refold pivot): 10/10. - report-ogar: 4/4. @@ -65,9 +67,7 @@ The physical planner now picks a **fold major**: the widest remaining ordinal wh ## OPEN -- ⊘ SUPERSEDED with the Pair path — **Pair choice is greedy, not pass-minimal** (found in review). Today the planner takes the widest ordinal as the fold key, then the widest ordinal that fits with it. With domains 100 × 60 × 60 and a 3600 budget, 100 × 60 does not fit, so there is no pair, and 60 × 60 = 3600 partition passes run. Folding 60 × 60 and partitioning over 100 would need only 100 passes. **REVISIT WHEN** the planner is next touched: choose `(fold key, fold major)` jointly to minimise passes. That is "the best exact fold universe" — still Quack's Pair, no new abstraction. Kept out of #1331 on purpose, so the fold key stays exactly as it was. - -- **The missing primitive:** a destination-resolving keyed fold (resolved key tuple → compact slot in the same pass; keys SoA + value columns ∝ observed destinations). Layer order: ndarray keyed-reduction family → mask-risc `GroupKey` + an `Out` carrying keys and values → Quack `GroupAddr`; Report and lgj consume it. Contract first — see the fold-contract recon. +- **The missing primitive:** a destination-resolving keyed fold, stated semantically: BIND / RESOLVE (semantic coordinates, functional references) → a destination → ACCUMULATE directly into it. **Law:** a join/pivot used only to determine an aggregate destination must compile to destination resolution + fold — not to an intermediate relation, and not automatically to a dense product; accumulator state scales with the demanded / resolved destination universe. The normal fast path binds destinations beforehand (resident ordinal, CAM / codebook binding, `Via` or composed functional reference, or another pre-bound destination representation); only a destination universe that genuinely cannot be bound beforehand may use a query-local compact resolver. No physical representation is chosen here. Substrate-first; contract first — see the fold-contract recon. - **`CoordSpec` has no functional-reference coordinate:** Report cannot group by `user → department` although `GroupAddr::Via` / `Filter::EqU32Via` already exist. A separable small step. - Owner of the execution membrane (Program + World → ResultRef). It is needed by both z8run and graph-flow. - Whether the merge-law re-roll of totals is a Quack fold or presentation. diff --git a/crates/lance-graph-report/src/exec.rs b/crates/lance-graph-report/src/exec.rs index 1f36cb2d6..6521c0a16 100644 --- a/crates/lance-graph-report/src/exec.rs +++ b/crates/lance-graph-report/src/exec.rs @@ -42,16 +42,22 @@ //! The honest limit: a partition costs one pass per member tuple, so a plan //! whose partition side is high-cardinality AND densely observed exceeds the //! pass budget and is REFUSED with [`ReportError::PassBudget`] rather than -//! run slowly or allocated densely. The primitive that would lift it is a -//! named substrate gap, not something to hand-roll here: a -//! **destination-resolving keyed fold** — a keyed-reduction address in -//! `ndarray::simd` / mask-risc that maps a row's resolved key tuple to a -//! COMPACT accumulator slot in the same pass, so accumulator state scales with -//! the observed destinations, never with the product of the dimension domains. -//! (A composite mixed-radix key — Quack's `GroupAddr::Pair`, `hi · stride + -//! lo` — exists, but it addresses the dense product and is therefore NOT that -//! primitive; this planner does not lower to it. It belongs only to a -//! problem that explicitly demands the dense cube.) +//! run slowly or allocated densely. What would lift it is a named substrate +//! gap, not something to hand-roll here: a **destination-resolving keyed +//! fold**. A row's semantic coordinates and functional references are bound +//! and resolved to its aggregate DESTINATION, and the row folds directly into +//! it. A join or pivot used only to determine an aggregate destination must +//! compile to destination resolution + fold — never to an intermediate +//! relation, and never automatically to a dense product — so accumulator +//! state scales with the demanded / resolved destination universe, not with +//! an accidental Cartesian product of the dimension domains. The normal path +//! binds destinations beforehand (a resident ordinal, a CAM / codebook +//! binding, a `Via` or composed functional reference); a query-local compact +//! resolver is only for a destination universe that genuinely cannot be bound +//! beforehand. (Quack's `GroupAddr::Pair`, `hi · stride + lo`, addresses the +//! dense product and is therefore NOT this primitive; this planner does not +//! lower to it. It belongs only to a problem that explicitly demands a dense +//! mixed-radix destination universe.) use std::collections::HashMap; use std::sync::Arc; From eb716753795efee0cc8fb9f3e6d8bb9446961089 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 10:21:27 +0000 Subject: [PATCH 5/6] report: locate the missing half in BIND/RESOLVE; correct the sparse rescan cost Wording only (codex review on #1331): - the ACCUMULATE half exists (GroupAddr::Local / Via fold into a K-slot sink; K can be the compact destination count). The missing half is binding/resolving ONE destination from several coordinates in Report's CoordSpec / binder -- not an ndarray fold. - the sparse path costs one discovery scan per ordinal partition plus one run per fold state per tuple of the product of observed members, not one rescan per observed member. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DCEP2fZdHYdMCtcEVcTpS2 --- .../2026-10-05-report-pair-key-and-stack-recon.md | 8 ++++---- crates/lance-graph-report/src/exec.rs | 15 ++++++++++----- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md index 73e3b6dbc..48a24eaf2 100644 --- a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md +++ b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md @@ -9,11 +9,11 @@ - **What stays true:** Pair was useful *evidence* that Report re-implemented what Quack can fold, and the old doc's "substrate gap" claim was stale. - **What was wrong:** Pair is not the general Pivot/Join execution model. A destination-resolving keyed fold is. - **What the source shows (VERIFIED-IN-CODE):** - - no fold path resolves a row to a destination outside a dense K-slot universe. ndarray/mask-risc/Quack have ONE keyed-reduction walker with three addresses — `Lane`, `Via` (`table[fk[i]]`), `Pair` — all writing a dense K-slot sink (ndarray `simd_masking_ops.rs` `masked_group_*`, mask-risc `ir.rs` `GroupKey`, Quack `GroupAddr`, lgj `plan_lower.rs` Local/Via). - - The only compact structure is Report's `Layout::Sparse` RESULT index, reached by rescanning once per observed partition member. + - ndarray/mask-risc/Quack have ONE keyed-reduction walker with three addresses — `Lane`, `Via` (`table[fk[i]]`), `Pair` — all writing a caller-sized K-slot sink (ndarray `simd_masking_ops.rs` `masked_group_*`, mask-risc `ir.rs` `GroupKey`, Quack `GroupAddr`, lgj `plan_lower.rs` Local/Via). **The ACCUMULATE half exists:** given a pre-bound compact destination ordinal (a resident lane, or `Via`), K can be the destination count. **The missing half is BIND / RESOLVE:** Report's `CoordSpec` / binder cannot derive or represent one destination from several coordinates, nor intern a tuple during a query (codex review, #1331). + - The only compact structure is Report's `Layout::Sparse` RESULT index. It is reached by one discovery scan per ordinal partition dimension, then one run per fold state for EVERY tuple in the product of the per-dimension observed members (`exec.rs` sparse arm, `for_each_tuple` × states) — e.g. two partitions with 100 observed members each and four states cost 2 discovery scans + 40,000 population scans, not ~200 (codex review, #1331). - **Reverted:** `fold_major`, `fold_addr`, the Pair SUM normalization, the product budget, the Pair strides and decode, the Pair-pinning tests. Report is back to main's planner. - **Kept:** `GroupAddr::Pair` in Quack/mask-risc/ndarray (correct for an explicitly dense mixed-radix problem; untouched here). -- **Shipped:** `exec.rs`'s gap doc now names the real gap: a destination-resolving keyed fold, stated semantically (bind/resolve → destination → fold), substrate-first. +- **Shipped:** `exec.rs`'s gap doc now names the real gap: a destination-resolving keyed fold, stated semantically (bind/resolve → destination → fold), with the missing half located in BIND / RESOLVE, not in the fold. ## The cut (⊘ SUPERSEDED — reverted before merge; kept as the record of what was tried) @@ -67,7 +67,7 @@ The physical planner now picks a **fold major**: the widest remaining ordinal wh ## OPEN -- **The missing primitive:** a destination-resolving keyed fold, stated semantically: BIND / RESOLVE (semantic coordinates, functional references) → a destination → ACCUMULATE directly into it. **Law:** a join/pivot used only to determine an aggregate destination must compile to destination resolution + fold — not to an intermediate relation, and not automatically to a dense product; accumulator state scales with the demanded / resolved destination universe. The normal fast path binds destinations beforehand (resident ordinal, CAM / codebook binding, `Via` or composed functional reference, or another pre-bound destination representation); only a destination universe that genuinely cannot be bound beforehand may use a query-local compact resolver. No physical representation is chosen here. Substrate-first; contract first — see the fold-contract recon. +- **The missing primitive:** a destination-resolving keyed fold, stated semantically: BIND / RESOLVE (semantic coordinates, functional references) → a destination → ACCUMULATE directly into it. **Law:** a join/pivot used only to determine an aggregate destination must compile to destination resolution + fold — not to an intermediate relation, and not automatically to a dense product; accumulator state scales with the demanded / resolved destination universe. The normal fast path binds destinations beforehand (resident ordinal, CAM / codebook binding, `Via` or composed functional reference, or another pre-bound destination representation); only a destination universe that genuinely cannot be bound beforehand may use a query-local compact resolver. No physical representation is chosen here. The ACCUMULATE half exists (`Local` / `Via` into a K-slot sink, K = destination count); the open work is BIND / RESOLVE of a multi-coordinate destination, starting at Report's `CoordSpec` / binder. Only the unbindable-universe fallback may need a substrate resolver. Contract first — see the fold-contract recon. - **`CoordSpec` has no functional-reference coordinate:** Report cannot group by `user → department` although `GroupAddr::Via` / `Filter::EqU32Via` already exist. A separable small step. - Owner of the execution membrane (Program + World → ResultRef). It is needed by both z8run and graph-flow. - Whether the merge-law re-roll of totals is a Quack fold or presentation. diff --git a/crates/lance-graph-report/src/exec.rs b/crates/lance-graph-report/src/exec.rs index 6521c0a16..01edbd9e2 100644 --- a/crates/lance-graph-report/src/exec.rs +++ b/crates/lance-graph-report/src/exec.rs @@ -42,11 +42,16 @@ //! The honest limit: a partition costs one pass per member tuple, so a plan //! whose partition side is high-cardinality AND densely observed exceeds the //! pass budget and is REFUSED with [`ReportError::PassBudget`] rather than -//! run slowly or allocated densely. What would lift it is a named substrate -//! gap, not something to hand-roll here: a **destination-resolving keyed -//! fold**. A row's semantic coordinates and functional references are bound -//! and resolved to its aggregate DESTINATION, and the row folds directly into -//! it. A join or pivot used only to determine an aggregate destination must +//! run slowly or allocated densely. What would lift it is a named gap, not +//! something to hand-roll here: a **destination-resolving keyed fold**. A +//! row's semantic coordinates and functional references are bound and +//! resolved to its aggregate DESTINATION, and the row folds directly into it. +//! The ACCUMULATE half already exists: `GroupAddr::Local` / `GroupAddr::Via` +//! fold into a caller-sized K-slot sink, and K can be the compact destination +//! count. The missing half is BIND / RESOLVE — this planner's `CoordSpec` / +//! binder cannot derive or represent ONE destination from several coordinates +//! (nor intern a tuple during a query), so it partitions instead. A join or +//! pivot used only to determine an aggregate destination must //! compile to destination resolution + fold — never to an intermediate //! relation, and never automatically to a dense product — so accumulator //! state scales with the demanded / resolved destination universe, not with From 9e26e3765d04b060b9c1150a7c6f78732ed503c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 10:25:15 +0000 Subject: [PATCH 6/6] report: name the gap as destination binding, not a fold; exact sparse cost Wording only (codex review on #1331, operator-confirmed): - the missing capability is destination binding / coordinate resolution for Report. Once a compact destination ordinal or functional reference is bound, the existing GroupAddr::Local / Via keyed fold is the execution primitive. CoordSpec (Field, Bucket, MaskSet) cannot express a functional reference, a composed route, or several coordinates bound to one compact destination ordinal. A query-local interner is only a fallback. - Layout::Sparse costs one discovery scan per ordinal partition dimension, then T x FoldStates population folds, where T is the product of the partition radices (discovered members for ordinals, full radix for Bucket / MaskSet), plus a ReusedMask selection scan when chosen. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DCEP2fZdHYdMCtcEVcTpS2 --- ...6-10-05-report-pair-key-and-stack-recon.md | 16 +++---- crates/lance-graph-report/src/exec.rs | 46 ++++++++++--------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md index 48a24eaf2..f69f3f113 100644 --- a/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md +++ b/.claude/board/entries/2026-10-05-report-pair-key-and-stack-recon.md @@ -1,4 +1,4 @@ -# Report Pair experiment superseded; destination-resolving fold gap and stack recon (2026-10-05) +# Report Pair experiment superseded; destination-binding gap and stack recon (2026-10-05) **Status:** ⊘ SUPERSEDED IN PART (2026-10-05, same PR, before merge) — the Pair execution path described under "The cut" was REVERTED; this PR now ships only the corrected gap doc in `exec.rs` and this record. See "Correction" below. Recon findings and decisions 2–5 stand. ~~MEASURED — the code change and its tests are in this PR.~~ The recon findings are VERIFIED-IN-CODE against z8run `3a8a758`, lance-graph `97a3610d`, OGAR `e5de84e`, rs-graph-llm `e824977` and rig `d165343`. It ratifies the five decisions below and nothing more. @@ -7,17 +7,17 @@ `GroupAddr::Pair` addresses `hi · stride + lo`: the **dense product** of two dimension domains. Lowering every two-ordinal report to it made the Cartesian product the execution geometry because the primitive existed, not because any consumer demanded a dense cube. The intended fold model is the PowerShell-Hashtable / Excel-Pivot one: a row resolves its accumulator **destination** directly, and accumulator state scales with the **demanded / resolved destination universe**, not with an accidental Cartesian product. For IAM-sized dimensions (64k × 64k) a Pair sink is 4·10⁹ slots whatever the data demands. - **What stays true:** Pair was useful *evidence* that Report re-implemented what Quack can fold, and the old doc's "substrate gap" claim was stale. -- **What was wrong:** Pair is not the general Pivot/Join execution model. A destination-resolving keyed fold is. +- **What was wrong:** Pair is not the general Pivot/Join execution model. Destination binding/resolution + the EXISTING keyed fold is. - **What the source shows (VERIFIED-IN-CODE):** - - ndarray/mask-risc/Quack have ONE keyed-reduction walker with three addresses — `Lane`, `Via` (`table[fk[i]]`), `Pair` — all writing a caller-sized K-slot sink (ndarray `simd_masking_ops.rs` `masked_group_*`, mask-risc `ir.rs` `GroupKey`, Quack `GroupAddr`, lgj `plan_lower.rs` Local/Via). **The ACCUMULATE half exists:** given a pre-bound compact destination ordinal (a resident lane, or `Via`), K can be the destination count. **The missing half is BIND / RESOLVE:** Report's `CoordSpec` / binder cannot derive or represent one destination from several coordinates, nor intern a tuple during a query (codex review, #1331). - - The only compact structure is Report's `Layout::Sparse` RESULT index. It is reached by one discovery scan per ordinal partition dimension, then one run per fold state for EVERY tuple in the product of the per-dimension observed members (`exec.rs` sparse arm, `for_each_tuple` × states) — e.g. two partitions with 100 observed members each and four states cost 2 discovery scans + 40,000 population scans, not ~200 (codex review, #1331). + - ndarray/mask-risc/Quack have ONE keyed-reduction walker with three addresses — `Lane`, `Via` (`table[fk[i]]`), `Pair` — all writing a caller-sized K-slot sink (ndarray `simd_masking_ops.rs` `masked_group_*`, mask-risc `ir.rs` `GroupKey`, Quack `GroupAddr`, lgj `plan_lower.rs` Local/Via). **That keyed fold IS the execution primitive once a destination is bound:** a compact resident destination ordinal folds through `GroupAddr::Local`; `fk[row]` resolves to a foreign destination ordinal through `GroupAddr::Via`; K is the destination count. **The missing capability is destination binding / coordinate resolution for Report:** `CoordSpec` has only `Field`, `Bucket` and `MaskSet`, so it cannot express a functional-reference coordinate (`user → department`), a composed functional route, or several semantic coordinates bound to ONE compact destination ordinal (codex review, #1331). + - The only compact structure is Report's `Layout::Sparse` RESULT index, and reaching it costs (`exec.rs` sparse arm): one discovery population scan for each ORDINAL partition dimension (a domain-sized `GroupReduce Count`); then, with T = the product of the partition radices — DISCOVERED members for an ordinal dimension, the FULL configured radix for a `Bucket` / `MaskSet` dimension — one population fold for every tuple in T for every `FoldState` (`for_each_tuple` × states). Main fold cost: **T × FoldStates** population scans, plus the discovery scans, plus one selection-materialization scan when the `ReusedMask` carrier is chosen. E.g. 100 × 100 observed members × 4 FoldStates = 40,000 fold scans after discovery — not one scan per member (codex review, #1331). This is the partition-rescan architecture the destination-binding gap exists to eliminate. - **Reverted:** `fold_major`, `fold_addr`, the Pair SUM normalization, the product budget, the Pair strides and decode, the Pair-pinning tests. Report is back to main's planner. - **Kept:** `GroupAddr::Pair` in Quack/mask-risc/ndarray (correct for an explicitly dense mixed-radix problem; untouched here). -- **Shipped:** `exec.rs`'s gap doc now names the real gap: a destination-resolving keyed fold, stated semantically (bind/resolve → destination → fold), with the missing half located in BIND / RESOLVE, not in the fold. +- **Shipped:** `exec.rs`'s gap doc now names the missing capability as destination binding / coordinate resolution for Report, with the existing `Local` / `Via` keyed fold acknowledged as the execution primitive. ## The cut (⊘ SUPERSEDED — reverted before merge; kept as the record of what was tried) -`lance-graph-report` planned two-dimensional reports as **one population pass per partition member**. Its module doc called the composite-key group fold "a named substrate gap". That gap had already closed: Quack has `GroupAddr::Pair { hi, lo, stride }`. +`lance-graph-report` planned two-dimensional reports as **one population fold per partition tuple per FoldState** (see the corrected cost under "Correction"). Its module doc called the composite-key group fold "a named substrate gap". That gap had already closed: Quack has `GroupAddr::Pair { hi, lo, stride }`. The physical planner now picks a **fold major**: the widest remaining ordinal whose product with the fold key's domain fits `domain_buffer_budget`. It folds both coordinates in ONE pass, through the pair key (`exec.rs` `fold_addr`). @@ -67,8 +67,8 @@ The physical planner now picks a **fold major**: the widest remaining ordinal wh ## OPEN -- **The missing primitive:** a destination-resolving keyed fold, stated semantically: BIND / RESOLVE (semantic coordinates, functional references) → a destination → ACCUMULATE directly into it. **Law:** a join/pivot used only to determine an aggregate destination must compile to destination resolution + fold — not to an intermediate relation, and not automatically to a dense product; accumulator state scales with the demanded / resolved destination universe. The normal fast path binds destinations beforehand (resident ordinal, CAM / codebook binding, `Via` or composed functional reference, or another pre-bound destination representation); only a destination universe that genuinely cannot be bound beforehand may use a query-local compact resolver. No physical representation is chosen here. The ACCUMULATE half exists (`Local` / `Via` into a K-slot sink, K = destination count); the open work is BIND / RESOLVE of a multi-coordinate destination, starting at Report's `CoordSpec` / binder. Only the unbindable-universe fallback may need a substrate resolver. Contract first — see the fold-contract recon. -- **`CoordSpec` has no functional-reference coordinate:** Report cannot group by `user → department` although `GroupAddr::Via` / `Filter::EqU32Via` already exist. A separable small step. +- **The missing capability — destination binding / coordinate resolution for Report** (not a fold primitive): semantic coordinate(s) → BIND / RESOLVE DESTINATION → compact destination ordinal or functional route → the EXISTING `Local` / `Via` keyed fold. Report's `CoordSpec` (`Field`, `Bucket`, `MaskSet`) cannot yet express a functional-reference coordinate (`user → department`, although `GroupAddr::Via` / `Filter::EqU32Via` exist), a composed functional route, or several semantic coordinates bound to ONE compact destination ordinal. **Law:** a join/pivot used only to determine an aggregate destination must compile to destination binding/resolution + fold — not to an intermediate relation, and not automatically to a dense product; accumulator state scales with the demanded / resolved destination universe. The normal path binds beforehand (resident ordinal, CAM / codebook binding, `Via` or composed functional reference). A query-local compact tuple interner is only a possible fallback for a genuinely unbound destination universe, not the canonical path. No representation, type or variant is chosen here. Contract first — see the fold-contract recon. +- ~~**`CoordSpec` has no functional-reference coordinate**~~ — folded into the missing-capability item above (it is its first, separable case). - Owner of the execution membrane (Program + World → ResultRef). It is needed by both z8run and graph-flow. - Whether the merge-law re-roll of totals is a Quack fold or presentation. - Whether `with_roles` may hide dimensions, since it merges over them: a GROUP BY done in the view. diff --git a/crates/lance-graph-report/src/exec.rs b/crates/lance-graph-report/src/exec.rs index 01edbd9e2..b7ff9a6a5 100644 --- a/crates/lance-graph-report/src/exec.rs +++ b/crates/lance-graph-report/src/exec.rs @@ -42,27 +42,31 @@ //! The honest limit: a partition costs one pass per member tuple, so a plan //! whose partition side is high-cardinality AND densely observed exceeds the //! pass budget and is REFUSED with [`ReportError::PassBudget`] rather than -//! run slowly or allocated densely. What would lift it is a named gap, not -//! something to hand-roll here: a **destination-resolving keyed fold**. A -//! row's semantic coordinates and functional references are bound and -//! resolved to its aggregate DESTINATION, and the row folds directly into it. -//! The ACCUMULATE half already exists: `GroupAddr::Local` / `GroupAddr::Via` -//! fold into a caller-sized K-slot sink, and K can be the compact destination -//! count. The missing half is BIND / RESOLVE — this planner's `CoordSpec` / -//! binder cannot derive or represent ONE destination from several coordinates -//! (nor intern a tuple during a query), so it partitions instead. A join or -//! pivot used only to determine an aggregate destination must -//! compile to destination resolution + fold — never to an intermediate -//! relation, and never automatically to a dense product — so accumulator -//! state scales with the demanded / resolved destination universe, not with -//! an accidental Cartesian product of the dimension domains. The normal path -//! binds destinations beforehand (a resident ordinal, a CAM / codebook -//! binding, a `Via` or composed functional reference); a query-local compact -//! resolver is only for a destination universe that genuinely cannot be bound -//! beforehand. (Quack's `GroupAddr::Pair`, `hi · stride + lo`, addresses the -//! dense product and is therefore NOT this primitive; this planner does not -//! lower to it. It belongs only to a problem that explicitly demands a dense -//! mixed-radix destination universe.) +//! run slowly or allocated densely. The missing capability is **destination +//! binding / coordinate resolution for Report** — not a fold primitive. Once +//! a compact destination ordinal or a functional reference is bound, the +//! existing keyed fold IS the execution primitive: a resident destination +//! ordinal folds through `GroupAddr::Local`, and `fk[row]` resolves to a +//! foreign destination ordinal through `GroupAddr::Via`, each into a +//! caller-sized K-slot sink where K is the destination count. What Report +//! lacks is the binding step: `CoordSpec` has only `Field`, `Bucket` and +//! `MaskSet`, so it cannot express a functional-reference coordinate +//! (`user → department`), a composed functional route, or several semantic +//! coordinates bound to ONE compact destination ordinal — and so it +//! partitions instead. +//! +//! The law: a join or pivot used only to determine an aggregate destination +//! must compile to destination binding / resolution + fold — never to an +//! intermediate relation, and never automatically to a dense product — so +//! accumulator state scales with the demanded / resolved destination universe, +//! not with an accidental Cartesian product of the dimension domains. The +//! normal path binds destinations beforehand (a resident ordinal, a CAM / +//! codebook binding, a `Via` or composed functional reference). A query-local +//! compact interner is only a possible fallback for a destination universe +//! that genuinely cannot be bound beforehand, not the canonical path. (Quack's +//! `GroupAddr::Pair`, `hi · stride + lo`, addresses the dense product; it is +//! valid only for a consumer that explicitly demands a dense mixed-radix +//! destination universe, and this planner does not lower to it.) use std::collections::HashMap; use std::sync::Arc;