From 46fb43b854cf8fb51b6b1dbcc6b518e1f443a891 Mon Sep 17 00:00:00 2001 From: Paddy Mullen Date: Wed, 24 Jun 2026 15:39:43 -0400 Subject: [PATCH 1/2] =?UTF-8?q?test(row-cache):=20failing=20test=20?= =?UTF-8?q?=E2=80=94=20rowidsInRange=20must=20clamp=20negative=20start?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IdentityView.rowidsInRange(-3, 2) returns nonexistent negative rowids ([-3,-2,-1,0,1]) while SortView/FilterView return an empty slice for the same range. rowidsInRange already clamps end and handles inverted ranges defensively, so the negative-start divergence is an oversight, not a contract choice. This becomes a live bug once 7b wires rowidsInRange into the AG-Grid datasource at a boundary window near row 0. Test only — fix follows. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/DFViewerParts/Views.test.ts | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/buckaroo-js-core/src/components/DFViewerParts/Views.test.ts b/packages/buckaroo-js-core/src/components/DFViewerParts/Views.test.ts index 7458363da..3bdd29487 100644 --- a/packages/buckaroo-js-core/src/components/DFViewerParts/Views.test.ts +++ b/packages/buckaroo-js-core/src/components/DFViewerParts/Views.test.ts @@ -166,6 +166,31 @@ describe("View contract — invariants across all view kinds", () => { }); +describe("View contract — rowidsInRange clamps a negative start", () => { + // rowidsInRange already clamps end at length() and returns [] for an + // inverted range (see the IdentityView/SortView tests above), so it is + // a defensive range accessor. A negative start must clamp to 0 the same + // way, and all three view kinds must agree. Otherwise a boundary window + // (the renderer asking for e.g. [-3, 2) near row 0) yields nonexistent + // negative rowids from IdentityView but an empty slice from + // SortView/FilterView — a silent divergence the RowStore can't satisfy. + const cases: Array<[string, View]> = [ + ["IdentityView(8)", new IdentityView(8)], + ["SortView", new SortView("k", "asc", Int32Array.from([3, 1, 4, 1, 5, 9, 2, 6]))], + ["FilterView", new FilterView("f", Int32Array.from([3, 1, 4, 1, 5, 9, 2, 6]))], + ]; + + test.each(cases)("%s — rowidsInRange(-3, 2) clamps start to 0", (_label, v) => { + expect(v.rowidsInRange(-3, 2)).toStrictEqual(v.rowidsInRange(0, 2)); + }); + + test("IdentityView never emits negative rowids for a negative start", () => { + const v = new IdentityView(10); + expect(v.rowidsInRange(-3, 2)).toStrictEqual([0, 1]); + }); +}); + + describe("View — out-of-bounds positionAt", () => { // Phase 2 contract: callers are expected to clamp via length() // before calling positionAt. The view does not need to be defensive From c755463306dce3baa7a2a9eb9c8438bf225d2834 Mon Sep 17 00:00:00 2001 From: Paddy Mullen Date: Wed, 24 Jun 2026 15:42:30 -0400 Subject: [PATCH 2/2] fix(row-cache): clamp negative start in View.rowidsInRange Clamp start to 0 in IdentityView/SortView/FilterView.rowidsInRange so all three view kinds agree and never index out of range. rowidsInRange already clamped end and handled inverted ranges; this completes that defensive contract on the start side. The positionAt "callers must clamp" contract is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/components/DFViewerParts/Views.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/buckaroo-js-core/src/components/DFViewerParts/Views.ts b/packages/buckaroo-js-core/src/components/DFViewerParts/Views.ts index bfbc215d8..e5f1ea38f 100644 --- a/packages/buckaroo-js-core/src/components/DFViewerParts/Views.ts +++ b/packages/buckaroo-js-core/src/components/DFViewerParts/Views.ts @@ -53,10 +53,11 @@ export class IdentityView implements View { } public rowidsInRange(start: number, end: number): number[] { + const clampedStart = Math.max(0, start); const clampedEnd = Math.min(end, this._length); - if (clampedEnd <= start) return []; - const out: number[] = new Array(clampedEnd - start); - for (let i = 0; i < out.length; i++) out[i] = start + i; + if (clampedEnd <= clampedStart) return []; + const out: number[] = new Array(clampedEnd - clampedStart); + for (let i = 0; i < out.length; i++) out[i] = clampedStart + i; return out; } @@ -86,9 +87,10 @@ export class SortView implements View { } public rowidsInRange(start: number, end: number): number[] { + const clampedStart = Math.max(0, start); const clampedEnd = Math.min(end, this.rowidOrder.length); - if (clampedEnd <= start) return []; - return Array.from(this.rowidOrder.subarray(start, clampedEnd)); + if (clampedEnd <= clampedStart) return []; + return Array.from(this.rowidOrder.subarray(clampedStart, clampedEnd)); } public viewKey(): string { @@ -115,9 +117,10 @@ export class FilterView implements View { } public rowidsInRange(start: number, end: number): number[] { + const clampedStart = Math.max(0, start); const clampedEnd = Math.min(end, this.rowidSubset.length); - if (clampedEnd <= start) return []; - return Array.from(this.rowidSubset.subarray(start, clampedEnd)); + if (clampedEnd <= clampedStart) return []; + return Array.from(this.rowidSubset.subarray(clampedStart, clampedEnd)); } public viewKey(): string {