Skip to content

Commit fec1c3f

Browse files
committed
fix two contracts that still promised the zero-crossing cache, and a miscount of my own
CodeRabbit review on #85, both findings real and both mine. 1. I re-pinned the TEST and left two JAVADOC CONTRACTS asserting the behaviour the pin had just contradicted. `Mask.java` said every materializeRows() after the first is "an in-process segment access with no further crossing at all"; `Graph.java` said "a one-time describe, then zero further crossings for repeated calls". Both describe the pure cache that preceded the epoch re-validation. Corrected in place with the struck text kept, and stating what the cache DOES still buy, which is the part worth keeping: 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. The cost is constant per call, not zero after the first. Worth naming plainly: the previous commit's message argued that a doc-comment claim is not a behaviour, and then left two doc comments claiming the opposite of the behaviour it had just measured. No test could catch it, because a javadoc contract has none -- review did. 2. `BricksQuery.sumBy()` said BricksAuthTest pins the cost across "two group counts". It uses three: 1, Orders.REGIONS, and 4 * Orders.REGIONS. My own miscount, in the same paragraph that argues the group-count arm is what makes the claim falsifiable. Gates unchanged and re-run: core 612/612, bricks 70/70, graph 68/68, trades 12/12 + 3/3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXUahz73MZxtxWcfpHp9dG
1 parent 555e69a commit fec1c3f

3 files changed

Lines changed: 26 additions & 9 deletions

File tree

‎consumers/bricks/src/main/java/com/adaworldapi/bricks/BricksQuery.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ public long sum(com.adaworldapi.lancegraph.I32Field field) {
104104
* selection mask, then the reduction), measured here at <strong>32</strong>. That path was
105105
* invariant in the number of rows but proportional to the number of groups; this one is
106106
* invariant in both, which is the stronger claim and is asserted as such — {@code
107-
* BricksAuthTest} pins the cost at 1 across two row counts <em>and</em> two group counts.
107+
* BricksAuthTest} pins the cost at 1 across two row counts <em>and</em> three group
108+
* counts: 1, {@link Orders#REGIONS}, and four times {@code REGIONS}.
108109
*
109110
* <p>Groups come from {@link Orders#REGIONS}, the fixture's region cardinality. Every id in
110111
* {@code 0..REGIONS-1} appears as a key in the returned map, including ids no selected row

‎consumers/graph/src/main/java/com/adaworldapi/graph/Graph.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,11 @@ public long count() {
215215
/**
216216
* The current frontier's row indices — the ONE named materialising terminal on this class (root
217217
* {@code CLAUDE.md}'s "named exceptions": row ids out). {@code O(n)} in the frontier's
218-
* population; see {@link Mask#materializeRows()} for the exact cost shape (a one-time describe,
219-
* then zero further crossings for repeated calls against the same underlying {@link Mask}).
218+
* population; see {@link Mask#materializeRows()} for the exact cost shape — one lifecycle
219+
* crossing per call, repeated calls included, because the cached window is re-validated
220+
* against the registry every time rather than read on trust. Corrected 2026-09-22: this
221+
* read *"a one-time describe, then zero further crossings for repeated calls"*, which was
222+
* the pure-cache behaviour that preceded the re-validation.
220223
*/
221224
public long[] materializeRows() {
222225
return frontier == null ? new long[0] : frontier.materializeRows();

‎java/src/main/java/com/adaworldapi/lancegraph/Mask.java‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,25 @@ public final class Mask implements AutoCloseable {
4040
private boolean closed;
4141

4242
/**
43-
* The mask's own packed-bit word window, resolved once via {@code lgj_mask_describe} (a
44-
* lifecycle crossing, per abi.md §6, not a bulk one) and cached: mask storage is allocated
45-
* once and never reallocated, resized, or moved while the resource is alive (a hard ABI
46-
* guarantee), the same invariant {@link RowStore#rawLane()} relies on for its own caching.
47-
* Every read or write through {@link #materializeRows()} after the first call is an
48-
* in-process segment access with no further crossing at all.
43+
* The mask's own packed-bit word window, obtained via {@code lgj_mask_describe} (a lifecycle
44+
* crossing, per abi.md §6, never a bulk one) and cached: mask storage is allocated once and
45+
* never reallocated, resized, or moved while the resource is alive (a hard ABI guarantee),
46+
* the same invariant {@link RowStore#rawLane()} relies on for its own caching.
47+
*
48+
* <p><strong>The cache is re-validated, not trusted, so every facade call pays one lifecycle
49+
* crossing — including calls after the first.</strong> {@link #words()} re-describes and
50+
* compares the returned epoch against the stamp it holds, so a cached address can never be
51+
* read after the substrate has moved under it. What the cache still buys is what matters:
52+
* the words are never re-fetched and the population is never re-scanned, because
53+
* {@code lgj_mask_describe} fills a descriptor and does no work over the rows. The cost is
54+
* therefore CONSTANT PER CALL, not zero after the first.
55+
*
56+
* <p>⊘ This paragraph replaces *"resolved once … and cached: every read or write through
57+
* {@link #materializeRows()} after the first call is an in-process segment access with no
58+
* further crossing at all"*, which described the pure-cache behaviour that preceded the
59+
* re-validation and was false once it landed. Corrected 2026-09-22 alongside the
60+
* {@code GraphHopTest} pin that measured it; found by review, not by a test, because a
61+
* javadoc contract has none.
4962
*/
5063
private Engine.LaneWindow words;
5164

0 commit comments

Comments
 (0)