diff --git a/src/server/app.js b/src/server/app.js index bfdd4ee..91067a4 100644 --- a/src/server/app.js +++ b/src/server/app.js @@ -17,6 +17,7 @@ import { ServerGhidra } from './ghidra.js'; import { functionRegions } from './split.js'; import { CheckQueue, WorkerUnavailable } from './checks.js'; import { withGuidance } from './guidance.js'; +import { reviewerStats } from './reviewers.js'; import { Router, HttpError, readJson, sendJson, RateLimiter } from './http.js'; import { PEImage } from '../shared/pe.js'; import { loadProject, loadToolchainLock } from '../shared/project.js'; @@ -601,7 +602,7 @@ export function createApp(ctx) { router.on('GET', '/api/admin/users', ({ req, query }) => { auth(req, { role: 'admin', session: true }); - return { roles: ROLES, users: accounts.listUsers(query.get('status') ?? undefined), work: claimedWork() }; + return { roles: ROLES, users: accounts.listUsers(query.get('status') ?? undefined), work: claimedWork(), reviewers: reviewerStats(store) }; }); // What each user currently holds, by name: leased tasks grouped by unit, peer reviews, submitted batches in flight. @@ -632,6 +633,17 @@ export function createApp(ctx) { reject: (name, admin) => accounts.reject(name, admin.id), revoke: (name, admin) => accounts.revoke(name, admin.id), reinstate: (name, admin) => accounts.reinstate(name, admin.id), + // Pause or allow an account's peer reviews (e.g. rubber-stamping); its existing verdicts stay. + 'block-reviews': (name, admin) => setReviewBlock(name, 1, admin), + 'allow-reviews': (name, admin) => setReviewBlock(name, 0, admin), + }; + const setReviewBlock = (name, blocked, admin) => { + const u = accounts.byName(name); + if (!u) throw new HttpError(404, 'no such user'); + store.run('UPDATE users SET peer_review_blocked = ? WHERE id = ?', blocked, u.id); + if (blocked) store.run('DELETE FROM peer_leases WHERE reviewer_id = ?', u.id); + store.event(blocked ? 'reviews_blocked' : 'reviews_allowed', admin.id, { name }); + return { name, peerReviewsBlocked: !!blocked }; }; router.on('POST', '/api/admin/users/:name/:action', async ({ req, params }) => { const admin = auth(req, { role: 'admin', session: true }); diff --git a/src/server/db.js b/src/server/db.js index 1275f68..fa1d700 100644 --- a/src/server/db.js +++ b/src/server/db.js @@ -242,7 +242,8 @@ export class Store { this.addColumn('func_status', 'code_hash', 'TEXT'); this.addColumn('versions', 'label', 'TEXT'); this.addColumn('check_jobs', 'files_hash', 'TEXT'); // a retry of the same check joins the queued one - this.addColumn('batches', 'cost', 'INTEGER'); // units its verification compiles (queue order) // human-readable name, e.g. "World of Warcraft 5.0.3 (build 15890, x64)" + this.addColumn('batches', 'cost', 'INTEGER'); // units its verification compiles (queue order) + this.addColumn('users', 'peer_review_blocked', 'INTEGER NOT NULL DEFAULT 0'); // no peer reviews from this account // human-readable name, e.g. "World of Warcraft 5.0.3 (build 15890, x64)" this.addColumn('attempts', 'batch_id', 'INTEGER REFERENCES batches(id)'); this.db.exec('CREATE INDEX IF NOT EXISTS attempts_batch ON attempts(batch_id)'); } diff --git a/src/server/peer.js b/src/server/peer.js index 568fab1..4c68e4d 100644 --- a/src/server/peer.js +++ b/src/server/peer.js @@ -51,12 +51,14 @@ export class PeerReviews { (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 = ?) ORDER BY a.state = 'in_review', a.finished_at LIMIT ?`, now(), user.id, user.id, + user.id, Math.min(limit, 1000), ) .filter((r) => (r.state === 'in_review' ? this.openForAutoAccept(r) : Math.max(r.confirmations, r.objections) + r.in_progress < this.required)) @@ -162,6 +164,7 @@ export class PeerReviews { if (!a) throw new HttpError(404, 'no such attempt'); if (a.state !== 'peer_review' && a.state !== 'in_review') throw new HttpError(409, `attempt is ${a.state}, not waiting for peer review`); if (a.user_id === user.id) throw new HttpError(403, 'you cannot review your own attempt'); + if (this.store.get('SELECT 1 FROM users WHERE id = ? AND peer_review_blocked = 1', user.id)) throw new HttpError(403, 'peer reviews from this account are paused by a maintainer'); if (this.store.get('SELECT 1 FROM peer_reviews WHERE attempt_id = ? AND reviewer_id = ?', attemptId, user.id)) throw new HttpError(409, 'you already reviewed this attempt'); 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); diff --git a/src/server/reviewers.js b/src/server/reviewers.js new file mode 100644 index 0000000..ff46f98 --- /dev/null +++ b/src/server/reviewers.js @@ -0,0 +1,45 @@ +// How each peer reviewer judges, so a maintainer can see who to trust: how often they agree with +// the other reviewers of the same function and with the human decision, and how quickly they give +// verdicts in a row (rubber-stamping shows up as many verdicts seconds apart). + +const QUICK_MS = 10_000; + +export function reviewerStats(store) { + const verdicts = store.all( + `SELECT p.attempt_id, p.reviewer_id, p.verdict, p.created_at, u.name, u.peer_review_blocked AS blocked + FROM peer_reviews p JOIN users u ON u.id = p.reviewer_id ORDER BY p.reviewer_id, p.created_at`, + ); + // The human decision on an attempt: approved means the logic was right. + const human = new Map( + store.all(`SELECT attempt_id, action FROM reviews WHERE action IN ('approve', 'request_changes', 'reject') ORDER BY id`).map((r) => [r.attempt_id, r.action === 'approve' ? 'same' : 'different']), + ); + const byAttempt = new Map(); + for (const v of verdicts) { + if (!byAttempt.has(v.attempt_id)) byAttempt.set(v.attempt_id, []); + byAttempt.get(v.attempt_id).push(v); + } + + const stats = new Map(); + let last = null; + for (const v of verdicts) { + const s = stats.get(v.name) ?? { name: v.name, blocked: !!v.blocked, reviews: 0, same: 0, different: 0, peerCompared: 0, peerAgreed: 0, humanCompared: 0, humanAgreed: 0, quick: 0 }; + stats.set(v.name, s); + s.reviews++; + s[v.verdict]++; + // The majority of the other reviewers of this function, when there is one. + const others = byAttempt.get(v.attempt_id).filter((o) => o.reviewer_id !== v.reviewer_id); + const sameOthers = others.filter((o) => o.verdict === 'same').length; + if (others.length && sameOthers * 2 !== others.length) { + s.peerCompared++; + if ((sameOthers * 2 > others.length ? 'same' : 'different') === v.verdict) s.peerAgreed++; + } + const decided = human.get(v.attempt_id); + if (decided) { + s.humanCompared++; + if (decided === v.verdict) s.humanAgreed++; + } + if (last && last.reviewer_id === v.reviewer_id && v.created_at - last.created_at < QUICK_MS) s.quick++; + last = v; + } + return [...stats.values()].sort((a, b) => b.reviews - a.reviews); +} diff --git a/src/server/web/app.js b/src/server/web/app.js index 13d9b34..d5c21fa 100644 --- a/src/server/web/app.js +++ b/src/server/web/app.js @@ -239,7 +239,7 @@ async function adminAction(name, action, body) { } async function loadUsers() { - const { users, roles, work = {} } = await api('/api/admin/users'); + const { users, roles, work = {}, reviewers = [] } = await api('/api/admin/users'); const box = $('users'); box.replaceChildren(); const pending = users.filter((u) => u.status === 'pending'); @@ -283,6 +283,29 @@ async function loadUsers() { }), ), ); + box.append(el('h3', 'Peer reviewers')); + box.append(el('p', 'How reviewers judge: agreement with the other reviewers of the same function and with your decisions, and how many verdicts came within 10 seconds of their previous one (a sign of not reading). Pausing someone stops new peer reviews from them; their verdicts so far stay.', 'muted')); + if (!reviewers.length) box.append(el('p', 'No peer reviews yet.', 'muted')); + else { + const pct = (a, b) => (b ? `${Math.round((a / b) * 100)}% (${b})` : '-'); + box.append( + table( + ['Reviewer', 'Reviews', 'Same / different', 'Agrees with peers', 'Agrees with you', 'Quick verdicts', ''], + reviewers.map((r) => { + const flags = []; + if (r.reviews >= 15 && r.different === 0) flags.push('never objects'); + if (r.reviews >= 15 && r.same / r.reviews < 0.3) flags.push('mostly objects'); + if (r.reviews >= 10 && r.quick / r.reviews > 0.5) flags.push('very fast'); + const cell = [el('span', r.name)]; + if (flags.length) cell.push(el('div', flags.join(', '), 'warn')); + const action = r.blocked + ? button('Allow reviews', () => adminAction(r.name, 'allow-reviews')) + : button('Pause reviews', () => confirm(`Pause peer reviews from ${r.name}?`) && adminAction(r.name, 'block-reviews'), 'danger'); + return [cell, r.reviews, `${r.same} / ${r.different}`, pct(r.peerAgreed, r.peerCompared), pct(r.humanAgreed, r.humanCompared), pct(r.quick, r.reviews), action]; + }), + ), + ); + } } // Summary of what a user holds right now; the units it covers open below it. diff --git a/test/server.test.js b/test/server.test.js index e99a10c..6fb58db 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -11,6 +11,7 @@ import { fixture, FN_ADDR, CALLEE_ADDR, GLOBAL_ADDR } from './helpers.js'; import { openServer, createApp } from '../src/server/app.js'; import { Store, now } from '../src/server/db.js'; import { STATUS } from '../src/shared/compare.js'; +import { reviewerStats } from '../src/server/reviewers.js'; const ROOT = path.join(path.dirname(fileURLToPath(import.meta.url)), '..'); let dataDir, ctx, server, base; @@ -449,6 +450,19 @@ test('five agreeing peers and no objection accept a function without human revie } }); +test('reviewer stats and pausing a reviewer', async () => { + // The auto-accept test left verdicts from aapeer1..5; aapeer1 objected on the contested function. + const stats = Object.fromEntries(reviewerStats(ctx.store).map((r) => [r.name, r])); + assert.ok(stats.aapeer2.reviews >= 2); + assert.equal(stats.aapeer1.different, 1); + assert.ok(stats.aapeer2.peerCompared >= 1); + + const peer = ctx.accounts.byName('aapeer6'); + ctx.store.run('UPDATE users SET peer_review_blocked = 1 WHERE id = ?', peer.id); + assert.deepEqual(ctx.peers.available(peer), [], 'a paused reviewer gets no reviews'); + ctx.store.run('UPDATE users SET peer_review_blocked = 0 WHERE id = ?', peer.id); +}); + test('the crew view is public and shows no account details', async () => { const token = await activeUser('crewview'); await call('GET', '/api/tasks', null, token); // the agent checks in