bricks sumBy() is one fused grouped fold (D-LGJ-FOLD-5), and a red materializeRows() pin on main root-caused

[AdaWorldAPI]

What

The consumer half of ABI minor 12, plus a red pin on main root-caused on the way.

1. bricks sumBy() is one fused grouped fold (D-LGJ-FOLD-5). It was sixteen queries — one sumOf over where(REGION.eq(v)) per region, each paying plan evaluation into a selection mask plus the reduction over it. Now one sumByGroup. No selection mask is built at any point and nothing is asked per group.

2. GraphHopTest's materializeRows() steady-state pin re-pinned 0 → 1. Pre-existing failure, not introduced here.

Measured through the consumer

	n = 1,000	n = 64,000
before	32	32
after	1	1

And, new, across group counts at both row counts:

groups	1	16	64
before	2	32	128
after	1	1	1

The old path was already invariant in rows, which is why its literal was asserted at both row counts. This one is invariant in groups as well — strictly stronger, so it gets its own assertion instead of riding on the row-count arm.

That arm needed a seam. The public sumBy() always asks for exactly Orders.REGIONS groups, so nothing previously reachable varied the group count and "one crossing whatever the group count" was a doc comment no input could falsify. A package-private sumByGroupCount serves that arm alone; getMethods() does not see it, so the aggregate-only-egress guard still audits exactly the public surface the guarantee is about.

Orders.REGIONS replaces three literal 16s with one named source for the native generator's classid cardinality, which the ABI manifest does not report. Because it is a mirror it is pinned against the fixture's own behaviour, not against a copied number — a second Java literal would drift in lockstep with whatever edited it and prove nothing. Two-sided: every id below REGIONS must carry rows, and id REGIONS itself must carry none.

Return type stays Map<Integer, Long> — sized by the question, one entry per group, never by the data. Handing back GroupTotals would leak a facade type into a consumer's public surface for nothing.

The pre-existing red, and why the test moved rather than the code

GraphHopTest reported 1 FAILED / 65 passed on a clean origin/main worktree at bb81d80 — established by running it there, not inferred from this diff's shape.

Mask.words() no longer returns its cached window directly: it re-describes through lgj_mask_describe and compares the returned epoch against its stamp, so a cached address is never read after the substrate moved under it. Root CLAUDE.md names that as the shipped property. The pin asserting a second materializeRows() costs zero was written against the pure-cache behaviour that preceded it. Reverting the number would mean deleting the re-validation.

The plan's §3.5 "describe cached per Mask; zero crossings steady-state" is superseded for Mask: the words are still not re-fetched and the population is still never re-scanned — lgj_mask_describe fills a descriptor and does no work over the rows — it is simply no longer free. Steady-state is constant per call, and the re-pin asserts that rather than flipping a literal: a third call is measured and required to equal the second. A one-off extra describe satisfies a bare == 1 on the second call; only a per-call cost satisfies second == third.

Disable arms — four, each red-then-green

arm	disable	result
A	revert sumBy to the per-group loop	exactly the 8 crossing assertions red (2 / 32 / 128 at 1 / 16 / 64 groups), parity GREEN throughout
B	Orders.REGIONS 16 → 20	the "every id below REGIONS is populated" half red (ids 16–19 empty); the other half stays green
C	Orders.REGIONS 16 → 8	the "id REGIONS carries no rows" half red at 3,960 rows; the other half stays green
D	restore Mask.words() to a pure cache	the magnitude arm red (expected 1, was 0); the constancy arm stays GREEN at 0

Arm A is the one that makes this a migration rather than a behaviour change: the answer is identical, only the cost differs. Note even the degenerate case discriminates — 1 group costs 2 on the old path.

B and C together are why the cardinality check is two-sided: each direction is caught by a different half, so neither half alone would catch both.

D is why the second- and third-call assertions are independent rather than redundant — it kills one and leaves the other green, because they test different properties.

Gates

765 checks: core 612/612, bricks 70/70 (was 62 — the six group-count arms and two cardinality checks are the +8), graph 68/68 (was 65 + 1 failed — the third-call cost and its content check are the +2), trades 12/12 + 3/3. JDK 28 --release 28 --enable-preview against a freshly built abi 0.12, ndarray::simd avx512, release.

The finding worth acting on separately

main was red and nothing reported it. The one workflow gates lgj-abi only — fmt, clippy, test — so no Java compiles in CI at all, and the core suite plus all three consumer suites are local gates run by whoever remembers. Filed as ISS-LGJ-CONSUMERS-HAVE-NO-CI-LINE, with the reason it is not merely "add a job" (JDK 28 is preview-gated and an EA build apt does not carry) and a falsifier: re-introduce the stale pin and the job must go red, since a job that builds the consumers without running their mains would pass and be no-gate-with-extra-steps.

