js-rigor: fix batch operation ordering (last-write-wins), batch-order campaign
updateRelationsBatch previously pre-sorted ops remove->modify->add, which changed the final state whenever one tuple was touched by mixed kinds: [add, remove] left the tuple present, [modify, add, modify] ended with the middle value. Now ops apply strictly in the given order via the dedup-aware _addRelationInternal (in-place last-write-wins) with upfront validation, post-batch PLTC edge updates, and per-relation arbiter-level cache invalidation. batch-order-parity.test.js pins the contract with an in-order mirror (modify-of-missing is a silent no-op).
This commit is contained in:
John Dvorak
@@ -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,
|
||||
|
||||
@@ -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'));
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user