Admins can merge a batch sent back only for lower matching scores

src/server/app.js

@@ -1000,6 +1000,15 @@ export function createApp(ctx) {
     return r;
   });
 
+  // An admin merges a batch that came back only for lowering matching scores (merge.js).
+  router.on('POST', '/api/admin/batches/:id/accept-regressions', async ({ req, params }) => {
+    const user = auth(req, { role: 'admin', session: true });
+    const body = await readJson(req, 16 * 1024);
+    const r = merger.acceptRegressions(user, intParam(params.id), body.comment ? String(body.comment) : null);
+    if (runsWorker) setImmediate(() => merger.pump().catch(() => {}));
+    return r;
+  });
+
   // --- maintainers and admins (browser session only)
 
   router.on('POST', '/api/admin/tasks/:id', async ({ req, params }) => {

src/server/cli.js

@@ -48,6 +48,8 @@ Usage: td-server <command> [options]   (all commands accept --data <dir>, defaul
   ghidra build | refresh                 the server's Ghidra project (download, pseudocode); refresh also runs after merges
   rebuild                                compile all of main and refresh match status
   tasks generate                         create tasks for functions that have none
+  batch accept-regressions <id> <admin> [comment]
+                                         merge a batch sent back only for lower matching scores
   toolchain pack [--image I]             package the compiler from the compile image for td toolchain
                                          install (downloads: approved accounts only)
 `;
@@ -345,6 +347,14 @@ async function main() {
     return;
   }
 
+  if (cmd === 'batch' && sub === 'accept-regressions') {
+    // The running worker picks the reopened batch up on its next round.
+    const ctx = openServer(dataDir);
+    const [id, admin, ...comment] = rest;
+    console.log(ctx.merger.acceptRegressions(ctx.accounts.requireUser(admin), Number(id), comment.join(' ') || null));
+    return;
+  }
+
   if (cmd === 'rebuild') {
     const ctx = openServer(dataDir);
     const t0 = Date.now();

src/server/db.js

@@ -313,6 +313,8 @@ export class Store {
     this.addColumn('peer_reviews', 'weight', 'INTEGER NOT NULL DEFAULT 1');
     this.addColumn('peer_reviews', 'token_id', 'INTEGER');
     this.addColumn('attempts', 'peer_skipped', 'INTEGER NOT NULL DEFAULT 0');
+    // A maintainer accepted that this attempt lowers matching scores (cleaner source over closer bytes).
+    this.addColumn('attempts', 'regressions_accepted', 'INTEGER NOT NULL DEFAULT 0');
     this.db.exec('CREATE INDEX IF NOT EXISTS attempts_user_time ON attempts(user_id, created_at)');
     // "No decompile" claims: an attempt that says a function needs no source (peer reviewed like code).
     this.addColumn('attempts', 'kind', "TEXT NOT NULL DEFAULT 'code'"); // code | skip

src/server/merge.js

@@ -102,6 +102,35 @@ export class Merger {
     return { state: newState };
   }
 
+  /**
+   * An admin's decision that a batch sent back only for lowering matching scores merges anyway:
+   * the project prefers that source (e.g. no calling convention the original code did not need)
+   * over closer bytes. Every function of the batch is accepted as it is, without peer review, and
+   * the merge records the lower scores.
+   */
+  acceptRegressions(admin, batchId, comment) {
+    if (!hasRole(admin, 'admin')) throw new HttpError(403, 'only admins can accept lower matching scores');
+    return this.store.tx(() => {
+      const batch = this.store.get('SELECT * FROM batches WHERE id = ?', batchId);
+      if (!batch) throw new HttpError(404, 'no such batch');
+      if (batch.state !== 'returned') throw new HttpError(409, `batch is ${batch.state}`);
+      const attempts = batchAttempts(this.store, batchId);
+      const blocking = attempts.filter((a) => {
+        const fb = a.result_json ? JSON.parse(a.result_json).feedback ?? {} : {};
+        return !['needs_cleanup', 'returned'].includes(a.state) || fb.compileErrors?.length || fb.annotationErrors?.length || fb.summary === 'compilation failed';
+      });
+      if (!attempts.length || blocking.length) throw new HttpError(409, `only a batch that compiled and came back for its scores can be accepted (${blocking.map((a) => a.symbol_id).join(', ') || 'empty'})`);
+      for (const a of attempts) {
+        this.store.run(`UPDATE attempts SET state = 'approved', regressions_accepted = 1, peer_skipped = 1 WHERE id = ?`, a.id);
+        this.store.run(`UPDATE tasks SET state = 'approved', updated_at = ? WHERE id = ? AND state != 'done'`, now(), a.task_id);
+        this.store.run('INSERT INTO reviews (attempt_id, reviewer_id, action, comment, created_at) VALUES (?, ?, ?, ?, ?)', a.id, admin.id, 'approve', comment?.slice(0, 4000) ?? null, now());
+      }
+      this.store.run(`UPDATE batches SET state = 'open', result_json = json_remove(COALESCE(result_json, '{}'), '$.returned', '$.mergeFailure') WHERE id = ?`, batchId);
+      this.store.event('regressions_accepted', admin.id, { batch: batchId, attempts: attempts.map((a) => a.id) });
+      return { batchId, accepted: attempts.map((a) => a.id) };
+    });
+  }
+
   /**
    * Merges the oldest open batch whose attempts are all approved. Failing that, splits off the
    * approved part of a batch whose other functions still wait for review, so finished work
@@ -333,8 +362,9 @@ export class Merger {
     const outside = this.verifier.changedOutside(check, merged.files, targets, batch.user_id);
     const confirmedIds = new Set();
     const results = [];
+    const allowRegressions = attempts.some((a) => a.regressions_accepted);
     for (const a of attempts) {
-      const ev = this.verifier.evaluate({ check, rec, targetId: a.symbol_id, files: merged.files, changedHeaders, outside });
+      const ev = this.verifier.evaluate({ check, rec, targetId: a.symbol_id, files: merged.files, changedHeaders, outside, allowRegressions });
       // Byte-identical, or confirmed by peers and approved by a human; anything worse bounces.
       const identical = ev.state === 'approved' || ev.state === 'in_review';
       const peerConfirmed = ev.state === 'peer_review' && this.peers?.confirmed(a.id);

src/server/verify.js

@@ -663,7 +663,7 @@ export class Verifier {
   }
 
   /** Routes a check result: rejected, needs_cleanup or in_review. */
-  evaluate({ check, rec, targetId, files, changedHeaders, outside = [] }) {
+  evaluate({ check, rec, targetId, files, changedHeaders, outside = [], allowRegressions = false }) {
     const fileSet = new Set(Object.keys(files));
     const policy = this.project.matchPolicy;
     const feedback = { summary: '', warnings: [] };
@@ -687,7 +687,7 @@ export class Verifier {
     for (const w of annWarnings) feedback.warnings.push(`${w.file}:${w.line} ${w.message}`);
     const failed = check.units.filter((u) => !u.compiled);
     const target = check.units.flatMap((u) => u.functions.map((f) => ({ ...f, file: u.file }))).find((f) => f.id === targetId);
-    const regressions = this.regressions(check, files);
+    const regressions = allowRegressions ? [] : this.regressions(check, files);
 
     feedback.target = target ?? null;
     feedback.annotationErrors = annErrors;

test/server.test.js

@@ -344,6 +344,43 @@ test('the last admin cannot be revoked or demoted', async () => {
   assert.equal((await request('POST', '/api/admin/users/root/role', { role: 'contributor' }, { cookie: admin })).status, 409);
 });
 
+test('an admin accepts a batch sent back only for lower scores; it merges past the regression check', async () => {
+  ctx.store.run(`INSERT INTO func_status (symbol_id, file, status, score, commit_hash, updated_at, confirmed) VALUES ('fn_00002', 'src/a.cpp', 'mismatch', 0.8, 'x', 0, 1)`);
+  const lower = { annotationErrors: [], units: [{ file: 'src/a.cpp', compiled: true, lintErrors: [], functions: [{ id: 'fn_00002', status: STATUS.MISMATCH, score: 0.7, relocations: [] }], unannotated: [] }] };
+  const rec = { conflicts: [], unknown: [], learned: [] };
+  const ev = (allowRegressions) => ctx.verifier.evaluate({ check: lower, rec, targetId: 'fn_00002', files: { 'src/a.cpp': '' }, changedHeaders: [], allowRegressions });
+  assert.equal(ev(false).state, 'needs_cleanup', 'a lower score of a confirmed function is a regression');
+  assert.equal(ev(true).state, 'peer_review', 'accepted: judged like any other mismatch');
+  ctx.store.run(`DELETE FROM func_status`);
+
+  await ctx.accounts.createUser('rgmaint', 'maintainer', PASSWORD);
+  await activeUser('rgauthor');
+  const author = ctx.accounts.byName('rgauthor');
+  const v = ctx.store.currentVersion();
+  const sym = ctx.store.ensureSymbol({ kind: 'function', versionId: v.id, addr: 0x401a00, size: 8, unit: 'src/rg.cpp' }).symbol.id;
+  ctx.tasks.generate();
+  const task = ctx.store.get('SELECT id FROM tasks WHERE symbol_id = ?', sym).id;
+  ctx.store.run(`UPDATE tasks SET state = 'needs_cleanup' WHERE id = ?`, task);
+  const batch = Number(ctx.store.run(`INSERT INTO batches (user_id, base_commit, files_json, state, created_at) VALUES (?, ?, '{}', 'returned', ?)`, author.id, ctx.repo.head(), now()).lastInsertRowid);
+  const feedback = (f) => JSON.stringify({ feedback: f });
+  const attempt = Number(ctx.store.run(`INSERT INTO attempts (task_id, user_id, base_commit, files_json, state, batch_id, result_json, created_at) VALUES (?, ?, ?, '{}', 'needs_cleanup', ?, ?, ?)`, task, author.id, ctx.repo.head(), batch, feedback({ summary: 'your change breaks 1 function(s) that matched before', compileErrors: [], annotationErrors: [] }), now()).lastInsertRowid);
+
+  assert.throws(() => ctx.merger.acceptRegressions(ctx.accounts.byName('rgmaint'), batch, null), /only admins/);
+  ctx.store.run(`UPDATE attempts SET result_json = ? WHERE id = ?`, feedback({ summary: 'compilation failed', compileErrors: [{ file: 'src/rg.cpp' }] }), attempt);
+  assert.throws(() => ctx.merger.acceptRegressions(ctx.accounts.byName('root'), batch, null), /compiled and came back for its scores/, 'code that does not build is never forced in');
+  ctx.store.run(`UPDATE attempts SET result_json = ? WHERE id = ?`, feedback({ summary: 'your change breaks 1 function(s) that matched before', compileErrors: [], annotationErrors: [] }), attempt);
+
+  assert.deepEqual(ctx.merger.acceptRegressions(ctx.accounts.byName('root'), batch, 'prefer no __stdcall').accepted, [attempt]);
+  const a = ctx.store.get('SELECT * FROM attempts WHERE id = ?', attempt);
+  assert.deepEqual([a.state, a.regressions_accepted, a.peer_skipped], ['approved', 1, 1]);
+  assert.ok(ctx.peers.confirmed(attempt), 'the merge re-check accepts it without peer verdicts');
+  assert.equal(ctx.store.get('SELECT state FROM batches WHERE id = ?', batch).state, 'open', 'back in the merge queue');
+  assert.throws(() => ctx.merger.acceptRegressions(ctx.accounts.byName('root'), batch, null), /batch is open/);
+  ctx.store.run(`UPDATE batches SET state = 'superseded' WHERE id = ?`, batch);
+  ctx.store.run(`UPDATE attempts SET state = 'rejected' WHERE id = ?`, attempt);
+  ctx.store.run(`UPDATE tasks SET state = 'done' WHERE id = ?`, task);
+});
+
 test('changing the password logs out browser sessions', async () => {
   const s = await login('erin');
   const r = await request('POST', '/api/me/password', { current: PASSWORD, password: 'a brand new password' }, { cookie: s });