From 50117671ee0595b463529e01c92143dcd011c1b5 Mon Sep 17 00:00:00 2001 From: Saty <5752427+SatyPardus@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:09:18 +0100 Subject: [PATCH] Peer review: clear majority, a cap on reviewers, shared bodies skip review "same" now needs 70% of the weight as well as a lead of three (7:3, not 10:7), so a contested function is not worn down until "same" happens to lead. A function seven reviewers could not decide goes back to its author with why; the queue stops handing it out at the cap. Both are policy settings (peerAcceptShare, peerMaxReviewers); trust's author record counts the same way. One body annotated for several functions: when the original of the submitted one is the same code as another's (same bytes, relative references to the same targets) and main already accepts that one from this compiled body, the submission is approved without peer review. Co-Authored-By: Claude Opus 5.5 --- src/server/guidance.js | 2 +- src/server/merge.js | 2 +- src/server/peer.js | 52 +++++++++++++++++++-------- src/server/trust.js | 12 ++++--- src/server/verify.js | 36 +++++++++++++++++-- src/shared/compare.js | 23 ++++++++++++ test/compare.test.js | 22 +++++++++++- test/server.test.js | 82 ++++++++++++++++++++++++++++++++---------- 8 files changed, 189 insertions(+), 42 deletions(-) diff --git a/src/server/guidance.js b/src/server/guidance.js index 16cd30d..48199a1 100644 --- a/src/server/guidance.js +++ b/src/server/guidance.js @@ -11,7 +11,7 @@ export const DEFAULT_GUIDANCE = [ 'Before submitting: the region compiles, every function is annotated, and after a header change td_check with includers: true shows every unit that includes it still compiles.', 'Port, do not polish. Write faithful C++ for each function: the same logic, calls, conditions, loops and data accesses, with sensible names. Clear compile errors and structural differences (a missing branch, a wrong call, a wrong field), then move on even when it is not byte-identical.', 'Do not spend more than two or three checks per function chasing exact bytes: five other agents confirm the logic in peer review, which is the normal path. Byte-identical code merges right away.', - 'Peer reviews come in groups (up to 40 functions of one batch and file): read the original and the submission (it is known to compile; do not compile or diff), and send all verdicts in one td_submit_peer_review call. A function merges once "same" leads "different" by three (3:0, 4:1, 5:2); it goes back once at least three "different" lead by two (3:1, 4:2).', + 'Peer reviews come in groups (up to 40 functions of one batch and file): read the original and the submission (it is known to compile; do not compile or diff), and send all verdicts in one td_submit_peer_review call. A function merges once "same" leads "different" by three and holds 70% of the votes (3:0, 4:1, 5:2, 7:3); it goes back once at least three "different" lead by two (3:1, 4:2), or after seven reviewers without a decision.', 'Quality counts: an account whose functions peers keep sending back, or whose submissions keep breaking, is limited and then paused for a while automatically (the quality field of td_get_batch / /api/me says why). Fewer, checked submissions beat many careless ones.', 'Trust pays: an agent whose close matches keep passing peer review gets functions scoring 95% or more accepted without review, and a reviewer whose verdicts keep agreeing with the others (including on the differences they find) counts twice or three times. Each agent (token) earns it on its own; td_quality shows where it stands.', 'If your code (or someone else\'s) breaks a neighbouring function so a region does not compile, claim that function too (td_claim_region covers it when it is in your region) and fix it in the same submission.', diff --git a/src/server/merge.js b/src/server/merge.js index 59db047..a95e711 100644 --- a/src/server/merge.js +++ b/src/server/merge.js @@ -373,7 +373,7 @@ export class Merger { (peers.length ? `\nPeer-reviewed-by: ${peers.join(', ')}` : '') + (reviewers.length ? `\nReviewed-by: ${reviewers.join(', ')}` : '') + (!reviewers.length && !confirmedIds.size ? '\nMerged automatically: every function is byte-identical to the original.' : '') + - (!reviewers.length && confirmedIds.size ? `\nAccepted without human review: peers confirmed the logic ("same" ahead by at least ${this.peers?.margin ?? 3}).` : '') + + (!reviewers.length && confirmedIds.size ? `\nAccepted without human review: peers confirmed the logic ("same" ahead by at least ${this.peers?.margin ?? 3}, with ${Math.round((this.peers?.acceptShare ?? 0.7) * 100)}% of the votes).` : '') + '\n'; // New source files join the build list in the same commit (with the flags of their folder). const listFile = this.project.compileCommands ?? null; diff --git a/src/server/peer.js b/src/server/peer.js index 236c7de..f237750 100644 --- a/src/server/peer.js +++ b/src/server/peer.js @@ -14,6 +14,9 @@ // (Attempts from before this rule may wait in in_review; they are accepted the same way.) // Verdicts are weighted: a reviewer whose verdicts keep agreeing with the others' counts twice or // three times (trust.js). Either decision still takes at least two reviewers. +// Accepting also takes a clear majority: "same" at least `peerAcceptShare` (default 0.7) of the +// weight, so a contested function is not worn down until "same" happens to lead (10:7). One that +// `peerMaxReviewers` (default 7) reviewed without a decision goes back to its author. import { HttpError } from './http.js'; import { now } from './db.js'; @@ -28,7 +31,7 @@ const REVIEW_GROUP = 40; export const PEER_INSTRUCTIONS = [ 'You hold every function listed under functions. Review them all, then send all verdicts in one call (td_submit_peer_review with a verdicts list).', 'A function with a claim field is not ported code but a claim that it needs no decompilation (and why). Read the original: "same" if the claim is right, "different" if it is ordinary code that should be ported.', - 'Decide whether the submitted C++ does the same thing as the original function. It compiles (the server proved that when it was submitted) but is not byte-identical; your judgment is what counts. A function merges once "same" leads "different" by three (3:0, 4:1, 5:2); it goes back once at least three "different" lead by two (3:1, 4:2); verdicts of reviewers who keep agreeing with the others count twice or three times, so careful reviews pay off.', + 'Decide whether the submitted C++ does the same thing as the original function. It compiles (the server proved that when it was submitted) but is not byte-identical; your judgment is what counts. A function merges once "same" leads "different" by three and holds 70% of the votes (3:0, 4:1, 5:2, 7:3); it goes back once at least three "different" lead by two (3:1, 4:2), or when seven reviewers could not decide it; verdicts of reviewers who keep agreeing with the others count twice or three times, so careful reviews pay off.', 'Read only: the original assembly (td_disassemble), the original pseudocode (td_decompile) and the submitted code below. Do not compile, run td_check, diff bytes or use other automated comparison.', 'Check: every branch and condition, loop bounds, arithmetic and signedness, the functions called and their arguments, fields and globals read and written, return values, and error paths.', 'Answer "same" only if a caller could not tell the two apart. Otherwise answer "different" and name the concrete difference (where, and what the original does instead).', @@ -56,6 +59,16 @@ export class PeerReviews { return this.project.matchPolicy.peerRejectMargin ?? 2; } + // The share of the weight "same" needs as well as its lead: 7:3, not 10:7. + get acceptShare() { + return this.project.matchPolicy.peerAcceptShare ?? 0.7; + } + + // Reviewers after which an undecided function goes back to its author. + get maxReviewers() { + return this.project.matchPolicy.peerMaxReviewers ?? 7; + } + // The fewest "same" verdicts that accept a function (nobody objected). get acceptClean() { return this.margin; @@ -63,7 +76,8 @@ export class PeerReviews { /** "same" verdicts (by weight) this attempt needs, given the "different" ones. */ needed(different) { - return different + this.margin; + const share = this.acceptShare; + return Math.max(different + this.margin, share < 1 ? Math.ceil((share * different) / (1 - share) - 1e-9) : Infinity); } get leaseMs() { @@ -84,13 +98,14 @@ export class PeerReviews { SELECT a.id AS attempt_id, a.batch_id, a.task_id, a.state, t.symbol_id, s.name, s.unit, a.finished_at, (SELECT COALESCE(SUM(p.weight), 0) FROM peer_reviews p WHERE p.attempt_id = a.id AND p.verdict = 'same') AS confirmations, (SELECT COALESCE(SUM(p.weight), 0) FROM peer_reviews p WHERE p.attempt_id = a.id AND p.verdict = 'different') AS objections, + (SELECT COUNT(*) FROM peer_reviews p WHERE p.attempt_id = a.id) AS reviewers, (SELECT COUNT(*) FROM peer_leases l WHERE l.attempt_id = a.id AND l.expires_at > ?) AS in_progress FROM attempts a JOIN tasks t ON t.id = a.task_id JOIN symbols s ON s.id = t.symbol_id WHERE a.state IN ('peer_review', 'in_review') AND a.user_id != ? AND NOT EXISTS (SELECT 1 FROM users b WHERE b.id = ? AND b.peer_review_blocked = 1) AND NOT EXISTS (SELECT 1 FROM peer_reviews p WHERE p.attempt_id = a.id AND p.reviewer_id = ?) ) - WHERE NOT (objections >= ? AND objections - confirmations >= ?) AND confirmations + in_progress < objections + ?`, + WHERE NOT (objections >= ? AND objections - confirmations >= ?) AND confirmations + in_progress < objections + ? AND reviewers + in_progress < ?`, now(), user.id, user.id, @@ -98,7 +113,8 @@ export class PeerReviews { this.required, this.rejectMargin, this.margin, - ); + this.maxReviewers, + ).filter((r) => r.confirmations + r.in_progress < this.needed(r.objections)); // Random by batch, so a batch stays together. const key = new Map(); for (const r of rows) { @@ -215,7 +231,7 @@ export class PeerReviews { const leased = this.store.get('SELECT 1 FROM peer_leases WHERE attempt_id = ? AND reviewer_id = ? AND expires_at > ?', attemptId, user.id, now()); const counts = this.counts(attemptId); const others = this.store.get('SELECT COUNT(*) AS n FROM peer_leases WHERE attempt_id = ? AND reviewer_id != ? AND expires_at > ?', attemptId, user.id, now()).n; - const full = counts.same + others >= this.needed(counts.different); + const full = counts.same + others >= this.needed(counts.different) || counts.sameReviewers + counts.differentReviewers + others >= this.maxReviewers; if (!leased && full) throw new HttpError(409, 'enough reviewers are already on this attempt; claim another'); const weight = this.trust?.reviewer(user.id, user.tokenId ?? null).weight ?? 1; @@ -243,18 +259,25 @@ export class PeerReviews { this.store.event('auto_accepted', null, { attempt: a.id, same, different }); return { state: 'approved', same, different, required: this.needed(different) }; } - if (this.rejects(c)) { - // Enough independent reviewers found a difference: back to the author with their reasons. + const reviewers = c.sameReviewers + c.differentReviewers; + const undecided = !this.rejects(c) && reviewers >= this.maxReviewers; + if (this.rejects(c) || undecided) { + // Enough independent reviewers found a difference, or so many could not agree that more + // would not settle it: back to the author with their reasons. + const why = undecided + ? `${reviewers} peer reviewers could not agree (${same} "same" to ${different} "different" by weight); look at the "different" reasons, change the code and resubmit` + : `${different} peer reviews found a difference`; this.store.run('DELETE FROM peer_leases WHERE attempt_id = ?', a.id); - this.store.run(`UPDATE attempts SET state = 'needs_cleanup' WHERE id = ?`, a.id); + this.store.run(`UPDATE attempts SET state = 'needs_cleanup', result_json = json_set(COALESCE(result_json, '{}'), '$.peerOutcome', ?) WHERE id = ?`, why, a.id); + if (undecided) this.store.event('peer_undecided', null, { attempt: a.id, same, different, reviewers }); this.store.run( `UPDATE tasks SET state = 'needs_cleanup', cleanup_cycles = cleanup_cycles + 1, preferred_user = ?, preferred_until = ?, updated_at = ? WHERE id = ? ${notOthers} AND NOT EXISTS (SELECT 1 FROM attempts o WHERE o.task_id = tasks.id AND o.id != ? AND o.state IN ('queued', 'verifying', 'peer_review', 'in_review', 'approved', 'merging'))`, a.user_id, now() + 6 * 3600_000, now(), a.task_id, a.id, ); // Only this function goes back; the rest of its batch carries on. - settleBatch(this.store, a.batch_id, `${different} peer reviews found a difference in ${a.task_id}`); - return { state: 'needs_cleanup', same, different, required: this.required }; + settleBatch(this.store, a.batch_id, `${why} (${a.task_id})`); + return { state: 'needs_cleanup', same, different, required: this.required, ...(undecided && { undecided: true }) }; } // Undecided: it stays in review until one side leads by enough. return { state: a.state, same, different, required: this.needed(different) }; @@ -322,11 +345,12 @@ export class PeerReviews { return { same: r?.same ?? 0, different: r?.different ?? 0, sameReviewers: r?.sameReviewers ?? 0, differentReviewers: r?.differentReviewers ?? 0 }; } - // The side that leads decides: "same" ahead by `margin` (3:0, 4:1, 5:2) accepts; at least - // `required` "different" ahead by `rejectMargin` (3:1, 4:2) send it back. Close votes wait for - // more verdicts. However much a verdict weighs, one reviewer never decides. + // The side that leads decides: "same" ahead by `margin` with `acceptShare` of the weight (3:0, + // 4:1, 5:2, 7:3) accepts; at least `required` "different" ahead by `rejectMargin` (3:1, 4:2) send + // it back. Close votes wait for more verdicts, up to `maxReviewers` (decide). However much a + // verdict weighs, one reviewer never decides. accepts(c) { - return c.same - c.different >= this.margin && c.sameReviewers >= 2; + return c.same - c.different >= this.margin && c.same >= this.acceptShare * (c.same + c.different) && c.sameReviewers >= 2; } rejects(c) { diff --git a/src/server/trust.js b/src/server/trust.js index a4e689d..312b1b4 100644 --- a/src/server/trust.js +++ b/src/server/trust.js @@ -53,12 +53,16 @@ export class Trust { return { ...TRUST_DEFAULTS, ...p, weights: p.weights ?? TRUST_DEFAULTS.weights }; } - // The peer review rule (peer.js): "same" ahead by margin accepts, "different" ahead by - // rejectMargin (at least required of them) sends back. + // The peer review rule (peer.js): "same" ahead by margin with acceptShare of the weight accepts, + // "different" ahead by rejectMargin (at least required of them) sends back. get margin() { return this.project.matchPolicy.peerMargin ?? 3; } + get acceptShare() { + return this.project.matchPolicy.peerAcceptShare ?? 0.7; + } + get rejectMargin() { return this.project.matchPolicy.peerRejectMargin ?? 2; } @@ -92,11 +96,11 @@ export class Trust { const s = this.scope('a', tokenId); const w = (verdict) => `(SELECT COALESCE(SUM(r.weight), 0) FROM peer_reviews r WHERE r.attempt_id = a.id AND r.verdict = '${verdict}')`; const r = this.store.get( - `SELECT SUM(same - different >= ?) AS accepted, SUM(different >= ? AND different - same >= ?) AS rejected FROM ( + `SELECT SUM(same - different >= ? AND same >= ? * (same + different)) AS accepted, SUM(different >= ? AND different - same >= ?) AS rejected FROM ( SELECT ${w('same')} AS same, ${w('different')} AS different FROM attempts a WHERE a.user_id = ? AND ${s.sql} AND a.created_at > ? AND a.kind = 'code' AND a.peer_skipped = 0 AND a.score >= ?)`, - this.margin, this.required, this.rejectMargin, userId, ...s.args, now() - p.authorDays * DAY, p.bandAt, + this.margin, this.acceptShare, this.required, this.rejectMargin, userId, ...s.args, now() - p.authorDays * DAY, p.bandAt, ); const accepted = r?.accepted ?? 0; const rejected = r?.rejected ?? 0; diff --git a/src/server/verify.js b/src/server/verify.js index 0a4edd4..fa2dd78 100644 --- a/src/server/verify.js +++ b/src/server/verify.js @@ -7,7 +7,7 @@ import { lintSubmission } from '../shared/lint.js'; import { freeTextErrors } from '../shared/freetext.js'; import { checkUnits } from '../shared/check.js'; import { reconcile } from '../shared/reconcile.js'; -import { STATUS } from '../shared/compare.js'; +import { STATUS, sameOriginal } from '../shared/compare.js'; import { listSources, isUnit } from '../shared/project.js'; import { changedNames, unitsMentioning } from './affected.js'; import { undefinedDeclarations } from './undefined.js'; @@ -444,6 +444,29 @@ export class Verifier { ); } + /** + * Another function annotated on the same compiled body as `target`, accepted on main (byte- + * identical or confirmed by peers) with this very compiled code, whose original is the same code + * as the target's (sameOriginal). Its ID, or null. + */ + acceptedTwin(check, target) { + if (!target?.symbol || !target.addr || !target.codeHash) return null; + const size = target.size?.original; + const pe = this.getPe(); + // The target's file is its unit (evaluate); a decorated name stands for one body in a unit. + for (const u of check.units.filter((x) => x.file === target.file)) { + for (const f of u.functions) { + if (f.id === target.id || f.symbol !== target.symbol || !f.addr || f.codeHash !== target.codeHash) continue; + if (f.size?.original !== size) continue; + const main = this.store.get('SELECT status, confirmed, code_hash FROM func_status WHERE symbol_id = ?', f.id); + if (!main || main.code_hash !== f.codeHash) continue; + if (!(main.status === STATUS.EXACT || main.status === STATUS.UNRESOLVED || main.confirmed)) continue; + if (sameOriginal(pe, target.addr, f.addr, size)) return f.id; + } + } + return null; + } + /** * A function that three peers already confirmed keeps that confirmation when its compiled code is * unchanged in a resubmission (the batch came back because of another function). @@ -552,6 +575,15 @@ export class Verifier { ev.feedback.trusted = true; this.store.run('UPDATE attempts SET peer_skipped = 1 WHERE id = ?', a.id); } + // One body annotated for several functions: when the original of this one is the same code + // as another's that main already accepts from that body, the verdict on it holds for this one. + const twin = ev.state === 'peer_review' && a.kind !== 'skip' ? this.acceptedTwin(check, ev.feedback.target) : null; + if (twin) { + ev.state = 'approved'; + ev.feedback.summary = `not byte-identical (score ${(ev.score * 100).toFixed(1)}%), accepted without peer review: the original is the same code as ${twin}, which main already accepts from this body`; + ev.feedback.sameAs = twin; + this.store.run('UPDATE attempts SET peer_skipped = 1 WHERE id = ?', a.id); + } if (ev.state === 'needs_cleanup' || ev.state === 'rejected') failed++; this.finish(a, ev.state, ev.score, ev.feedback); } @@ -661,7 +693,7 @@ export class Verifier { state = auto ? 'approved' : 'in_review'; } else if (target.status === STATUS.MISMATCH || target.status === STATUS.UNRESOLVED) { // Not byte-identical, but it compiles cleanly: other agents judge whether the logic is the same. - feedback.summary = `not byte-identical (score ${(score * 100).toFixed(1)}%), sent to peer review: other agents compare it with the original (it merges once "same" leads "different" by ${policy.peerMargin ?? 3}; at least ${policy.peerReviews ?? 3} "different" leading by ${policy.peerRejectMargin ?? 2} send it back)`; + feedback.summary = `not byte-identical (score ${(score * 100).toFixed(1)}%), sent to peer review: other agents compare it with the original (it merges once "same" leads "different" by ${policy.peerMargin ?? 3} with ${Math.round((policy.peerAcceptShare ?? 0.7) * 100)}% of the votes; at least ${policy.peerReviews ?? 3} "different" leading by ${policy.peerRejectMargin ?? 2}, or ${policy.peerMaxReviewers ?? 7} reviewers without a decision, send it back)`; state = 'peer_review'; } else { feedback.summary = `the annotated function was not compiled or has no address in the current version (${target.status})`; diff --git a/src/shared/compare.js b/src/shared/compare.js index 33fecb3..d5de483 100644 --- a/src/shared/compare.js +++ b/src/shared/compare.js @@ -384,6 +384,29 @@ function diffRegions(diffs, ours, orig, proc) { }); } +/** + * Whether two functions of the original are the same code: the same size and bytes, where a + * 4-byte field may differ only as a relative reference (call, jump, RIP-relative operand) to the + * same target from both places. Jumps inside the function are relative to it and so already equal. + */ +export function sameOriginal(pe, addrA, addrB, size) { + if (!size) return false; + if (addrA === addrB) return true; + const a = pe.read(addrA, size); + const b = pe.read(addrB, size); + for (let i = 0; i < size; i++) { + if (a[i] === b[i]) continue; + // The field this byte belongs to starts at most three bytes earlier (which were equal). + let field = -1; + for (let p = Math.max(0, i - 3); p <= i && field < 0; p++) { + if (p + 4 <= size && a.readInt32LE(p) - b.readInt32LE(p) === addrB - addrA) field = p; + } + if (field < 0) return false; + i = field + 3; + } + return true; +} + /** Similarity ratio based on the longest common subsequence, like difflib. */ export function lcsRatio(a, b) { if (a.length === 0 && b.length === 0) return 1; diff --git a/test/compare.test.js b/test/compare.test.js index 07c65aa..1b8f671 100644 --- a/test/compare.test.js +++ b/test/compare.test.js @@ -5,7 +5,7 @@ import { PEImage } from '../src/shared/pe.js'; import { CoffObject } from '../src/shared/coff.js'; import { parseListing } from '../src/shared/listing.js'; import { parseAnnotations } from '../src/shared/annotations.js'; -import { compareObject, STATUS, lcsRatio } from '../src/shared/compare.js'; +import { compareObject, STATUS, lcsRatio, sameOriginal } from '../src/shared/compare.js'; import { reconcile } from '../src/shared/reconcile.js'; const FILE = 'src/player.cpp'; @@ -144,3 +144,23 @@ test('a file-local static resolves by its unit, never by the bare name', () => { const own = compare(new Map([['?Callee@@YAXXZ', 'fn_00002'], ['?g_val@@3HA', 'gl_09999'], ['src/player.obj|?g_val@@3HA', 'gl_00001']])); assert.equal(own.status, STATUS.EXACT); }); + +test('sameOriginal: two copies of a function match when their calls reach the same target', () => { + // Two 12-byte copies at 0x1000 and 0x1100: mov ecx, 1; call 0x2000; ret; nop; nop. + const image = Buffer.alloc(0x200, 0xcc); + const copy = (at, target) => { + const off = at - 0x1000; + image.set([0xb9, 1, 0, 0, 0, 0xe8], off); + image.writeInt32LE(target - (at + 10), off + 6); + image.set([0xc3, 0x90], off + 10); + }; + copy(0x1000, 0x2000); + copy(0x1100, 0x2000); + copy(0x1180, 0x2040); + const pe = { read: (va, len) => image.subarray(va - 0x1000, va - 0x1000 + len) }; + assert.equal(sameOriginal(pe, 0x1000, 0x1100, 12), true, 'same call target from two places'); + assert.equal(sameOriginal(pe, 0x1000, 0x1180, 12), false, 'calls another function'); + image[0x101] = 2; // mov ecx, 2 + assert.equal(sameOriginal(pe, 0x1000, 0x1100, 12), false, 'an immediate differs'); + assert.equal(sameOriginal(pe, 0x1000, 0x1100, 0), false, 'no size, no claim'); +}); diff --git a/test/server.test.js b/test/server.test.js index f5ab9c3..ccde157 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -882,6 +882,34 @@ test('approving a batch leaves its functions in peer review alone unless asked', for (const id of ids) ctx.store.run(`UPDATE tasks SET state = 'done' WHERE symbol_id = ?`, id); }); +test('one body for two functions: an accepted twin with the same original skips peer review', () => { + const image = Buffer.alloc(0x100, 0x90); + const realPe = ctx.verifier.getPe; + ctx.verifier.getPe = () => ({ read: (va, len) => image.subarray(va - 0x7000, va - 0x7000 + len) }); + const v = ctx.store.currentVersion(); + const [newId, oldId] = [0x7000, 0x7080].map((addr) => ctx.store.ensureSymbol({ kind: 'function', versionId: v.id, addr, size: 16, unit: 'src/twin.cpp' }).symbol.id); + const fn = (id, addr) => ({ id, file: 'src/twin.cpp', symbol: '?Swim@@QEAAXXZ', addr, size: { ours: 16, original: 16 }, codeHash: 'twin-hash', status: STATUS.MISMATCH, score: 0.3 }); + const check = { units: [{ file: 'src/twin.cpp', functions: [fn(newId, 0x7000), fn(oldId, 0x7080)] }] }; + const target = check.units[0].functions[0]; + const twin = (status, confirmed, hash = 'twin-hash') => + ctx.store.run( + `INSERT INTO func_status (symbol_id, file, status, score, commit_hash, updated_at, confirmed, code_hash) VALUES (?, 'src/twin.cpp', ?, 0.3, 'x', ?, ?, ?) + ON CONFLICT(symbol_id) DO UPDATE SET status = excluded.status, confirmed = excluded.confirmed, code_hash = excluded.code_hash`, + oldId, status, now(), confirmed, hash, + ); + twin(STATUS.MISMATCH, 1); + assert.equal(ctx.verifier.acceptedTwin(check, target), oldId, 'confirmed on main, same original bytes'); + twin(STATUS.MISMATCH, 0); + assert.equal(ctx.verifier.acceptedTwin(check, target), null, 'the twin is not accepted'); + twin(STATUS.EXACT, 0, 'other-hash'); + assert.equal(ctx.verifier.acceptedTwin(check, target), null, 'main accepted other code for the twin'); + twin(STATUS.EXACT, 0); + image[0x85] = 0x01; + assert.equal(ctx.verifier.acceptedTwin(check, target), null, 'the originals differ'); + ctx.verifier.getPe = realPe; + ctx.store.run('DELETE FROM func_status WHERE symbol_id = ?', oldId); +}); + test('a batch that merges only as a whole goes back once a function drops out of it', () => { const author = ctx.accounts.byName('pmauthor'); const v = ctx.store.currentVersion(); @@ -1247,30 +1275,46 @@ test('the live socket pushes new events, refuses other sites and ignores what cl } }); -test('a close vote waits for more verdicts; the side that leads decides', async () => { +test('a close vote waits for more verdicts; a clear majority accepts, seven undecided reviewers send it back', async () => { await activeUser('odauthor'); const author = ctx.accounts.byName('odauthor'); const peers = []; for (let i = 1; i <= 8; i++) peers.push(await activeUser(`odpeer${i}`)); const v = ctx.store.currentVersion(); - const id = ctx.store.ensureSymbol({ kind: 'function', versionId: v.id, addr: 0x402a00, size: 8, unit: 'src/od.cpp' }).symbol.id; - ctx.tasks.generate(); - const task = ctx.store.get('SELECT id FROM tasks WHERE symbol_id = ?', id).id; - ctx.store.run(`UPDATE tasks SET state = 'in_review' WHERE id = ?`, task); - // An attempt from the earlier rule: in human review, objections were not counted there. - const attempt = Number(ctx.store.run(`INSERT INTO attempts (task_id, user_id, base_commit, files_json, state, created_at) VALUES (?, ?, ?, '{}', 'in_review', ?)`, task, author.id, ctx.repo.head(), now()).lastInsertRowid); - const reviewers = ctx.peers; - for (const [i, verdict] of ['different', 'different', 'different', 'same', 'same', 'same', 'same'].entries()) { - ctx.store.run(`INSERT INTO peer_reviews (attempt_id, reviewer_id, verdict, reason, created_at) VALUES (?, ?, ?, 'checked every branch and call', ?)`, attempt, ctx.accounts.byName(`odpeer${i + 1}`).id, verdict, now()); - } - void reviewers; - await call('POST', '/api/peer-reviews/claim', { attemptId: attempt }, peers[7]); - const r = await call('POST', `/api/peer-reviews/${attempt}/verdict`, { verdict: 'same', reason: 'the same calls and the same returns' }, peers[7]); - assert.equal(r.body.state, 'in_review', 'five "same" against three "different": a lead of two, not yet'); - ctx.store.run(`INSERT INTO peer_reviews (attempt_id, reviewer_id, verdict, reason, created_at) VALUES (?, ?, 'same', 'checked every branch and call', ?)`, attempt, ctx.accounts.userByApiToken(await activeUser('odpeer9')).id, now()); - assert.deepEqual(ctx.peers.settleAll(), { approved: 1, needs_cleanup: 0 }, 'six to three: settled by the worker'); - assert.equal(ctx.store.get('SELECT state FROM attempts WHERE id = ?', attempt).state, 'approved'); - ctx.store.run(`UPDATE tasks SET state = 'done' WHERE id = ?`, task); + const make = (addr) => { + const id = ctx.store.ensureSymbol({ kind: 'function', versionId: v.id, addr, size: 8, unit: 'src/od.cpp' }).symbol.id; + ctx.tasks.generate(); + const task = ctx.store.get('SELECT id FROM tasks WHERE symbol_id = ?', id).id; + ctx.store.run(`UPDATE tasks SET state = 'peer_review' WHERE id = ?`, task); + const attempt = Number(ctx.store.run(`INSERT INTO attempts (task_id, user_id, base_commit, files_json, state, created_at) VALUES (?, ?, ?, '{}', 'peer_review', ?)`, task, author.id, ctx.repo.head(), now()).lastInsertRowid); + return { task, attempt }; + }; + const vote = (attempt, i, verdict, weight = 1) => + ctx.store.run(`INSERT INTO peer_reviews (attempt_id, reviewer_id, verdict, weight, reason, created_at) VALUES (?, ?, ?, ?, 'checked every branch and call', ?)`, attempt, ctx.accounts.byName(`odpeer${i}`).id, verdict, weight, now()); + const state = (attempt) => ctx.store.get('SELECT state FROM attempts WHERE id = ?', attempt).state; + + // 6:3 by weight leads by three but is only two thirds of the votes: it waits. + const a = make(0x402a00); + for (const i of [1, 2, 3]) vote(a.attempt, i, 'different'); + for (const i of [4, 5, 6]) vote(a.attempt, i, 'same', 2); + ctx.peers.settleAll(); + assert.equal(state(a.attempt), 'peer_review', '6:3 is not a clear majority'); + assert.equal(ctx.peers.needed(3), 7); + await call('POST', '/api/peer-reviews/claim', { attemptId: a.attempt }, peers[6]); + const r = await call('POST', `/api/peer-reviews/${a.attempt}/verdict`, { verdict: 'same', reason: 'the same calls and the same returns' }, peers[6]); + assert.equal(r.body.state, 'approved', '7:3 accepts'); + + // Seven reviewers who cannot agree: back to the author, with why. + const b = make(0x402a40); + for (const i of [1, 2, 3]) vote(b.attempt, i, 'different'); + for (const i of [4, 5, 6]) vote(b.attempt, i, 'same'); + await call('POST', '/api/peer-reviews/claim', { attemptId: b.attempt }, peers[6]); + const u = await call('POST', `/api/peer-reviews/${b.attempt}/verdict`, { verdict: 'same', reason: 'the same calls and the same returns' }, peers[6]); + assert.equal(u.body.state, 'needs_cleanup', '4:3 after seven reviewers'); + assert.equal(u.body.undecided, true); + assert.match(JSON.parse(ctx.store.get('SELECT result_json FROM attempts WHERE id = ?', b.attempt).result_json).peerOutcome, /7 peer reviewers could not agree/); + assert.equal(ctx.store.get('SELECT state FROM tasks WHERE id = ?', b.task).state, 'needs_cleanup'); + for (const t of [a.task, b.task]) ctx.store.run(`UPDATE tasks SET state = 'done' WHERE id = ?`, t); }); test('authors whose work peers keep sending back are limited, then paused; trusted ones are not', async () => {