Board hygiene lands in the third commit; the minor-12 section's now-false "Still owed" clause is struck in place with its discharge date rather than deleted.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GXUahz73MZxtxWcfpHp9dG

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: c8517a32-1858-4be9-a013-2b22b03e1177

📥 Commits

Reviewing files that changed from the base of the PR and between bb81d80 and fec1c3f.

📒 Files selected for processing (9)

• .claude/board/ISSUES.md
• .claude/board/LATEST_STATE.md
• .claude/board/STATUS_BOARD.md
• consumers/bricks/src/main/java/com/adaworldapi/bricks/BricksQuery.java
• consumers/bricks/src/main/java/com/adaworldapi/bricks/Orders.java
• consumers/bricks/src/test/java/com/adaworldapi/bricks/BricksAuthTest.java
• consumers/graph/src/main/java/com/adaworldapi/graph/Graph.java
• consumers/graph/src/test/java/com/adaworldapi/graph/GraphHopTest.java
• java/src/main/java/com/adaworldapi/lancegraph/Mask.java

 __________________________________________________________________________________________________________________________________
< Functions delay binding; data structures induce binding. Moral: Structure data late in the programming process. - Alan J. Perlis >
 ----------------------------------------------------------------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot 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_45698556-6495-4b5b-98f4-54ed44f76d74)

[AdaWorldAPI]

@coderabbitai review

Requesting a review explicitly since this is a draft. Two things worth aiming at, because CI cannot see either of them:

1. This repo's CI compiles no Java at all. The only workflow gates lgj-abi with fmt, clippy and test. So a green check suite on this PR says nothing about the change, which is entirely Java. The gate is the local run recorded in the PR body: 765 checks across the core suite and all three consumer suites, on JDK 28 with --release 28 --enable-preview. That gap is filed as ISS-LGJ-CONSUMERS-HAVE-NO-CI-LINE, and it is why main was carrying a red consumer check that this PR fixes.

2. The second commit re-pins a number rather than changing code, so the reasoning is the thing to check. GraphHopTest asserted a second materializeRows() costs zero crossings. It costs one, because Mask.words() re-describes and re-validates its epoch on every facade call. I concluded the pin was stale rather than the behaviour wrong, on the grounds that the re-validation is the shipped safety property named in the root CLAUDE.md, and that reverting the number would mean deleting it. If that reading is wrong, the fix is in the wrong file.

---

Generated by Claude Code

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== changed files ==='
git diff --name-status origin/main...HEAD

echo '=== diff stat ==='
git diff --stat origin/main...HEAD

echo '=== workflows ==='
fd -t f .github 2>/dev/null | sort
for f in $(fd -t f .github/workflows 2>/dev/null | sort); do
  echo "--- $f"
  cat -n "$f"
done

echo '=== candidate files ==='
fd -t f '^(CLAUDE\.md|GraphHopTest\.java|Orders\.java|Bricks.*\.java|.*Bricks.*\.java|Mask\.java)$' . | sort

echo '=== changed diff ==='
git diff --find-renames --find-copies --unified=80 origin/main...HEAD -- . ':(exclude)*.lock' | sed -n '1,2400p'

Length of output: 105227

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== outlines ==='
ast-grep outline java/src/main/java/com/adaworldapi/lancegraph/View.java --items all 2>/dev/null || true
ast-grep outline java/src/main/java/com/adaworldapi/lancegraph/Mask.java --items all 2>/dev/null || true
ast-grep outline java/src/main/java/com/adaworldapi/lancegraph/GroupTotals.java --items all 2>/dev/null || true
ast-grep outline consumers/bricks/src/main/java/com/adaworldapi/bricks/BricksQuery.java --items all 2>/dev/null || true
ast-grep outline consumers/bricks/src/test/java/com/adaworldapi/bricks/BricksAuthTest.java --items all 2>/dev/null || true

echo '=== grouped-fold definitions and use ==='
rg -n -C 12 --glob '*.java' '\bsumByGroup\s*\(' java consumers

echo '=== Mask words and materialization ==='
rg -n -C 20 --glob '*.java' '\b(words|materializeRows)\s*\(' java/src/main/java consumers/graph/src/main/java

echo '=== group validation and GroupTotals API ==='
rg -n -C 14 --glob '*.java' '(groups|GroupTotals|sumByGroup)' java/src/main/java/com/adaworldapi/lancegraph/View.java java/src/main/java/com/adaworldapi/lancegraph/GroupTotals.java

