diff --git a/src/brainy.ts b/src/brainy.ts index d54cfe1c..422cb2e6 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -3024,7 +3024,7 @@ export class Brainy implements BrainyInterface { // rollback can re-add through the BigInt addVerb contract) const { sourceInt, targetInt } = this.resolveVerbEndpointInts(verb) tx.addOperation( - new RemoveFromGraphIndexOperation(this.graphIndex, verb, sourceInt, targetInt, this.graphWriteGeneration) + new RemoveFromGraphIndexOperation(this.graphIndex, verb, { sourceInt, targetInt }, this.graphWriteGeneration) ) // Delete verb metadata tx.addOperation( @@ -3734,7 +3734,7 @@ export class Brainy implements BrainyInterface { // Operation 3: Add to graph index for O(1) lookups tx.addOperation( new AddToGraphIndexOperation( - this.graphIndex, verb, sourceInt, targetInt, + this.graphIndex, verb, { sourceInt, targetInt }, this.graphWriteGeneration, (verbInt) => this.cacheVerbInt(verbInt, id) ) @@ -3772,7 +3772,7 @@ export class Brainy implements BrainyInterface { // Operation 6: Add reverse relationship to graph index tx.addOperation( new AddToGraphIndexOperation( - this.graphIndex, reverseVerb, targetInt, sourceInt, + this.graphIndex, reverseVerb, { sourceInt: targetInt, targetInt: sourceInt }, this.graphWriteGeneration, (verbInt) => this.cacheVerbInt(verbInt, reverseId) ) @@ -3854,7 +3854,7 @@ export class Brainy implements BrainyInterface { if (verb && endpointInts) { tx.addOperation( new RemoveFromGraphIndexOperation( - this.graphIndex, verb, endpointInts.sourceInt, endpointInts.targetInt, this.graphWriteGeneration + this.graphIndex, verb, endpointInts, this.graphWriteGeneration ) ) } @@ -4006,12 +4006,12 @@ export class Brainy implements BrainyInterface { if (typeChanged && reindexInts) { tx.addOperation( new RemoveFromGraphIndexOperation( - this.graphIndex, existing, reindexInts.sourceInt, reindexInts.targetInt, this.graphWriteGeneration + this.graphIndex, existing, reindexInts, this.graphWriteGeneration ) ) tx.addOperation( new AddToGraphIndexOperation( - this.graphIndex, verbForIndex, reindexInts.sourceInt, reindexInts.targetInt, + this.graphIndex, verbForIndex, reindexInts, this.graphWriteGeneration, (verbInt) => this.cacheVerbInt(verbInt, params.id) ) @@ -6634,7 +6634,7 @@ export class Brainy implements BrainyInterface { for (const verb of allVerbs) { const { sourceInt, targetInt } = this.resolveVerbEndpointInts(verb) tx.addOperation( - new RemoveFromGraphIndexOperation(this.graphIndex, verb, sourceInt, targetInt, this.graphWriteGeneration) + new RemoveFromGraphIndexOperation(this.graphIndex, verb, { sourceInt, targetInt }, this.graphWriteGeneration) ) tx.addOperation( new DeleteVerbMetadataOperation(this.storage, verb.id) @@ -9014,9 +9014,12 @@ export class Brainy implements BrainyInterface { } plan.operations.push(new DeleteNounMetadataOperation(this.storage, id)) for (const verb of cascade.values()) { - const { sourceInt, targetInt } = this.resolveVerbEndpointInts(verb) plan.operations.push( - new RemoveFromGraphIndexOperation(this.graphIndex, verb, sourceInt, targetInt, this.graphWriteGeneration), + // Endpoint ints resolve at EXECUTE time — a cascade verb (or its + // endpoints) may have been created earlier in this same batch, so a + // plan-time resolution would ask the id mapper about entities that do + // not exist yet (the native mapper rightly refuses). + new RemoveFromGraphIndexOperation(this.graphIndex, verb, () => this.resolveVerbEndpointInts(verb), this.graphWriteGeneration), new DeleteVerbMetadataOperation(this.storage, verb.id) ) plan.touchedVerbs.push(verb.id) @@ -9163,8 +9166,6 @@ export class Brainy implements BrainyInterface { data: params.data, createdAt: now } - const { sourceInt, targetInt } = this.resolveVerbEndpointInts(verb) - plan.operations.push( new SaveVerbOperation(this.storage, { id, @@ -9175,7 +9176,11 @@ export class Brainy implements BrainyInterface { targetId: params.to }), new SaveVerbMetadataOperation(this.storage, id, verbMetadata), - new AddToGraphIndexOperation(this.graphIndex, verb, sourceInt, targetInt, this.graphWriteGeneration, (verbInt) => + // Endpoint ints resolve at EXECUTE time — after any same-batch add of + // an endpoint has applied. Plan-time resolution was a consumer-reported + // native/JS parity bug: transact([add X, relate →X]) asked the native + // id mapper to assign an int for an entity that did not exist yet. + new AddToGraphIndexOperation(this.graphIndex, verb, () => this.resolveVerbEndpointInts(verb), this.graphWriteGeneration, (verbInt) => this.cacheVerbInt(verbInt, id) ) ) @@ -9203,9 +9208,9 @@ export class Brainy implements BrainyInterface { ...verb, id: reverseId, sourceId: params.to, - targetId: params.from, - sourceInt: targetInt, - targetInt: sourceInt + targetId: params.from + // sourceInt/targetInt are mirrored onto this object when the graph + // operation resolves endpoints at execute time. } plan.operations.push( new SaveVerbOperation(this.storage, { @@ -9217,7 +9222,7 @@ export class Brainy implements BrainyInterface { targetId: params.from }), new SaveVerbMetadataOperation(this.storage, reverseId, verbMetadata), - new AddToGraphIndexOperation(this.graphIndex, reverseVerb, targetInt, sourceInt, this.graphWriteGeneration, (verbInt) => + new AddToGraphIndexOperation(this.graphIndex, reverseVerb, () => this.resolveVerbEndpointInts(reverseVerb), this.graphWriteGeneration, (verbInt) => this.cacheVerbInt(verbInt, reverseId) ) ) @@ -9259,9 +9264,10 @@ export class Brainy implements BrainyInterface { : (state.verbs.get(id) ?? (await this.storage.getVerb(id))) if (verb) { - const { sourceInt, targetInt } = this.resolveVerbEndpointInts(verb) plan.operations.push( - new RemoveFromGraphIndexOperation(this.graphIndex, verb, sourceInt, targetInt, this.graphWriteGeneration) + // Endpoint ints resolve at EXECUTE time — the verb (or its endpoints) + // may have been created earlier in this same batch (forward refs). + new RemoveFromGraphIndexOperation(this.graphIndex, verb, () => this.resolveVerbEndpointInts(verb), this.graphWriteGeneration) ) } plan.operations.push(new DeleteVerbMetadataOperation(this.storage, id)) diff --git a/src/transaction/operations/IndexOperations.ts b/src/transaction/operations/IndexOperations.ts index f0bede05..97c39270 100644 --- a/src/transaction/operations/IndexOperations.ts +++ b/src/transaction/operations/IndexOperations.ts @@ -176,14 +176,36 @@ export class RemoveFromMetadataIndexOperation implements Operation { * generation is reused for the rollback removal, so an add and its undo * reference one watermark in a provider's per-generation edge chain. */ +/** + * A verb's interned endpoint ints — eager (already resolved), or a thunk + * evaluated when the operation EXECUTES. The lazy form exists for + * `transact()` forward references: a relate whose endpoint is added in the + * SAME batch cannot resolve ints at plan time (the entity does not exist yet + * — a strict native id mapper rightly refuses to assign, and even a permissive + * one would leak the assignment if the batch is rejected at precommit). + * Deferring to execute time resolves after the batch's add operations have + * applied, mirroring how `generationFn` is already evaluated lazily. + */ +export type VerbEndpointInts = + | { sourceInt: bigint; targetInt: bigint } + | (() => { sourceInt: bigint; targetInt: bigint }) + +/** Resolve a {@link VerbEndpointInts} at execution time. */ +function resolveEndpointInts( + endpoints: VerbEndpointInts +): { sourceInt: bigint; targetInt: bigint } { + return typeof endpoints === 'function' ? endpoints() : endpoints +} + export class AddToGraphIndexOperation implements Operation { readonly name = 'AddToGraphIndex' /** * @param index - The graph-index provider (JS baseline or native). * @param verb - The verb to index (`sourceInt`/`targetInt` mirrored on it). - * @param sourceInt - The source entity's interned int. - * @param targetInt - The target entity's interned int. + * @param endpointInts - The endpoints' interned ints — eager, or a thunk + * evaluated at execute time (REQUIRED for transact forward references; + * see {@link VerbEndpointInts}). * @param generationFn - Resolves the commit generation to stamp this edge at, * evaluated when the operation executes (see class note). * @param onVerbInt - Optional hook invoked with the interned verb int @@ -192,17 +214,18 @@ export class AddToGraphIndexOperation implements Operation { constructor( private readonly index: GraphIndexProvider, private readonly verb: GraphVerb, - private readonly sourceInt: bigint, - private readonly targetInt: bigint, + private readonly endpointInts: VerbEndpointInts, private readonly generationFn: () => bigint, private readonly onVerbInt?: (verbInt: bigint) => void ) {} async execute(): Promise { // Stamp this edge at the in-flight commit generation; reuse it for the - // rollback so add + undo reference the same watermark. + // rollback so add + undo reference the same watermark. Endpoint ints + // resolve HERE — after any same-batch adds have applied. const generation = this.generationFn() - const verbInt = await this.index.addVerb(this.verb, this.sourceInt, this.targetInt, generation) + const { sourceInt, targetInt } = resolveEndpointInts(this.endpointInts) + const verbInt = await this.index.addVerb(this.verb, sourceInt, targetInt, generation) this.onVerbInt?.(verbInt) // Return rollback action @@ -231,28 +254,32 @@ export class RemoveFromGraphIndexOperation implements Operation { /** * @param index - The graph-index provider (JS baseline or native). * @param verb - The verb being removed (required for rollback re-add). - * @param sourceInt - The source entity's interned int (rollback re-add). - * @param targetInt - The target entity's interned int (rollback re-add). + * @param endpointInts - The endpoints' interned ints for the rollback + * re-add — eager, or a thunk evaluated at execute time (required when the + * verb or its endpoints were created in the SAME transact batch; see + * {@link VerbEndpointInts}). * @param generationFn - Resolves the commit generation for this removal, * evaluated when the operation executes. */ constructor( private readonly index: GraphIndexProvider, private readonly verb: GraphVerb, // Required for rollback - private readonly sourceInt: bigint, - private readonly targetInt: bigint, + private readonly endpointInts: VerbEndpointInts, private readonly generationFn: () => bigint ) {} async execute(): Promise { // Resolve the removal generation once; reuse it for the rollback re-add. + // Endpoint ints resolve HERE (after any same-batch adds applied) and are + // captured for the rollback, whose re-add must use the same mappings. const generation = this.generationFn() + const { sourceInt, targetInt } = resolveEndpointInts(this.endpointInts) await this.index.removeVerb(this.verb.id, generation) // Return rollback action return async () => { // Re-add verb with original data - await this.index.addVerb(this.verb, this.sourceInt, this.targetInt, generation) + await this.index.addVerb(this.verb, sourceInt, targetInt, generation) } } } diff --git a/tests/integration/transact-forward-ref-graph.test.ts b/tests/integration/transact-forward-ref-graph.test.ts new file mode 100644 index 00000000..a9619424 --- /dev/null +++ b/tests/integration/transact-forward-ref-graph.test.ts @@ -0,0 +1,140 @@ +/** + * @module tests/integration/transact-forward-ref-graph + * @description Regression for a consumer-reported native/JS parity bug: + * `transact([{op:'add', id:X}, {op:'relate', to:X}])` — the platform's first + * atomic add+relate consumer — threw in `EntityIdMapper.getOrAssign` at PLAN + * time on the native id mapper (an entity added in the same batch does not + * exist when the planner runs, so a strict mapper rightly refuses to assign; + * the permissive JS mapper masked the bug — and silently leaked an int + * assignment whenever the batch was later rejected at precommit). + * + * Fix: graph-index operations resolve endpoint ints at EXECUTE time (a lazy + * thunk, mirroring the existing lazy `generationFn`), after the batch's add + * operations have applied. These tests pin every forward-ref shape on the JS + * path, including the JS-observable deferral proof: a REJECTED batch leaves + * the id mapper untouched (pre-fix, plan-time resolution assigned ints for + * entities that never came to exist). The native side is locked in by the + * accelerator's own gate running this same shape. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import * as fs from 'node:fs' +import * as os from 'node:os' +import * as path from 'node:path' +import { Brainy } from '../../src/brainy.js' +import { NounType, VerbType } from '../../src/types/graphTypes.js' + +let seq = 0 +const freshId = (): string => + `00000000-0000-4000-8000-${(++seq).toString(16).padStart(12, '0')}` + +describe('transact() forward references into the graph index (native/JS parity)', () => { + let dir: string + let brain: any + + beforeEach(async () => { + process.env.BRAINY_DETERMINISTIC_EMBEDDINGS = 'true' + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'brainy-fwd-ref-')) + brain = new Brainy({ + requireSubtype: false, + storage: { type: 'filesystem', path: dir }, + dimensions: 384, + silent: true + }) + await brain.init() + }) + + afterEach(async () => { + await brain.close() + fs.rmSync(dir, { recursive: true, force: true }) + }) + + it('CASE 1 (the exact repro): add + relate-to-that-add in ONE transact', async () => { + const agent = await brain.add({ id: freshId(), data: 'agent probe', type: NounType.Person }) + const thread = freshId() + + const db = await brain.transact([ + { op: 'add', id: thread, data: 'thread PROBE-ONE', type: NounType.Thing }, + { op: 'relate', from: agent, to: thread, type: VerbType.RelatedTo } + ]) + + // The batch committed as one generation and the edge traverses. + expect(db.generation).toBeGreaterThan(0) + const related = await brain.related(agent) + expect(related.map((r: any) => r.to)).toContain(thread) + }) + + it('both endpoints created in-batch: add A + add B + relate A→B', async () => { + const a = freshId() + const b = freshId() + await brain.transact([ + { op: 'add', id: a, data: 'node A', type: NounType.Thing }, + { op: 'add', id: b, data: 'node B', type: NounType.Thing }, + { op: 'relate', from: a, to: b, type: VerbType.Contains } + ]) + expect((await brain.related(a)).map((r: any) => r.to)).toContain(b) + }) + + it('bidirectional relate to an in-batch add: both edges traverse', async () => { + const hub = await brain.add({ id: freshId(), data: 'hub', type: NounType.Thing }) + const spoke = freshId() + await brain.transact([ + { op: 'add', id: spoke, data: 'spoke', type: NounType.Thing }, + { op: 'relate', from: hub, to: spoke, type: VerbType.RelatedTo, bidirectional: true } + ]) + expect((await brain.related(hub)).map((r: any) => r.to)).toContain(spoke) + expect((await brain.related(spoke)).map((r: any) => r.to)).toContain(hub) + }) + + it('add + relate + remove in ONE transact: the cascade covers the in-batch verb', async () => { + const keeper = await brain.add({ id: freshId(), data: 'keeper', type: NounType.Thing }) + const doomed = freshId() + await brain.transact([ + { op: 'add', id: doomed, data: 'doomed', type: NounType.Thing }, + { op: 'relate', from: keeper, to: doomed, type: VerbType.Contains }, + { op: 'remove', id: doomed } + ]) + expect(await brain.get(doomed)).toBeNull() + expect((await brain.related(keeper)).length).toBe(0) + }) + + it('CASE 2 control: the same two ops split across two transacts still work', async () => { + const agent = await brain.add({ id: freshId(), data: 'agent two', type: NounType.Person }) + const thread = freshId() + await brain.transact([{ op: 'add', id: thread, data: 'thread two', type: NounType.Thing }]) + await brain.transact([{ op: 'relate', from: agent, to: thread, type: VerbType.RelatedTo }]) + expect((await brain.related(agent)).map((r: any) => r.to)).toContain(thread) + }) + + it('a REJECTED forward-ref batch leaves the id mapper untouched (the deferral proof)', async () => { + const existing = await brain.add({ + id: freshId(), + data: 'cas anchor', + type: NounType.Thing + }) + const never = freshId() + + // Reject the batch via a stale per-op CAS — planning completes, the + // commit precondition throws, nothing applies. + await expect( + brain.transact([ + { op: 'add', id: never, data: 'never exists', type: NounType.Thing }, + { op: 'relate', from: existing, to: never, type: VerbType.RelatedTo }, + { op: 'update', id: existing, metadata: { poke: 1 }, ifRev: 999 } + ]) + ).rejects.toMatchObject({ name: 'RevisionConflictError' }) + + // Nothing applied… + expect(await brain.get(never)).toBeNull() + // …and the id mapper was never asked to assign for the phantom entity — + // pre-fix, plan-time resolution leaked an int here on every rejected batch. + const idMapper = brain['metadataIndex'].getIdMapper() + expect(idMapper.getInt(never)).toBeUndefined() + + // The same shape then succeeds cleanly on retry. + await brain.transact([ + { op: 'add', id: never, data: 'exists now', type: NounType.Thing }, + { op: 'relate', from: existing, to: never, type: VerbType.RelatedTo } + ]) + expect((await brain.related(existing)).map((r: any) => r.to)).toContain(never) + }) +})