lgj-abi minor 12: the grouped sum that never builds a selection (fold-distillation wave 4) - #84
Conversation
`lgj_plan_group_sum_i32` lowers the fused plan and `GROUP BY key SUM(val)`
as ONE mask-risc program (`plan_lower::lower_group_sum`, the same prefix
rewrite and survivor skip as `plan_eval`, ending in `Terminal::GroupSumI32`
over the accumulator instead of `Keep`). The executor runs it tile by tile;
the only state sized by the answer is the caller's `i64` buffer, one per
group, and that is all that crosses back. Java's `View.sumByGroup` is
therefore one crossing for 16 groups at any row count, where the path it
replaces (plan_eval into a mask, then reduce per group) measured 32.
`via_res`/`via_lane` give the fk-keyed form: the group of a row is the
second table's `u32` lane read through this table's key lane, fused inside
`Terminal::GroupSumViaI32` — no partner-side mask, no remapped key lane.
`View.sumByGroupVia` is the Java spelling, still one crossing.
An empty plan is legal here (there is no destination mask a caller could
fill itself) and lowers to the whole-lane `Pred::Range { 0, n_rows }` — the
one Range this crate emits, pinned to be that range and nothing else so
`exec_error_to_status`'s RangeOutOfBounds arm stays an internal-bug mapping.
Native: 191 tests (8 new: two-crossing-path parity over the combine sweep
on single- and multi-tile populations, empty/all-OR plans against a hand
walk, n_groups as a sink bound, tiled executor vs scalar reference on both
key shapes, the fk-keyed form against a hand-walked join with zero-fallback
drops, every refusal leaving out_sums untouched, and a quack-lowering
convergence arm with Agg::GroupSumI32). Clippy -D warnings and fmt clean.
Java (JDK 28, --enable-preview): 612 checks (409 + GroupSumTest 203):
parity against per-group sumOf, one-crossing cost measured beside the
32-crossing path, the keyed form against an oracle that reads the second
resource's keys through the one named materialiser, argument checks before
any crossing. Minor12 lazy holder, requireMinor(12), an OldAbiCompatTest
gate, and the DoctrineFenceTest pin for the eighth named materialisation
site (Engine.groupSumI32's toArray, sized by groups, never rows).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXUahz73MZxtxWcfpHp9dG
…oard rows `docs/abi.md`: minor 12 in the version history, the symbol count 29 → 30, `lgj_plan_group_sum_i32` in §7's surface table, and §20 — why the grouped sum exists (every reduction consumed a mask; a GROUP BY consumed one per group), what it does (the plan lowered as `plan_eval` lowers it, ending in a sum terminal over a tile-local accumulator), the contract, why one symbol and no scalar twin, the Java spelling, and the six-arm disable table with what each arm observed. Root `CLAUDE.md`: `Engine.groupSumI32`'s `toArray` joins the exhaustive materialisation-site list as the eighth site, sized by the key domain the caller asked for, never by rows. Board: LATEST_STATE entry (measured crossings, gates, what is still owed — the bricks consumer still runs the 32-crossing path) and a STATUS_BOARD section with D-LGJ-FOLD-4 done and D-LGJ-FOLD-5 (the bricks migration) queued. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXUahz73MZxtxWcfpHp9dG
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (18)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. 📝 WalkthroughWalkthroughAdded ABI minor 12 grouped-sum execution for local and foreign keys. Added native lowering, Java APIs, ChangesGrouped sum execution
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Caller
participant View
participant Engine
participant NativeABI
participant GroupTotals
Caller->>View: call sumByGroup
View->>Engine: pass plan, lanes, and group count
Engine->>NativeABI: perform one grouped-sum crossing
NativeABI-->>Engine: return i64 totals
Engine-->>GroupTotals: materialize group-sized result
GroupTotals-->>Caller: expose indexed totals
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The ABI-12 grouped-sum APIs add local and foreign-key aggregation with validation and compatibility coverage; no current merge blocker is identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 44 functions across 14 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
I hop through plans where group sums grow, Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_58fc4011-98ac-4ca6-baf8-0054dee02ea1) |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ed8ecc02-4978-49af-b2e1-3b464562aafc) |
2 participants
The Java → Panama → mask-risc proof of the fold-distillation wave (lance-graph #1256, ndarray #318, both merged): a
GROUP BY … SUMcrosses the membrane once and no selection exists at any point.The gap it closes. Every reduction this ABI carried consumed a mask:
lgj_plan_evallanded the plan in a mask handle, then algj_reduce_*read it. Java never held the selection, but it was still a population-sized intermediate that existed only to be consumed by the very next fold — and aGROUP BYpaid for it once per group (bricks'sumBy()measured 32 crossings for 16 groups).ABI minor 12 — one symbol, no new status, no manifest growth.
lgj_plan_group_sum_i32(res, ops, n_ops, group_lane, val_lane, via_res, via_lane, out_sums, n_groups)lowers the plan exactly asplan_evaldoes (plan_lower::lower_group_sum: same prefix rewrite, same survivor skip) and ends inTerminal::GroupSumI32over the accumulator instead ofKeep. The executor runs it tile by tile; the accumulator isTILE_WORDSof scratch; the only answer-sized state is the caller'si64per group, which is all that crosses back.via_res != 0is the fk-keyed form (SUM(line.amount) GROUP BY partner.country,Terminal::GroupSumViaI32, the indirection fused — no partner-side mask, no remapped key lane).n_ops == 0is legal and means every row: the onePred::Rangethis crate emits, pinned to be0..n_rowsand nothing else soexec_error_to_status'sRangeOutOfBoundsarm stays an internal-bug mapping.Java.
View.sumByGroup(key, value, groups)→GroupTotals(addressed by key, exposes no array) andView.sumByGroupVia(key, via, viaKey, value, groups).Downcalls.Minor12lazy holder,requireMinor(12), anOldAbiCompatTestleg, and theDoctrineFenceTestpin for the eighth named materialisation site (Engine.groupSumI32'stoArray, sized by the key domain the caller typed, never by rows — rootCLAUDE.mdlist updated).Measured, not predicted:
sumByGroup: 1 crossing at 1,024 and at 65,536 rows for 16 groups; the per-group path measured 32 beside it in the same run.sumByGroupVia: 1.{AND,OR}^nsweep on single- and multi-tile populations; empty/all-OR plans against a hand walk;n_groupsas a sink bound; tiled executor vs scalar reference on both key shapes; the fk-keyed form against a hand-walked join incl. zero-fallback drops; every refusal leavingout_sumsuntouched; alance-graph-quackconvergence arm withAgg::GroupSumI32). Clippy-D warnings+ fmt clean.GroupSumTest203) under JDK 28--enable-preview,abi 0.12, ndarray::simd avx512.OldAbiCompatTestboth ways: 13/13 against this library; against a minor-11 library built from07e044f,View.sumByGroupthrowsAbiMismatchExceptionnaming minor 12, every minor-11 feature still working beside it.docs/abi.md§20.6), each restored from the commit.Docs/board:
docs/abi.md§2 history, §7 (29 → 30 symbols), §20;LATEST_STATEentry;STATUS_BOARDD-LGJ-FOLD-4 done, D-LGJ-FOLD-5 (migratebrickssumBy()ontosumByGroup, re-pin 32 → 1) queued.CI here is expected green: lance-graph
mainalready carries #1256.🤖 Generated with Claude Code
https://claude.ai/code/session_01GXUahz73MZxtxWcfpHp9dG
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Compatibility