From d540eed8dc3f858d889fceba1acd4227be0d133c Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Sun, 5 Apr 2026 13:34:47 +1000 Subject: [PATCH 1/5] only one idToPos fn --- client/lib/gumnut/floor.ts | 6 +-- client/lib/ymodel/api.test.ts | 4 +- client/lib/ymodel/api.ts | 18 +++------ client/lib/ymodel/internal/state.ts | 56 +++++++++++---------------- client/lib/ymodel/work-holder.test.ts | 2 +- client/lib/ymodel/work-holder.ts | 18 ++++----- 6 files changed, 41 insertions(+), 63 deletions(-) diff --git a/client/lib/gumnut/floor.ts b/client/lib/gumnut/floor.ts index 297373d..fda4c90 100644 --- a/client/lib/gumnut/floor.ts +++ b/client/lib/gumnut/floor.ts @@ -209,9 +209,9 @@ export class GumnutFloor { }; } - resolveRef(r: InoPosRef) { + resolveRef(r: InoPosRef): number { const impl = r as InoPosRefImpl; - return this.low.byIno(impl.target).posOf(impl.id); + return this.low.byIno(impl.target).idToPos(impl.id).pos; } perform(fn: FloorFn): InoChanges { @@ -225,7 +225,7 @@ export class GumnutFloor { }, resolveRef(r) { const impl = r as InoPosRefImpl; - return byIno(impl.target).posOf(impl.id); + return byIno(impl.target).idToPos(impl.id).pos; }, }; yield* fn(api); diff --git a/client/lib/ymodel/api.test.ts b/client/lib/ymodel/api.test.ts index 83a2d09..9ebb6ed 100644 --- a/client/lib/ymodel/api.test.ts +++ b/client/lib/ymodel/api.test.ts @@ -35,7 +35,7 @@ test('api', () => { api.applyOp({ r: [0], length: 7 }); const id = api.posToId(2); - const xxSet = api.applySet({ skip: api.posOf(id), body: [88, 88] }); // 'XX' + const xxSet = api.applySet({ skip: api.idToPos(id).pos, body: [88, 88] }); // 'XX' assert.strictEqual(api.length(), 7); assert.deepStrictEqual( @@ -49,7 +49,7 @@ test('api', () => { yield ['abc']; xxSet.rollback(); - api.applySet({ skip: api.posOf(id), body: [89, 89] }); // 'YY' + api.applySet({ skip: api.idToPos(id).pos, body: [89, 89] }); // 'YY' ++calls; }); diff --git a/client/lib/ymodel/api.ts b/client/lib/ymodel/api.ts index 961a462..5cf1cd9 100644 --- a/client/lib/ymodel/api.ts +++ b/client/lib/ymodel/api.ts @@ -67,11 +67,8 @@ export abstract class GumnutLow { posToId(pos: number): Id { return out.state.posToId(pos); }, - posOf(id: Id): number { - return out.state.posOf(id); - }, - posOfValid(id: Id): number { - return out.state.posOfValid(id); + idToPos(id) { + return out.state.idToPos(id); }, }, update: undefined, @@ -239,7 +236,7 @@ export abstract class GumnutLow { /** * Lookup where an ID is before the run of the given sequence, or -ve to check all. */ - private partialLookupId(beforeSeq: number, ino: string, id: Id) { + private partialLookupId(beforeSeq: number, ino: string, id: Id): number { let extra = 0; for (let i = this.pendingWork.length - 1; i >= 0; --i) { const { seq, pending } = this.pendingWork[i]; @@ -256,13 +253,8 @@ export abstract class GumnutLow { // otherwise, find it in the server const is = this._byIno.get(ino); - const out = is.serverState.posOfValid(id); - - if (out < 0) { - throw new Error(`panic; couldn't lookup ID, server didn't have it: ${id}`); - } - - return out; + const out = is.serverState.idToPos(id); + return out.pos; } /** diff --git a/client/lib/ymodel/internal/state.ts b/client/lib/ymodel/internal/state.ts index 4242104..c8727a9 100644 --- a/client/lib/ymodel/internal/state.ts +++ b/client/lib/ymodel/internal/state.ts @@ -3,6 +3,11 @@ import type { Op, SetPart } from '../types.ts'; import { zeroId, allocLocalId, type Id } from './shared.ts'; import type { RangeUpdate } from './range.ts'; +export type IdLookup = { + pos: number; + gone: boolean; +}; + /** * Internal state. */ @@ -44,51 +49,34 @@ export class RopeState { } /** - * Resolves this ID, including its most valid neighbor if deleted, or returns -1. + * Resolves this ID, including its most valid >=0 pos and whether it is gone. + * + * If the ID is not valid here, throws. */ - public posOf(id: Id): number { + public idToPos(id: Id): IdLookup { if (id === 0) { - return 0; + return { pos: 0, gone: false }; } const highId = this.tree.equalAfter(id); if (!highId) { - return -1; + // out of range - ID above all known IDs + throw new Error(`invalid ID for rope: ${id}`); } // find high pos - const pos = this.rope.find(highId); - - // confirm we're not in a deleted section - if (id !== highId) { - const lookup = this.rope.lookup(highId); - if (lookup.length === 0) { - return pos; - } - } - return pos - (highId - id); - } + const lookup = this.rope.lookup(highId); - /** - * Resolves this ID, returning -1 if deleted or not here. - */ - public posOfValid(id: Id) { - if (id === 0) { - return 0; - } - const highId = this.tree.equalAfter(id); - if (!highId) { - return -1; + const fromEnd = highId - id; + if (fromEnd > lookup.data.length) { + // out of range - the nearest ID didn't have data containing us + throw new Error(`invalid ID for rope: ${id}`); } - // confirm we're not in a deleted section - const lookup = this.rope.lookup(highId); + const pos = this.rope.find(highId); if (lookup.length === 0) { - return -1; + return { pos, gone: true }; } - - // find high pos - const pos = this.rope.find(highId); - return pos - (highId - id); + return { pos: pos - fromEnd, gone: false }; } /** @@ -177,7 +165,7 @@ export class RopeState { this.tree.insert(newId); if (exists) { - const pos = this.posOf(at); + const pos = this.rope.find(at); return { lo: pos, hi: pos, length }; } } @@ -211,7 +199,7 @@ export class RopeState { return; } - const pos = this.posOf(lo); + const pos = this.rope.find(lo); return { lo: pos, hi: pos + cleared, length: 0 }; } diff --git a/client/lib/ymodel/work-holder.test.ts b/client/lib/ymodel/work-holder.test.ts index d75d7ac..f628528 100644 --- a/client/lib/ymodel/work-holder.test.ts +++ b/client/lib/ymodel/work-holder.test.ts @@ -16,7 +16,7 @@ test('number ranges', () => { w.applySet({ skip: 2, body: ['server'] }); const out = w.forServer((id) => { - return actual.posOf(id); + return actual.idToPos(id).pos; }); // allows contiguous - we set a/b in new data, and then something at server pos 0 diff --git a/client/lib/ymodel/work-holder.ts b/client/lib/ymodel/work-holder.ts index eb9bd18..144bd8c 100644 --- a/client/lib/ymodel/work-holder.ts +++ b/client/lib/ymodel/work-holder.ts @@ -1,10 +1,11 @@ import type { Op, Patch, SetPart } from './types.ts'; import type { Id } from './internal/shared.ts'; import { + type IdLookup, type InternalPatch, type InternalSet, - rollbackForOp, type RopeState, + rollbackForOp, } from './internal/state.ts'; import type { RangeUpdate } from './internal/range.ts'; @@ -13,8 +14,7 @@ export interface ReadApi { read(start?: number, end?: number): X[]; posToId(pos: number): Id; - posOf(id: Id): number; - posOfValid(id: Id): number; + idToPos(id: Id): IdLookup; } export interface PatchApi extends ReadApi { @@ -154,12 +154,8 @@ export class WorkHolder implements PatchApi { return this.effectiveState().posToId(pos); } - posOf(id: Id): number { - return this.effectiveState().posOf(id); - } - - posOfValid(id: Id): number { - return this.effectiveState().posOfValid(id); + idToPos(id: Id): IdLookup { + return this.effectiveState().idToPos(id); } private effectiveState(): RopeState { @@ -264,9 +260,11 @@ function mergePatchSet( // aggregate first for (const set of src) { set.ids.forEach((id, index) => { - if (m.posOfValid(id) === -1) { + const { gone } = m.idToPos(id); + if (gone) { return; // don't send now-deleted sets } + let target = combinedLookupId(0, id); if (target === 0) { throw new Error(`panic; can't set at zero`); From bc84fc2ba019f9af5047304cda84929f021ef9a7 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Sun, 5 Apr 2026 14:33:07 +1000 Subject: [PATCH 2/5] fix length semantics --- client/lib/ymodel/internal/state.test.ts | 33 +++++++++++++++++ client/lib/ymodel/internal/state.ts | 45 ++++++++++++++++++++---- 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/client/lib/ymodel/internal/state.test.ts b/client/lib/ymodel/internal/state.test.ts index 99642a9..5519624 100644 --- a/client/lib/ymodel/internal/state.test.ts +++ b/client/lib/ymodel/internal/state.test.ts @@ -18,3 +18,36 @@ test('state', () => { r.applySet(r.coerceSet({ skip: 3, body: [999] })); assert.deepStrictEqual(r.read(), [97, 98, 99, 999, 2]); }); + +test('length complex semantics', () => { + const r = RopeState.new(); + assert.strictEqual(r.length(), 0); + + // just posToId shouldn't increase effective length + r.posToId(10); + assert.strictEqual(r.length(), 0, 'posToId should not increase length'); + + // applySet should increase length + r.applySet(r.coerceSet({ skip: 4, body: [42] })); + assert.strictEqual(r.length(), 5, 'applySet at index 4 should make length 5'); + + // insert at the beginning should increase length + r.applyOp(r.coerceOp({ r: [0], length: 2 })); + assert.strictEqual(r.length(), 7, 'insert 2 at start should increase length to 7'); + + // delete at the beginning should decrease length + r.applyOp(r.coerceOp({ r: [0, 2] })); + assert.strictEqual(r.length(), 5, 'delete 2 at start should decrease length back to 5'); + + // insert far ahead should NOT increase length + r.applyOp(r.coerceOp({ r: [10], length: 2 })); + assert.strictEqual(r.length(), 5, 'insert far ahead (pos 10) should not increase length'); + + // deleting far ahead should NOT decrease length + r.applyOp(r.coerceOp({ r: [10, 12] })); + assert.strictEqual(r.length(), 5, 'delete far ahead (pos 10-12) should not decrease length'); + + // insert at end should increase length + r.applyOp(r.coerceOp({ r: [5], length: 4 })); + assert.strictEqual(r.length(), 9, 'insert at end should increase length'); +}); diff --git a/client/lib/ymodel/internal/state.ts b/client/lib/ymodel/internal/state.ts index c8727a9..ca26467 100644 --- a/client/lib/ymodel/internal/state.ts +++ b/client/lib/ymodel/internal/state.ts @@ -14,11 +14,15 @@ export type IdLookup = { export class RopeState { private readonly rope: Rope; private readonly tree: AATree; + private effectiveLength: number; public static new() { return new RopeState(); } + /** + * Clones this {@link RopeState}, including its underlying data. + */ public clone(): RopeState { return new RopeState(this); } @@ -27,21 +31,33 @@ export class RopeState { if (other) { this.rope = other.rope.clone(); this.tree = other.tree.clone(); + this.effectiveLength = other.effectiveLength; } else { this.rope = new Rope(zeroId); this.tree = new AATree((a, b) => a - b); + this.effectiveLength = 0; } } + /** + * Returns the highest point data was ever set at, plus its future moves. + * + * For example, if we set data at index 4, this will return length of 5. + * If we then insert 2 empty elements at position 5, this will return length of 7. + * In this way, we can grow from the zero point. + */ public length() { - return this.rope.length(); + return this.effectiveLength; } - public read(start?: number, end?: number): X[] { + /** + * Reads the data here, optionally slicing it. + */ + public read(start?: number, end: number = this.effectiveLength): X[] { let out: X[] = []; for (const part of this.rope.iter(zeroId)) { if (part.length) { - out = out.concat(part.data); // concat to retain empty + out = out.concat(part.data); // concat to retain empty items } } @@ -164,10 +180,16 @@ export class RopeState { this.rope.insertAfter(at, newId, exists ? length : 0, new Array(length)); this.tree.insert(newId); - if (exists) { - const pos = this.rope.find(at); - return { lo: pos, hi: pos, length }; + if (!exists) { + return; } + + const pos = this.rope.find(at); + if (pos <= this.effectiveLength) { + this.effectiveLength += length; + } + + return { lo: pos, hi: pos, length }; } /** @@ -200,6 +222,13 @@ export class RopeState { } const pos = this.rope.find(lo); + + if (pos < this.effectiveLength) { + const fromEnd = this.effectiveLength - pos; + const effectiveCleared = Math.min(fromEnd, cleared); + this.effectiveLength -= effectiveCleared; + } + return { lo: pos, hi: pos + cleared, length: 0 }; } @@ -245,7 +274,7 @@ export class RopeState { throw new Error(`TODO: support flip`); default: - throw new Error(`unsupported Op`); + throw new Error(`unsupported InternalOp`); } } @@ -294,6 +323,8 @@ export class RopeState { const at = this.rope.find(hostId) - (hostId - id); pos.push(at); + + this.effectiveLength = Math.max(this.effectiveLength, at); }); if (!pos.length) { From f8221d02a1c6c4e6ac31264813326430027197c8 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Sun, 5 Apr 2026 14:50:46 +1000 Subject: [PATCH 3/5] store data as regular array --- client/lib/ymodel/internal/state.ts | 106 +++++++++++++++------------- 1 file changed, 57 insertions(+), 49 deletions(-) diff --git a/client/lib/ymodel/internal/state.ts b/client/lib/ymodel/internal/state.ts index ca26467..32dff46 100644 --- a/client/lib/ymodel/internal/state.ts +++ b/client/lib/ymodel/internal/state.ts @@ -12,10 +12,15 @@ export type IdLookup = { * Internal state. */ export class RopeState { - private readonly rope: Rope; + private readonly rope: Rope; private readonly tree: AATree; - private effectiveLength: number; + private data: X[]; + /** + * Creates a new {@link RopeState} to store {@link X}. + * + * Internally the data is stored in a regular {@link Array} which, in V8, is going to be HOLY. + */ public static new() { return new RopeState(); } @@ -31,11 +36,11 @@ export class RopeState { if (other) { this.rope = other.rope.clone(); this.tree = other.tree.clone(); - this.effectiveLength = other.effectiveLength; + this.data = other.data.slice(); } else { this.rope = new Rope(zeroId); this.tree = new AATree((a, b) => a - b); - this.effectiveLength = 0; + this.data = []; } } @@ -47,21 +52,14 @@ export class RopeState { * In this way, we can grow from the zero point. */ public length() { - return this.effectiveLength; + return this.data.length; } /** * Reads the data here, optionally slicing it. */ - public read(start?: number, end: number = this.effectiveLength): X[] { - let out: X[] = []; - for (const part of this.rope.iter(zeroId)) { - if (part.length) { - out = out.concat(part.data); // concat to retain empty items - } - } - - return out.slice(start, end); // TODO: do something better than slice at end + public read(start?: number, end?: number): X[] { + return this.data.slice(start, end); } /** @@ -83,7 +81,7 @@ export class RopeState { const lookup = this.rope.lookup(highId); const fromEnd = highId - id; - if (fromEnd > lookup.data.length) { + if (fromEnd > lookup.data) { // out of range - the nearest ID didn't have data containing us throw new Error(`invalid ID for rope: ${id}`); } @@ -111,7 +109,7 @@ export class RopeState { if (pos > this.rope.length()) { const extra = pos - this.rope.length(); const newId = allocLocalId(extra); - this.rope.insertAfter(this.rope.last(), newId, extra, new Array(extra)); + this.rope.insertAfter(this.rope.last(), newId, extra, extra); this.tree.insert(newId); return newId; } @@ -142,13 +140,12 @@ export class RopeState { const exists = host.length !== 0; if (highId !== id) { - const fromLeft = host.data.length - (highId - id); - const left = host.data.slice(0, fromLeft); - const right = host.data.slice(fromLeft); + const left = host.data - (highId - id); + const right = host.data - left; // adjust right node, insert new left node before it - this.rope.adjust(host.id, right, exists ? right.length : 0); - this.rope.insertAfter(host.prevId, id, exists ? left.length : 0, left); + this.rope.adjust(host.id, right, exists ? right : 0); + this.rope.insertAfter(host.prevId, id, exists ? left : 0, left); this.tree.insert(id); } @@ -177,7 +174,7 @@ export class RopeState { const exists = this.ensureIdEdge(at); - this.rope.insertAfter(at, newId, exists ? length : 0, new Array(length)); + this.rope.insertAfter(at, newId, exists ? length : 0, length); this.tree.insert(newId); if (!exists) { @@ -185,8 +182,12 @@ export class RopeState { } const pos = this.rope.find(at); - if (pos <= this.effectiveLength) { - this.effectiveLength += length; + if (pos === this.data.length) { + // add holes at end + this.data.length += length; + } else if (pos < this.data.length) { + // need to rewrite to create holes in middle + this.data = this.data.slice(0, pos).concat(new Array(length)).concat(this.data.slice(pos)); } return { lo: pos, hi: pos, length }; @@ -223,10 +224,17 @@ export class RopeState { const pos = this.rope.find(lo); - if (pos < this.effectiveLength) { - const fromEnd = this.effectiveLength - pos; + if (pos < this.data.length) { + const fromEnd = this.data.length - pos; const effectiveCleared = Math.min(fromEnd, cleared); - this.effectiveLength -= effectiveCleared; + + if (fromEnd === cleared) { + // trim end + this.data = this.data.slice(0, pos); + } else { + // trim in middle + this.data.splice(pos, effectiveCleared); + } } return { lo: pos, hi: pos + cleared, length: 0 }; @@ -305,41 +313,41 @@ export class RopeState { * Returns the 'real-space' {@link RangeUpdate} for this work. */ public applySet(int: InternalSet): RangeUpdate | undefined { - const pos: number[] = []; - if (int.ids.length !== int.data.length) { throw new Error(`panic; bad IDs/data mismatch`); } - int.ids.forEach((id, index) => { + let any = false; + let lo = 0; + let hi = 0; + + int.ids.forEach((id, i) => { const hostId = this.tree.equalAfter(id)!; const e = this.rope.lookup(hostId); if (!e.length) { return; // deleted } - const off = e.data.length - (hostId - id) - 1; - e.data[off] = int.data[index]; - - const at = this.rope.find(hostId) - (hostId - id); - pos.push(at); - - this.effectiveLength = Math.max(this.effectiveLength, at); + // we have to track every pos to return RangeUpdate + // set the data here with JS array semantics - grow as needed + const pos = this.rope.find(hostId) - (hostId - id); + const index = pos - 1; + this.data[index] = int.data[i]; + + // maintain lo/hi for RangeUpdate + if (!any) { + lo = index; + hi = pos; + any = true; + } else { + lo = Math.min(lo, index); + hi = Math.max(hi, pos); + } }); - if (!pos.length) { - return undefined; + if (any) { + return { lo, hi, length: hi - lo }; } - - // otherwise, because IDs can be anywhere, ... find the lo/hi - // this is stupidly slow because a set might apply in literally any placement after xform - - pos.sort((a, b) => a - b); - - const lo = pos[0] - 1; - const hi = pos.at(-1)!; - - return { lo, hi, length: hi - lo }; } } From 7004009a50301f2b5884e6008c91cc0f2353f855 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Sun, 5 Apr 2026 15:09:35 +1000 Subject: [PATCH 4/5] shared helpers --- client/lib/gumnut/floor.test.ts | 2 +- client/lib/gumnut/floor.ts | 2 +- client/lib/gumnut/wire.ts | 2 +- client/lib/model/layout.test.ts | 21 -- client/lib/model/layout.ts | 249 ------------------ client/lib/shared/array.test.ts | 61 +++++ .../lib/{ymodel/helper.ts => shared/array.ts} | 20 ++ client/lib/ymodel/api.test.ts | 2 +- client/lib/ymodel/internal/state.test.ts | 5 + client/lib/ymodel/work-holder.test.ts | 2 +- 10 files changed, 91 insertions(+), 275 deletions(-) delete mode 100644 client/lib/model/layout.test.ts delete mode 100644 client/lib/model/layout.ts create mode 100644 client/lib/shared/array.test.ts rename client/lib/{ymodel/helper.ts => shared/array.ts} (55%) diff --git a/client/lib/gumnut/floor.test.ts b/client/lib/gumnut/floor.test.ts index 435fd9c..7bfe431 100644 --- a/client/lib/gumnut/floor.test.ts +++ b/client/lib/gumnut/floor.test.ts @@ -1,7 +1,7 @@ import test from 'node:test'; import * as assert from 'node:assert'; import { GumnutFloor } from './floor.ts'; -import { stringToArray } from '../ymodel/helper.ts'; +import { stringToArray } from '../shared/array.ts'; test('floor server changes', () => { const f = new GumnutFloor(); diff --git a/client/lib/gumnut/floor.ts b/client/lib/gumnut/floor.ts index fda4c90..3a8d3ef 100644 --- a/client/lib/gumnut/floor.ts +++ b/client/lib/gumnut/floor.ts @@ -5,7 +5,7 @@ import { encodePatch, type Patch, type WirePatch, decodePatch } from '../ymodel/ import { InoPosRef, InoPosRefImpl, type GumnutData } from './types.ts'; import type { PatchApi } from '../ymodel/work-holder.ts'; import type { RangeUpdate } from '../ymodel/internal/range.ts'; -import { arrayToString } from '../ymodel/helper.ts'; +import { arrayToString } from '../shared/array.ts'; import { decodeGumnutDataArray, encodeGumnutDataArray } from './wire.ts'; type WireType = { diff --git a/client/lib/gumnut/wire.ts b/client/lib/gumnut/wire.ts index 7c946f5..efd5671 100644 --- a/client/lib/gumnut/wire.ts +++ b/client/lib/gumnut/wire.ts @@ -1,4 +1,4 @@ -import { stringToArray } from '../ymodel/helper.ts'; +import { stringToArray } from '../shared/array.ts'; import type { Id } from '../ymodel/internal/shared.ts'; import { emptySymbol, InoPosRefImpl, type GumnutData } from './types.ts'; diff --git a/client/lib/model/layout.test.ts b/client/lib/model/layout.test.ts deleted file mode 100644 index b3bfa96..0000000 --- a/client/lib/model/layout.test.ts +++ /dev/null @@ -1,21 +0,0 @@ -import test from 'node:test'; -import * as assert from 'node:assert'; -import { RopeLayout } from './layout.ts'; - -test('layout', () => { - const r = RopeLayout.new(); - - const idForHigh = r.posToId(100); - assert.deepStrictEqual(r.idToPos(idForHigh), { pos: 100, gone: false }); - - r.applyOp(r.coerceOp({ r: [0], length: 5 })); - assert.deepStrictEqual(r.idToPos(idForHigh), { pos: 105, gone: false }); - - const deleteOp = r.coerceOp({ r: [40, 105] }); - - r.applyOp(r.coerceOp({ r: [100, 110] })); - assert.deepStrictEqual(r.idToPos(idForHigh), { pos: 100, gone: true }); - - // confirm deletion is now _lower_, because we nuked the target range - assert.deepStrictEqual(r.applyOp(deleteOp), { lo: 40, hi: 100, length: 0 }); -}); diff --git a/client/lib/model/layout.ts b/client/lib/model/layout.ts deleted file mode 100644 index ba92e67..0000000 --- a/client/lib/model/layout.ts +++ /dev/null @@ -1,249 +0,0 @@ -import { AATree, Rope } from 'thorish'; -import type { Op } from '../ymodel/types.ts'; -import type { RangeUpdate } from '../ymodel/internal/range.ts'; - -export type InternalOp = { - r: number[]; - length: number; - id: number; -}; - -export type IdLookup = { - pos: number; - gone: boolean; -}; - -let globalLocalId = 20_000; - -function allocLocalId(length?: number): number { - if (length && length > 0) { - globalLocalId += length; - return globalLocalId; - } - return 0; -} - -/** - * @returns The last allocated ID, for use in tests. - */ -export function lastLocalId(): number { - return globalLocalId; -} - -/** - * Holds a layout with custom IDs. - * This has effectively infinite length and helps track positions, but no actual data. - * - * Unlike the server, this is ID-based so that end-users don't have to "update" their IDs to fetch new locations. - * Over time, the ID space will inevitably become fragmented. - */ -export class RopeLayout { - private constructor( - /** - * Points ID to length. - * However, the actual length inside the {@link Rope} might be zero to indicate deletion. - */ - private readonly rope: Rope = new Rope(0, 0), - - /** - * Tree for ID lookup only. - */ - private readonly tree: AATree = new AATree((a, b) => a - b), - ) {} - - /** - * Create a new instance. - */ - public static new() { - return new RopeLayout(); - } - - /** - * Clones this instance. - */ - public clone(): RopeLayout { - return new RopeLayout(this.rope.clone(), this.tree.clone()); - } - - /** - * Ensures an edge in the underlying {@link Rope} at the given {@link Id}. - * - * Returns whether the target ID exists (true) or is deleted (false). - */ - private ensureIdEdge(id: number): boolean { - if (id === 0) { - return true; - } - const highId = this.tree.equalAfter(id); - if (!highId) { - throw new Error(`can't find ID: ${id}`); - } - - // find host, ensure we're slicing "in the middle" - const host = this.rope.lookup(highId); - const exists = host.length !== 0; - if (highId === id) { - return exists; - } - - // adjust existing to be right side - const right = host.id - id; - this.rope.adjust(host.id, right, exists ? right : 0); - - // insert new left side - const left = host.data - right; - this.rope.insertAfter(host.prevId, id, exists ? left : 0, left); - - // insert new left side ID - this.tree.insert(id); - - return exists; - } - - /** - * Coerce this public {@link Op} at the current state to a repeatable {@link InternalOp}. - */ - coerceOp(op: Op): InternalOp { - return { - r: op.r.map((r) => this.posToId(r)), - length: op.length ?? 0, - id: allocLocalId(op.length), - }; - } - - /** - * Converts a >=0 position to an internal {@link Id}. - * There is no limit on the length here. - */ - public posToId(pos: number): number { - if (pos < 0) { - throw new Error(`can't posToId negative: ${pos}`); - } else if (pos === 0) { - return 0; - } - - const extra = pos - this.rope.length(); - if (extra > 0) { - const lastId = this.rope.last(); - - // TODO: would create a lot of cruft if a user simply calls posToId one-by-one ... - - const id = allocLocalId(extra); - this.rope.insertAfter(lastId, id, extra, extra); - this.tree.insert(id); - return id; - } - - const lookup = this.rope.byPosition(pos); // O(logn-ish) - return lookup.id - lookup.offset; - } - - /** - * Resolves this ID, including its most valid >=0 pos and whether it is gone. - * - * If the ID is not valid here, returns `undefined`. - */ - public idToPos(id: number): IdLookup | undefined { - if (id === 0) { - return { pos: 0, gone: false }; - } - const highId = this.tree.equalAfter(id); - if (!highId) { - return; - } - - // find high pos - const pos = this.rope.find(highId); - const lookup = this.rope.lookup(highId); - - const fromEnd = highId - id; - if (fromEnd > lookup.data) { - return; // out of range - } - - if (lookup.length === 0) { - return { pos, gone: true }; - } - return { pos: pos - fromEnd, gone: false }; - } - - /** - * Applies a previously coerced {@link InternalOp}. - */ - applyOp(int: InternalOp): RangeUpdate | undefined { - switch (int.r.length) { - case 1: - return this.internalInsert(int.r[0], int.id, int.length); - - case 2: - return this.internalDelete(int.r[0], int.r[1]); - } - - // TODO: support flip - throw new Error(`unsupported op: ${int.r.length}`); - } - - private internalInsert(at: number, newId: number, length: number): RangeUpdate | undefined { - if (length <= 0) { - return; - } - - // remember: the rope doesn't really care that we think long entries take up "rope space" - const prevHostId = this.tree.equalAfter(newId); - if (prevHostId !== undefined) { - const offset = prevHostId - newId; - const host = this.rope.lookup(prevHostId); - if (host.length > offset) { - throw new Error(`can't insert over prior id: insert=${newId} prev=${prevHostId}`); - } - } - - const exists = this.ensureIdEdge(at); - - // we always insert (even into deleted space) - this.rope.insertAfter(at, newId, exists ? length : 0, length); - this.tree.insert(newId); - - if (exists) { - const pos = this.rope.find(at); - return { lo: pos, hi: pos, length }; - } - } - - private internalDelete(lo: number, hi: number): RangeUpdate | undefined { - if (lo === hi) { - return; - } - - this.ensureIdEdge(lo); - this.ensureIdEdge(hi); - - if (!this.rope.before(lo, hi)) { - [lo, hi] = [hi, lo]; - } - - // walk and delete undeleted bits - let cleared = 0; - for (const prev of this.rope.iter(lo, hi)) { - if (prev.length) { - this.rope.adjust(prev.id, prev.data, 0); - cleared += prev.length; - } - } - - if (cleared !== 0) { - const pos = this.rope.find(lo); - return { lo: pos, hi: pos + cleared, length: 0 }; - } - } -} - -/** - * Generates the inverse operation required to remove this {@link InternalOp}. - * Just returns a delete for an insert. - */ -export function rollbackInternalOp(int: InternalOp): InternalOp | undefined { - if (int.r.length === 1) { - return { r: [int.r[0], int.id], length: 0, id: 0 }; - } -} diff --git a/client/lib/shared/array.test.ts b/client/lib/shared/array.test.ts new file mode 100644 index 0000000..94b070f --- /dev/null +++ b/client/lib/shared/array.test.ts @@ -0,0 +1,61 @@ +import test from 'node:test'; +import * as assert from 'node:assert'; +import { arrayToString, holeAtIndex } from './array.ts'; + +test('hole', () => { + const x = new Array(5); + + x[0] = undefined; + + assert.ok(holeAtIndex(x, 1)); + assert.ok(!holeAtIndex(x, 0)); + + assert.ok(holeAtIndex(x, 10000)); + + assert.ok(holeAtIndex(x, 0.1)); + assert.ok(holeAtIndex(x, -1)); + assert.ok(holeAtIndex(x, NaN)); + assert.ok(holeAtIndex(x, Infinity)); + assert.ok(holeAtIndex(x, -Infinity)); +}); + +test('arrayToString', () => { + assert.strictEqual(arrayToString(['Z']), '\0'); + assert.strictEqual(arrayToString([true, false, Infinity]), '\x01\0\0'); + assert.strictEqual(arrayToString([32.5, NaN]), ' \0'); + assert.strictEqual(arrayToString([null]), '\0'); + + // check toPrimitive + class Foo { + [Symbol.toPrimitive]() { + return 123; // '{' + } + } + assert.strictEqual(arrayToString([new Foo()]), '{'); + + // check boring class + class Bar {} + assert.strictEqual(arrayToString([new Bar()]), '\0'); + + // check bigint throws + assert.throws(() => { + arrayToString([1234n]); + }); + + // check symbol throws + assert.throws(() => { + arrayToString([Symbol('blah')]); + }); + + // check symbol throws + assert.throws(() => { + arrayToString([Symbol.for('blah')]); + }); + + // this is `number & 0xffff` semantics in JS - clamp to uint16 + const s = arrayToString([-100_000_000]); + assert.strictEqual(s.length, 1); + assert.strictEqual(s, String.fromCodePoint(7936)); + assert.strictEqual(arrayToString([-100_000_000.99]), s); + assert.strictEqual(arrayToString([-100_000_000.00001]), s); +}); diff --git a/client/lib/ymodel/helper.ts b/client/lib/shared/array.ts similarity index 55% rename from client/lib/ymodel/helper.ts rename to client/lib/shared/array.ts index 620dd27..c0842d6 100644 --- a/client/lib/ymodel/helper.ts +++ b/client/lib/shared/array.ts @@ -15,6 +15,9 @@ export function convertToEmpty(arr: any[]): any[] { return out; } +/** + * Convert this regular JS string to a {@link Array} of uint16's. + */ export function stringToArray(str: string): number[] { const out = new Uint16Array(str.length); for (let i = 0; i < str.length; ++i) { @@ -23,7 +26,24 @@ export function stringToArray(str: string): number[] { return Array.from(out); } +/** + * Convert this {@link Array} to a JS string. + * + * This allows any data here, but note that certain values are seen as "1": `true`, but _not_ an `object`. + */ export function arrayToString(arr: any[]): string { const data = new Uint16Array(arr); return new TextDecoder('utf-16le').decode(data); } + +/** + * Does the given array have a hole at the specified index? + * + * This will also be true for indicies out of range. + */ +export function holeAtIndex(arr: any[], index: number): boolean { + if (arr[index] !== undefined) { + return false; + } + return !(index in arr); +} diff --git a/client/lib/ymodel/api.test.ts b/client/lib/ymodel/api.test.ts index 9ebb6ed..ab0c314 100644 --- a/client/lib/ymodel/api.test.ts +++ b/client/lib/ymodel/api.test.ts @@ -3,7 +3,7 @@ import * as assert from 'node:assert'; import { GumnutLow } from './api.ts'; import type { Id } from './internal/shared.ts'; import type { Patch } from './types.ts'; -import { convertToEmpty } from './helper.ts'; +import { convertToEmpty } from '../shared/array.ts'; type TestWireType = { barrier?: string[]; update: Record> }; diff --git a/client/lib/ymodel/internal/state.test.ts b/client/lib/ymodel/internal/state.test.ts index 5519624..07db0f9 100644 --- a/client/lib/ymodel/internal/state.test.ts +++ b/client/lib/ymodel/internal/state.test.ts @@ -50,4 +50,9 @@ test('length complex semantics', () => { // insert at end should increase length r.applyOp(r.coerceOp({ r: [5], length: 4 })); assert.strictEqual(r.length(), 9, 'insert at end should increase length'); + + // construct holy array and confirm correct + const arr = new Array(9); + arr[4] = 42; + assert.deepStrictEqual(r.read(), arr); }); diff --git a/client/lib/ymodel/work-holder.test.ts b/client/lib/ymodel/work-holder.test.ts index f628528..d7c543d 100644 --- a/client/lib/ymodel/work-holder.test.ts +++ b/client/lib/ymodel/work-holder.test.ts @@ -2,7 +2,7 @@ import { test } from 'node:test'; import * as assert from 'node:assert'; import { WorkHolder } from './work-holder.ts'; import { RopeState } from './internal/state.ts'; -import { convertToEmpty } from './helper.ts'; +import { convertToEmpty } from '../shared/array.ts'; test('number ranges', () => { let actual = RopeState.new(); From 35981131d963986093f102fcf44f2ba07bdcc200 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Sun, 5 Apr 2026 15:43:56 +1000 Subject: [PATCH 5/5] invert hole behavior --- client/lib/shared/array.test.ts | 18 +++++++++--------- client/lib/shared/array.ts | 14 +++++++------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/client/lib/shared/array.test.ts b/client/lib/shared/array.test.ts index 94b070f..b3376f6 100644 --- a/client/lib/shared/array.test.ts +++ b/client/lib/shared/array.test.ts @@ -1,22 +1,22 @@ import test from 'node:test'; import * as assert from 'node:assert'; -import { arrayToString, holeAtIndex } from './array.ts'; +import { arrayToString, hasIndex } from './array.ts'; test('hole', () => { const x = new Array(5); x[0] = undefined; - assert.ok(holeAtIndex(x, 1)); - assert.ok(!holeAtIndex(x, 0)); + assert.ok(!hasIndex(x, 1)); + assert.ok(hasIndex(x, 0)); - assert.ok(holeAtIndex(x, 10000)); + assert.ok(!hasIndex(x, 10000)); - assert.ok(holeAtIndex(x, 0.1)); - assert.ok(holeAtIndex(x, -1)); - assert.ok(holeAtIndex(x, NaN)); - assert.ok(holeAtIndex(x, Infinity)); - assert.ok(holeAtIndex(x, -Infinity)); + assert.ok(!hasIndex(x, 0.1)); + assert.ok(!hasIndex(x, -1)); + assert.ok(!hasIndex(x, NaN)); + assert.ok(!hasIndex(x, Infinity)); + assert.ok(!hasIndex(x, -Infinity)); }); test('arrayToString', () => { diff --git a/client/lib/shared/array.ts b/client/lib/shared/array.ts index c0842d6..0c85b10 100644 --- a/client/lib/shared/array.ts +++ b/client/lib/shared/array.ts @@ -1,7 +1,7 @@ /** - * Converts {@link undefined} to a "hole" in an array. + * Converts an array to a new array, with {@link undefined} entries replaced with holes. * - * This is mostly for tests. + * This is useful for testing. */ export function convertToEmpty(arr: any[]): any[] { const out = new Array(arr.length); @@ -37,13 +37,13 @@ export function arrayToString(arr: any[]): string { } /** - * Does the given array have a hole at the specified index? + * Does the given array have data at the specified index? * - * This will also be true for indicies out of range. + * This will also be false for indicies out of range. */ -export function holeAtIndex(arr: any[], index: number): boolean { +export function hasIndex(arr: any[], index: number): boolean { if (arr[index] !== undefined) { - return false; + return true; } - return !(index in arr); + return index in arr; }