diff --git a/src/core/relation/RelationUpdates.js b/src/core/relation/RelationUpdates.js index 92c2ba7..277b6ea 100644 --- a/src/core/relation/RelationUpdates.js +++ b/src/core/relation/RelationUpdates.js @@ -382,33 +382,63 @@ export class RelationUpdates { if (this.manager.arbiter._snapshotReadOnly) { throw new Error('Cannot update relations in batch while in snapshot read-only mode. Disable snapshot before writing.'); } - const addOps = []; - const removeOps = []; - const modifyOps = []; - - // Pre-sort operations for better cache locality - var len = updates.length; - for (var i = 0; i < len; i++) { - const update = updates[i]; - switch (update.operation) { - case 'add': - addOps.push(update); - break; - case 'remove': - removeOps.push(update); - break; - case 'modify': - modifyOps.push(update); - break; - default: - throw new Error(`Unknown operation: ${update.operation}`); + + // Validate every operation up front so a bad op cannot partially apply. + for (const op of updates) { + if (!op || (op.operation !== 'add' && op.operation !== 'remove' && op.operation !== 'modify')) { + throw new Error(`Unknown operation: ${op.operation}`); } } - - // Process in optimal order: removes first, then modifications, then adds - this._processBatchRemovals(removeOps, chunkSize); - this._processBatchModifications(modifyOps, chunkSize); - this._processBatchAdditions(addOps, chunkSize); + + // Apply operations strictly in the given order. The previous + // remove->modify->add pre-sort changed the final state whenever one + // tuple was touched by mixed kinds: [add, remove] left the tuple + // present, and [modify, add, modify] ended with the middle value + // instead of the last one (last-write-wins). + const arbiter = this.manager.arbiter; + arbiter.batchUpdateInProgress = true; + try { + for (let i = 0; i < updates.length; i += chunkSize) { + const chunk = updates.slice(i, i + chunkSize); + for (const op of chunk) { + switch (op.operation) { + case 'add': + this._addRelationInternal(op.srcKey, op.relation, op.dstKey, op.options); + break; + case 'remove': + this.manager.removeRelation(op.srcKey, op.relation, op.dstKey); + break; + case 'modify': + this._modifyRelation(op.srcKey, op.relation, op.dstKey, op.options); + break; + } + } + } + } finally { + arbiter.batchUpdateInProgress = false; + } + + // After the batch completes, update PLTC indices for all touched edges + // (adds and removes both affect reachability). + const reachabilityChecker = arbiter.reachabilityChecker; + if (reachabilityChecker && + (reachabilityChecker.pltcIndex?.initialized || + reachabilityChecker.pltcBackwardIndex?.initialized)) { + const edgesToAdd = new Set(); + for (const op of updates) { + const srcId = arbiter.nodeIdByKey.get(op.srcKey); + const dstId = arbiter.nodeIdByKey.get(op.dstKey); + if (srcId !== undefined && dstId !== undefined) { + edgesToAdd.add(`${srcId}:${dstId}`); + } + } + reachabilityChecker.beginUpdateBatch(); + for (const edgeKey of edgesToAdd) { + const [srcId, dstId] = edgeKey.split(':').map(Number); + this.manager._updatePLTCIndicesOnAdd(srcId, dstId, null); + } + reachabilityChecker.commitUpdateBatch(); + } // The batch path bypasses Arbiter.addRelation/removeRelation, so the // arbiter-level rule caches (rule result cache, ChainRule caches, diff --git a/tests/rigor/batch-order-parity.test.js b/tests/rigor/batch-order-parity.test.js new file mode 100644 index 0000000..7c63ffc --- /dev/null +++ b/tests/rigor/batch-order-parity.test.js @@ -0,0 +1,155 @@ +/** + * rigor/batch-order-parity.test.js — batch update ordering semantics. + * + * updateRelationsBatch applies operations in the GIVEN order + * (last-write-wins per tuple). Contracts pinned: + * - [add v1, remove] ends absent; [remove, add v1] ends v1. + * - [modify, add, modify] ends with the LAST modify's value. + * - modify of a tuple that does not exist (yet) is a silent no-op. + * - decisions after a mixed batch equal the in-order mirror. + * + * The mirror is a plain last-write-wins map with the modify-no-op rule; + * the engine must agree after every mixed batch. + */ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { rigor } from '@rigor/core'; +import { Arbiter } from '../../src/index.js'; + +const KEY = 'u:0|owner|doc:0'; +const VALUES = [0.2, 0.5, 0.8, 0.9]; + +function applyMirror(tuples, ops) { + for (const op of ops) { + if (op.operation === 'remove') { + tuples.delete(KEY); + } else if (op.operation === 'modify' && !tuples.has(KEY)) { + continue; // engine no-op on missing tuple + } else { + tuples.set(KEY, op.value); + } + } +} + +function makeWrapper() { + const arbiter = new Arbiter(); + arbiter.addNode('u:0', 'user'); + arbiter.addNode('doc:0', 'doc'); + arbiter.setRelationConfig('can_read', { type: 'direct', relation: 'owner' }); + const tuples = new Map(); + const ops = []; + + const wrapper = { + engine: arbiter, + tuples, + add(value) { + arbiter.addRelation('u:0', 'owner', 'doc:0', { possibility: value }); + tuples.set(KEY, value); + return { ok: true }; + }, + remove() { + arbiter.removeRelation('u:0', 'owner', 'doc:0'); + tuples.delete(KEY); + return { ok: true }; + }, + mixedBatch(batchOps) { + const batch = batchOps.map((op) => ({ + operation: op.operation, + srcKey: 'u:0', + relation: 'owner', + dstKey: 'doc:0', + options: op.operation === 'remove' ? {} : { possibility: op.value } + })); + arbiter.relationManager.updateRelationsBatch(batch); + applyMirror(tuples, batchOps); + return { ok: true, count: batchOps.length }; + }, + check() { + const result = arbiter.check('u:0', 'can_read', 'doc:0'); + const expected = tuples.get(KEY) ?? 0; + return { engine: result.possibility, expected, reason: result.reason }; + }, + clone() { + const fresh = makeWrapper(); + for (const op of ops) { + const [name, ...args] = op; + fresh[name](...args); + } + return fresh; + } + }; + + const record = (name, fn) => (...args) => { + const res = fn(...args); + ops.push([name, ...args]); + return res; + }; + wrapper.add = record('add', wrapper.add); + wrapper.remove = record('remove', wrapper.remove); + wrapper.mixedBatch = record('mixedBatch', wrapper.mixedBatch); + wrapper.check = record('check', wrapper.check); + return wrapper; +} + +const opGen = rigor.gen.record({ + operation: rigor.gen.enum(['add', 'modify', 'remove']), + value: rigor.gen.oneOf(VALUES) +}); + +describe('Batch update ordering semantics (rigor)', () => { + it('FIXED MATRIX: mixed-kind batches preserve last-write-wins', () => { + const w = makeWrapper(); + w.add(0.9); + w.mixedBatch([ + { operation: 'add', value: 0.5 }, + { operation: 'remove', value: 0.5 } + ]); + assert.equal(w.check().engine, 0, '[add, remove] ends absent'); + + w.mixedBatch([ + { operation: 'remove', value: 0.5 }, + { operation: 'add', value: 0.5 } + ]); + assert.equal(w.check().engine, 0.5, '[remove, add] ends 0.5'); + + w.mixedBatch([ + { operation: 'modify', value: 0.6 }, + { operation: 'add', value: 0.8 }, + { operation: 'modify', value: 0.9 } + ]); + assert.equal(w.check().engine, 0.9, '[modify, add, modify] ends with last value'); + + w.mixedBatch([ + { operation: 'remove', value: 0.9 }, + { operation: 'modify', value: 0.7 } + ]); + assert.equal(w.check().engine, 0, 'modify after remove (missing tuple) is a no-op'); + }); + + it('PROPERTY CAMPAIGN: engine agrees with the in-order mirror after every mixed batch', async () => { + const result = await rigor.campaign( + [rigor.object('graph', makeWrapper, [ + rigor.method('add', function (v) { return this.add(v); }, rigor.args(rigor.gen.oneOf(VALUES))), + rigor.method('remove', function () { return this.remove(); }), + rigor.method('mixedBatch', function (batchOps) { return this.mixedBatch(batchOps); }, + rigor.args(rigor.gen.array(opGen, 2, 4))), + rigor.method('check', function () { return this.check(); }) + ])], + rigor.crucible([ + rigor.invariant('decision parity after every step', (ctx) => { + if (ctx.action !== 'graph.check' || ctx.error !== null) return true; + return ctx.actual.engine === ctx.actual.expected; + }), + rigor.invariant('no action errors', (ctx) => ctx.error === null) + ]) + ).run({ effort: 400, seed: 'batch-order-parity', maxTraceLength: 25 }); + + const inv = result.crucibleVerdict; + assert.equal(inv.passed, true, [ + `batch ordering violated in ${inv.failureCount} cases:`, + ...result.failures.slice(0, 3).map((f) => + ` [${f.invariant}] action=${f.action} args=${JSON.stringify(f.args)} actual=${JSON.stringify(f.actual)} error=${f.error}` + ) + ].join('\n')); + }); +});