echo '=== exact changed Java files ==='
git diff --check origin/main...HEAD
git diff --unified=12 origin/main...HEAD -- \
  consumers/bricks/src/main/java/com/adaworldapi/bricks/BricksQuery.java \
  consumers/bricks/src/main/java/com/adaworldapi/bricks/Orders.java \
  consumers/bricks/src/test/java/com/adaworldapi/bricks/BricksAuthTest.java \
  consumers/graph/src/test/java/com/adaworldapi/graph/GraphHopTest.java | tail -n +1 | sed -n '1,2600p'

Length of output: 106767

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== stale materialization-cost claims ==='
rg -n -i -C 4 --glob '*.java' '(zero further crossings|zero.*crossing.*repeated|zero crossings.*repeated|no further crossing|after the first call.*no.*crossing|steady-state)' java consumers

echo '=== changed group-count wording ==='
rg -n -C 3 --glob '*.java' '(two group counts|group counts|new int\[\].*\{1, Orders\.REGIONS)' \
  consumers/bricks/src/main/java/com/adaworldapi/bricks/BricksQuery.java \
  consumers/bricks/src/test/java/com/adaworldapi/bricks/BricksAuthTest.java

echo '=== Graph and Mask Javadoc ranges ==='
sed -n '35,55p;170,220p;255,340p' java/src/main/java/com/adaworldapi/lancegraph/Mask.java
sed -n '208,225p' consumers/graph/src/main/java/com/adaworldapi/graph/Graph.java

echo '=== all direct Java literals tied to region cardinality in bricks ==='
rg -n -C 2 --glob '*.java' '\b16\b|0\.\.15|0\.\.16' consumers/bricks

Length of output: 33662

---

@AdaWorldAPI I found two documentation defects.

1. materializeRows() documentation is now false.
   Mask.words() calls Engine.describeMask(handle) on each cached use. The new GraphHopTest pin correctly requires one crossing for the second and third calls. However, Mask.java still says repeated calls have “no further crossing at all.” Graph.java also says repeated materializeRows() calls have “zero further crossings.” Update both contracts to state one constant lifecycle crossing per call. The cached window still avoids a bulk fetch and a population scan.

2. BricksQuery.sumBy() describes the wrong number of group-count arms.
   The Javadoc says that BricksAuthTest pins the cost across “two group counts.” The test uses three group counts: 1, Orders.REGIONS, and 4 * Orders.REGIONS. Update the text to match the test.

The fused sumByGroup migration preserves the Map<Integer, Long> consumer contract. The package-private seam exercises group-count invariance. The GraphHopTest re-pin matches Mask.words() and its per-call epoch re-validation.

---

✏️ Learnings added

Learnt from: AdaWorldAPI
URL: https://github.com/AdaWorldAPI/lance-graph-java/pull/85

Timestamp: 2026-09-22T07:11:52.847Z
Learning: In lance-graph-java, `Mask.words()` re-describes the native mask through `Engine.describeMask(handle)` on every cached use to validate the epoch and parent liveness. Therefore, each `Mask.materializeRows()` call has one constant lifecycle crossing, including repeated calls; the cached descriptor avoids a bulk fetch and population scan but does not make later scans crossing-free.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[AdaWorldAPI]

Both findings were real and both were mine. Fixed in fec1c3f.

1. The two contracts. You are right, and the way I got it wrong is the instructive part: I re-pinned the test and left two javadoc contracts asserting the behaviour the pin had just contradicted. Mask.java promised "no further crossing at all" after the first call; Graph.java promised "zero further crossings for repeated calls". Both describe the pure cache that preceded the epoch re-validation.

Corrected in place, with the struck text retained, and stating what the cache does still buy rather than only what it no longer does: the words are never re-fetched and the population is never re-scanned, because lgj_mask_describe fills a descriptor and does no work over the rows. Constant per call, not zero after the first.

The previous commit message argued that a doc-comment claim is not a behaviour, then left two doc comments claiming the opposite of the behaviour it had just measured. No test could have caught that, because a javadoc contract has none.

2. The miscount. Also right. The javadoc said "two group counts"; the test uses three, 1, Orders.REGIONS, and 4 * Orders.REGIONS. Wrong in the same paragraph that argues the group-count arm is what makes the claim falsifiable. Now states all three.

Re-ran after the fix: core 612/612, bricks 70/70, graph 68/68, trades 12/12 and 3/3.

One note on your learning entry, which is accurate and worth keeping: the per-call crossing applies to Mask. RowStore's cached lanes consult only a Java-side closed flag and are not generation-checked, which is a measured won't-fix rather than an oversight. The per-access probe for it was measured at +52.76 ns/call against a pre-registered sample and failed its gate. The residue is tracked as ISS-LGJ-CACHED-DESCRIPTOR-CROSS-THREAD-WINDOW.

---

Generated by Claude Code

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot 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_4cf811a0-5659-4993-95c1-edd03add9d1d)