Merge upstream main through aa55f6e

src/server/app.js

@@ -393,7 +393,7 @@ export function createApp(ctx) {
     // several times in a row.
     const per = config.reviewsPerClaim ?? 3;
     const since = Date.now() - 3600_000;
-    const claims = store.get(`SELECT COUNT(*) AS n FROM events WHERE user_id = ? AND type IN ('claim', 'claim_batch') AND at > ?`, user.id, since).n;
+    const claims = store.get(`SELECT COUNT(*) AS n FROM events WHERE user_id = ? AND type IN ('claim', 'claim_batch', 'region_claim') AND at > ?`, user.id, since).n;
     const done = store.get('SELECT COUNT(*) AS n FROM peer_reviews WHERE reviewer_id = ? AND created_at > ?', user.id, since).n;
     const need = Math.min(per, (claims + 1) * per - done);
     if (need <= 0) return;

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.',

src/server/merge.js

@@ -127,9 +127,29 @@ export class Merger {
     this.log.info(`${done.length} function(s) accepted as needing no decompilation`);
   }
 
+  /**
+   * A batch that only merges as a whole (noSplit) can't once one of its functions has dropped out
+   * after that was decided (sent back, withdrawn, superseded): it all goes back to its author
+   * instead of holding its approved functions forever.
+   */
+  returnUnmergeable() {
+    const stuck = this.store.all(
+      `SELECT b.id, json_extract(b.result_json, '$.noSplit') AS reason FROM batches b WHERE b.state = 'open' AND json_extract(COALESCE(b.result_json, '{}'), '$.noSplit') IS NOT NULL
+          AND EXISTS (SELECT 1 FROM attempts a WHERE a.batch_id = b.id AND a.state IN ('needs_cleanup', 'rejected', 'returned', 'withdrawn', 'superseded'))`,
+    ).filter((b) => !this.active.has(b.id));
+    if (!stuck.length) return;
+    this.store.tx(() => {
+      for (const b of stuck) returnBatch(this.store, b.id, `a function of this batch dropped out and the rest cannot be merged without it: ${b.reason}`);
+    });
+    this.log.info(`${stuck.length} batch(es) returned: a function dropped out and the rest cannot merge without it`);
+  }
+
   /** Starts merges while a lane is free; resolves when the merges it started are done. */
   async pump() {
-    if (!this.stopped) this.finishSkips();
+    if (!this.stopped) {
+      this.finishSkips();
+      this.returnUnmergeable();
+    }
     const started = [];
     while (!this.stopped && this.running < MERGE_SLOTS) {
       const pick = this.next(this.runningBig > 0);
@@ -257,7 +277,7 @@ export class Merger {
     }
     this.store.run(`UPDATE batches SET result_json = json_set(COALESCE(result_json, '{}'), '$.noSplit', ?) WHERE id = ?`, reason, batch.id);
     // With a function sent back it can never merge as a whole, so it all goes back.
-    if (this.store.get(`SELECT 1 FROM attempts WHERE batch_id = ? AND state IN ('needs_cleanup', 'rejected', 'returned', 'withdrawn')`, batch.id)) {
+    if (this.store.get(`SELECT 1 FROM attempts WHERE batch_id = ? AND state IN ('needs_cleanup', 'rejected', 'returned', 'withdrawn', 'superseded')`, batch.id)) {
       returnBatch(this.store, batch.id, `a function of this batch was sent back and the rest cannot be merged without it: ${reason}`);
       return this.log.info(`batch ${batch.id} returned: ${reason}`);
     }
@@ -353,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;

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() {
@@ -80,25 +94,24 @@ export class PeerReviews {
     // Room for this reviewer is part of the query, so the limit counts functions they can take
     // (filtering after the limit returned an empty list while the first rows were fully leased).
     const rows = this.store.all(
-      `SELECT * FROM (
+      `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 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 = '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 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 AND p.verdict = 'same') AS same_reviewers,
-                (SELECT COUNT(*) FROM peer_leases l WHERE l.attempt_id = a.id AND l.expires_at > ?) AS in_progress
+              (SELECT COUNT(*) FROM peer_reviews p WHERE p.attempt_id = a.id AND p.verdict = 'different') AS different_reviewers,
-           FROM attempts a JOIN tasks t ON t.id = a.task_id JOIN symbols s ON s.id = t.symbol_id
+              (SELECT COUNT(*) FROM peer_leases l WHERE l.attempt_id = a.id AND l.expires_at > ?) AS in_progress
-          WHERE a.state IN ('peer_review', 'in_review') AND a.user_id != ?
+         FROM attempts a JOIN tasks t ON t.id = a.task_id JOIN symbols s ON s.id = t.symbol_id
-            AND NOT EXISTS (SELECT 1 FROM users b WHERE b.id = ? AND b.peer_review_blocked = 1)
+        WHERE a.state IN ('peer_review', 'in_review') AND a.user_id != ?
-            AND NOT EXISTS (SELECT 1 FROM peer_reviews p WHERE p.attempt_id = a.id AND p.reviewer_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 + ?`,
       now(),
       user.id,
       user.id,
       user.id,
-      this.required,
+    ).filter((r) => {
-      this.rejectMargin,
+      r.reviewers = r.same_reviewers + r.different_reviewers;
-      this.margin,
+      return !this.full({ same: r.confirmations, different: r.objections, sameReviewers: r.same_reviewers, differentReviewers: r.different_reviewers }, r.in_progress);
-    );
+    });
     // Random by batch, so a batch stays together.
     const key = new Map();
     for (const r of rows) {
@@ -215,8 +228,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);
+      if (!leased && this.full(counts, others)) throw new HttpError(409, 'enough reviewers are already on this attempt; claim another');
-      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;
       this.store.run(
@@ -243,18 +255,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)) {
+    const reviewers = c.sameReviewers + c.differentReviewers;
-      // Enough independent reviewers found a difference: back to the author with their reasons.
+    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}`);
+      settleBatch(this.store, a.batch_id, `${why} (${a.task_id})`);
-      return { state: 'needs_cleanup', same, different, required: this.required };
+      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,17 +341,26 @@ 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
+  // The side that leads decides: "same" ahead by `margin` with `acceptShare` of the weight (3:0,
-  // `required` "different" ahead by `rejectMargin` (3:1, 4:2) send it back. Close votes wait for
+  // 4:1, 5:2, 7:3) accepts; at least `required` "different" ahead by `rejectMargin` (3:1, 4:2) send
-  // more verdicts. However much a verdict weighs, one reviewer never decides.
+  // 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) {
     return c.different >= this.required && c.different - c.same >= this.rejectMargin && c.differentReviewers >= 2;
   }
 
+  // No room for another reviewer: decided, or decided once the `inProgress` leased reviews come
+  // back "same", or at `maxReviewers`. The same rules as decide(), reviewer counts included, so a
+  // function one heavy verdict leads is still offered until a second reviewer settles it.
+  full(c, inProgress = 0) {
+    const hoped = { ...c, same: c.same + inProgress, sameReviewers: c.sameReviewers + inProgress };
+    return this.rejects(c) || this.accepts(hoped) || c.sameReviewers + c.differentReviewers + inProgress >= this.maxReviewers;
+  }
+
   /** Passed peer review, or a trusted author's function that did not need it (trust.js). */
   confirmed(attemptId) {
     if (this.store.get('SELECT 1 FROM attempts WHERE id = ? AND peer_skipped = 1', attemptId)) return true;

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
+  // The peer review rule (peer.js): "same" ahead by margin with acceptShare of the weight accepts,
-  // rejectMargin (at least required of them) sends back.
+  // "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;

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).
@@ -539,8 +562,10 @@ export class Verifier {
         ev.feedback.warnings.push('your local toolchain differs from the pinned one; local results may not match the server');
       }
       if (ev.state === 'peer_review' && this.carryOver(a, ev.feedback.target)) {
-        ev.state = 'in_review';
+        // Their verdicts already accept it, so it goes on as approved (in_review would wait for a
-        ev.feedback.summary += '; the compiled code is unchanged since three peers confirmed it, so their verdicts carry over';
+        // human review, and the peer queue skips functions its rule has decided).
+        ev.state = 'approved';
+        ev.feedback.summary += '; the compiled code is unchanged since peers confirmed it, so their verdicts carry over and it is accepted';
       }
       // A trusted author's function that is close to the original: accepted without peer review
       // (but for a random sample that keeps the author's record measured; trust.js).
@@ -550,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);
     }
@@ -659,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})`;

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;

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');
+});

test/server.test.js

@@ -629,6 +629,27 @@ test('a long review queue makes new claims wait for a few peer reviews', async (
   }
 });
 
+test('region claims count toward the reviews a long queue asks for', async () => {
+  await activeUser('rcauthor');
+  const author = ctx.accounts.byName('rcauthor');
+  const token = await activeUser('rcworker');
+  const worker = ctx.accounts.userByApiToken(token);
+  const saved = ctx.store.get(`SELECT id, state FROM tasks WHERE state != 'done' ORDER BY id LIMIT 1`);
+  const attempts = [0, 1, 2, 3].map(() => Number(ctx.store.run(`INSERT INTO attempts (task_id, user_id, base_commit, files_json, state, created_at) VALUES (?, ?, ?, '{}', 'peer_review', ?)`, saved.id, author.id, ctx.repo.head(), now()).lastInsertRowid));
+  const before = ctx.config.peerReviewFirstAt;
+  ctx.config.peerReviewFirstAt = 0;
+  try {
+    for (const a of attempts.slice(0, 3)) ctx.peers.submit(worker, a, 'same', 'the same branches and calls as the original', null);
+    ctx.store.event('region_claim', worker.id, { kind: 'unit', key: 'src/rc.cpp', leased: 5, shared: 0 });
+    const r = await call('POST', '/api/regions/claim', {}, token);
+    assert.equal(r.status, 429, 'three reviews paid for the region already claimed, not for the next one');
+  } finally {
+    ctx.config.peerReviewFirstAt = before;
+    ctx.store.run(`UPDATE attempts SET state = 'superseded' WHERE id IN (${attempts.join(',')})`);
+    ctx.store.run('UPDATE tasks SET state = ? WHERE id = ?', saved.state, saved.id);
+  }
+});
+
 test('the review list skips fully leased functions before applying its limit', async () => {
   await activeUser('rlauthor');
   await activeUser('rlviewer');
@@ -882,6 +903,55 @@ 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();
+  const ids = [0x401880, 0x4018c0].map((addr) => ctx.store.ensureSymbol({ kind: 'function', versionId: v.id, addr, size: 8, unit: 'src/ns.cpp' }).symbol.id);
+  ctx.tasks.generate();
+  const batch = Number(ctx.store.run(`INSERT INTO batches (user_id, base_commit, files_json, state, created_at, result_json) VALUES (?, ?, '{}', 'open', ?, ?)`, author.id, ctx.repo.head(), now(), JSON.stringify({ noSplit: 'conflicts with newer work on main' })).lastInsertRowid);
+  const [approved, dropped] = ids.map((id) => {
+    const task = ctx.store.get('SELECT id FROM tasks WHERE symbol_id = ?', id).id;
+    ctx.store.run(`UPDATE tasks SET state = 'approved' WHERE id = ?`, task);
+    return Number(ctx.store.run(`INSERT INTO attempts (task_id, user_id, base_commit, files_json, state, batch_id, created_at) VALUES (?, ?, ?, '{}', 'approved', ?, ?)`, task, author.id, ctx.repo.head(), batch, now()).lastInsertRowid);
+  });
+  ctx.merger.returnUnmergeable();
+  assert.equal(ctx.store.get('SELECT state FROM batches WHERE id = ?', batch).state, 'open', 'everything approved: it can still merge whole');
+  ctx.store.run(`UPDATE attempts SET state = 'needs_cleanup' WHERE id = ?`, dropped);
+  ctx.merger.returnUnmergeable();
+  assert.equal(ctx.store.get('SELECT state FROM batches WHERE id = ?', batch).state, 'returned');
+  assert.equal(ctx.store.get('SELECT state FROM attempts WHERE id = ?', approved).state, 'returned');
+  assert.equal(ctx.store.get('SELECT t.state FROM tasks t JOIN attempts a ON a.task_id = t.id WHERE a.id = ?', approved).state, 'needs_cleanup');
+  for (const id of ids) ctx.store.run(`UPDATE tasks SET state = 'done' WHERE symbol_id = ?`, id);
+});
+
 test('split: the approved part of a batch merges while the rest waits for review', async () => {
   const author = await activeUser('sauthor');
   const peers = [];
@@ -1226,30 +1296,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;
+  const make = (addr) => {
-  ctx.tasks.generate();
+    const id = ctx.store.ensureSymbol({ kind: 'function', versionId: v.id, addr, size: 8, unit: 'src/od.cpp' }).symbol.id;
-  const task = ctx.store.get('SELECT id FROM tasks WHERE symbol_id = ?', id).id;
+    ctx.tasks.generate();
-  ctx.store.run(`UPDATE tasks SET state = 'in_review' WHERE id = ?`, task);
+    const task = ctx.store.get('SELECT id FROM tasks WHERE symbol_id = ?', id).id;
-  // An attempt from the earlier rule: in human review, objections were not counted there.
+    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 (?, ?, ?, '{}', 'in_review', ?)`, task, author.id, ctx.repo.head(), now()).lastInsertRowid);
+    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);
-  const reviewers = ctx.peers;
+    return { task, attempt };
-  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());
+  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());
-  void reviewers;
+  const state = (attempt) => ctx.store.get('SELECT state FROM attempts WHERE id = ?', attempt).state;
-  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]);
+  // 6:3 by weight leads by three but is only two thirds of the votes: it waits.
-  assert.equal(r.body.state, 'in_review', 'five "same" against three "different": a lead of two, not yet');
+  const a = make(0x402a00);
-  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());
+  for (const i of [1, 2, 3]) vote(a.attempt, i, 'different');
-  assert.deepEqual(ctx.peers.settleAll(), { approved: 1, needs_cleanup: 0 }, 'six to three: settled by the worker');
+  for (const i of [4, 5, 6]) vote(a.attempt, i, 'same', 2);
-  assert.equal(ctx.store.get('SELECT state FROM attempts WHERE id = ?', attempt).state, 'approved');
+  ctx.peers.settleAll();
-  ctx.store.run(`UPDATE tasks SET state = 'done' WHERE id = ?`, task);
+  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 () => {
@@ -1460,8 +1546,14 @@ test('an admin sets a reviewer weight: it counts at once, also on verdicts alrea
   const alone = open();
   assert.equal(ctx.peers.submit(trusted, alone, 'same', why, null).weight, 3);
   assert.equal(ctx.store.get('SELECT state FROM attempts WHERE id = ?', alone).state, 'peer_review', 'one reviewer never decides');
+  assert.ok(ctx.peers.available(other, 1000).some((o) => o.attempt_id === alone), 'still offered: a second reviewer settles it');
+  ctx.peers.submit(other, alone, 'same', why, null);
+  assert.equal(ctx.store.get('SELECT state FROM attempts WHERE id = ?', alone).state, 'approved');
+  const objected = open();
+  ctx.peers.submit(trusted, objected, 'different', 'the loop bound is off by one against the original', null);
+  assert.ok(ctx.peers.available(other, 1000).some((o) => o.attempt_id === objected), 'one heavy "different" alone is offered too');
   ctx.peers.setReviewWeight(trusted.id, null);
   assert.equal(ctx.trust.reviewer(trusted.id).weight, 1, 'auto: back to the earned weight');
-  ctx.store.run(`UPDATE attempts SET state = 'superseded' WHERE id IN (?, ?)`, waiting, alone);
+  ctx.store.run(`UPDATE attempts SET state = 'superseded' WHERE id IN (?, ?, ?)`, waiting, alone, objected);
   ctx.store.run('UPDATE tasks SET state = ? WHERE id = ?', saved.state, saved.id);
 });