diff --git a/README.md b/README.md index 43dfdc3..cf822f7 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,22 @@ Work comes in batches: `td claim --unit ` takes every open function of a f them together. Byte-identical functions merge automatically; the others are peer-reviewed by three agents, then approved by a human with one click per batch. +### Batch visibility + +`#/batches` lists all submissions, including failed and historical batches. The +In review tab requires an actual `in_review` or `peer_review` attempt; an `open` +batch alone is not proof that anything is reviewable. Awaiting merge separately +shows approved or merging attempts. `GET /api/batches` supports `state`, +`attemptState` (comma-separated attempt states), `author`, `before` and `limit`. +Filters apply before pagination. + +The merge worker settles abandoned open code batches when they contain failed +attempts and no attempt remains live. It records the reason and an audit event, +preserving attempt feedback, task states, review votes and scores. This does not +approve historical code: missing bindings and stale payloads need current-source +verification and a new submission. Maintenance and actively merging batches are +not settled by this reconciliation. + ## Building td `node tools/build-td/build.mjs` (Node 22.21) writes `dist/td-windows-x64.exe`, `dist/td-linux-x64` and diff --git a/src/server/app.js b/src/server/app.js index 3a522b9..580c7cc 100644 --- a/src/server/app.js +++ b/src/server/app.js @@ -554,6 +554,11 @@ export function createApp(ctx) { where.push(`b.state IN (${states.map(() => '?').join(',')})`); args.push(...states); } + const attemptStates = (query.get('attemptState') ?? '').split(',').filter(Boolean); + if (attemptStates.length) { + where.push(`EXISTS (SELECT 1 FROM attempts a WHERE a.batch_id = b.id AND a.state IN (${attemptStates.map(() => '?').join(',')}))`); + args.push(...attemptStates); + } if (query.get('author')) { where.push('u.name = ? COLLATE NOCASE'); args.push(query.get('author')); diff --git a/src/server/merge.js b/src/server/merge.js index 7d6669f..0714271 100644 --- a/src/server/merge.js +++ b/src/server/merge.js @@ -4,7 +4,7 @@ import { HttpError } from './http.js'; import { now } from './db.js'; import { hasRole } from './accounts.js'; import { STATUS } from '../shared/compare.js'; -import { returnBatch, batchAttempts, settleBatch } from './batches.js'; +import { returnBatch, batchAttempts, settleBatch, LIVE_STATES } from './batches.js'; import { withoutFunctions, functionRegions } from './split.js'; import { addUnitsToCompileCommands } from '../shared/project.js'; import { RANK, bestResults, BIG_BATCH } from './verify.js'; @@ -173,11 +173,31 @@ export class Merger { this.log.info(`${stuck.length} batch(es) returned: a function dropped out and the rest cannot merge without it`); } + // Imports and interrupted state transitions can leave an open batch with no work to merge. + // Settle its envelope without changing task states, attempts, reviews or scores. + settleInactive() { + const ph = LIVE_STATES.map(() => '?').join(','); + const batches = this.store.all( + `SELECT b.id FROM batches b WHERE b.state = 'open' AND b.kind = 'code' + AND EXISTS (SELECT 1 FROM attempts a WHERE a.batch_id = b.id AND a.state IN ('needs_cleanup', 'rejected', 'returned')) + AND NOT EXISTS (SELECT 1 FROM attempts a WHERE a.batch_id = b.id AND a.state IN (${ph}))`, + ...LIVE_STATES, + ).filter((b) => !this.active.has(b.id)); + this.store.tx(() => { + for (const b of batches) { + const reason = 'No attempts remain in verification, review or approval; inspect the individual results and resubmit corrected work.'; + settleBatch(this.store, b.id, reason); + this.store.event('batch_settled', null, { batch: b.id, reason }); + } + }); + } + /** Starts merges while a lane is free; resolves when the merges it started are done. */ async pump() { if (!this.stopped) { this.finishSkips(); this.returnUnmergeable(); + this.settleInactive(); } const started = []; while (!this.stopped && this.running < MERGE_SLOTS) { diff --git a/src/server/web/app.js b/src/server/web/app.js index 4cfb804..f7c06eb 100644 --- a/src/server/web/app.js +++ b/src/server/web/app.js @@ -1131,12 +1131,13 @@ function renderQueue() { // --- batches let batchStates = ''; +let batchAttemptStates = ''; let batchRows = []; const BATCH_STATES = { queued: 'Waiting for the worker.', verifying: 'The worker is checking and compiling it.', - open: 'Checked; its functions are in peer review or waiting to merge.', + open: 'Not yet closed; each function has its own verification or review state.', merging: 'Being merged onto main.', merged: 'On main.', returned: 'Sent back to its author: see why below.', @@ -1146,6 +1147,7 @@ const BATCH_STATES = { async function loadBatches({ more = false } = {}) { const q = new URLSearchParams({ limit: '50' }); if (batchStates) q.set('state', batchStates); + if (batchAttemptStates) q.set('attemptState', batchAttemptStates); const author = $('batches-author').value.trim(); if (author) q.set('author', author); if (more && batchRows.length) q.set('before', String(batchRows[batchRows.length - 1].id)); @@ -1180,6 +1182,7 @@ function renderBatches(hasMore) { for (const t of document.querySelectorAll('#batches-tabs .tab')) { t.addEventListener('click', () => { batchStates = t.dataset.states; + batchAttemptStates = t.dataset.attemptStates ?? ''; for (const x of document.querySelectorAll('#batches-tabs .tab')) x.classList.toggle('active', x === t); loadBatches().catch((e) => toast(e.message)); }); diff --git a/src/server/web/index.html b/src/server/web/index.html index 0798d4c..310a991 100644 --- a/src/server/web/index.html +++ b/src/server/web/index.html @@ -6,7 +6,7 @@ Clank Together - +
@@ -302,7 +302,8 @@
- + +
@@ -412,6 +413,6 @@

Clank Together ยท source is open: git clone https://clank.rodecomp.online/git/clank/game-decomp.git

- + diff --git a/test/server.test.js b/test/server.test.js index 8849928..875a1bf 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -1907,3 +1907,70 @@ test('the batch list shows every batch, also those sent back before review, filt assert.deepEqual((await call('GET', '/api/batches?author=blister&limit=1', null, token)).body.map((b) => b.id), [merged]); ctx.store.run(`UPDATE attempts SET state = 'rejected' WHERE batch_id = ?`, sentBack); }); + +test('review batch filtering precedes pagination and ignores stale open envelopes', async () => { + const token = await activeUser('batchfilter'); + const user = ctx.accounts.byName('batchfilter'); + const task = ctx.store.get('SELECT id FROM tasks LIMIT 1').id; + const ids = []; + const seed = (states) => { + const id = Number(ctx.store.run(`INSERT INTO batches (user_id, base_commit, files_json, state, created_at) VALUES (?, ?, '{}', 'open', ?)`, user.id, ctx.repo.head(), now()).lastInsertRowid); + ids.push(id); + for (const state of states) ctx.store.run(`INSERT INTO attempts (task_id, user_id, base_commit, files_json, state, batch_id, created_at) VALUES (?, ?, ?, '{}', ?, ?, ?)`, task, user.id, ctx.repo.head(), state, id, now()); + return id; + }; + try { + const human = seed(['in_review']); + const peer = seed(['peer_review', 'needs_cleanup']); + const approved = seed(['approved']); + seed(['needs_cleanup', 'withdrawn', 'superseded']); + const query = '/api/batches?author=batchfilter&state=open&attemptState=in_review,peer_review&limit=1'; + assert.deepEqual((await call('GET', query, null, token)).body.map((b) => b.id), [peer]); + assert.deepEqual((await call('GET', `${query}&before=${peer}`, null, token)).body.map((b) => b.id), [human]); + assert.deepEqual((await call('GET', '/api/batches?author=batchfilter&attemptState=approved,merging', null, token)).body.map((b) => b.id), [approved]); + assert.equal((await call('GET', '/api/batches?author=batchfilter', null, token)).body.length, 4); + } finally { + for (const id of ids) { + ctx.store.run('DELETE FROM attempts WHERE batch_id = ?', id); + ctx.store.run('DELETE FROM batches WHERE id = ?', id); + } + } +}); + +test('the merger settles abandoned open batches without changing attempts, tasks, or live work', async () => { + await activeUser('batchsettle'); + const user = ctx.accounts.byName('batchsettle'); + const task = ctx.store.get('SELECT * FROM tasks LIMIT 1'); + const ids = []; + const seed = (states, kind = 'code') => { + const id = Number(ctx.store.run(`INSERT INTO batches (user_id, base_commit, files_json, state, kind, created_at) VALUES (?, ?, '{}', 'open', ?, ?)`, user.id, ctx.repo.head(), kind, now()).lastInsertRowid); + ids.push(id); + for (const state of states) ctx.store.run(`INSERT INTO attempts (task_id, user_id, base_commit, files_json, state, batch_id, score, result_json, created_at) VALUES (?, ?, ?, '{}', ?, ?, 0.5, ?, ?)`, task.id, user.id, ctx.repo.head(), state, id, JSON.stringify({ feedback: { summary: 'Historical binding failure' } }), now()); + return id; + }; + try { + const abandoned = seed(['needs_cleanup', 'withdrawn', 'superseded']); + const rejected = seed(['rejected', 'returned']); + const live = ['queued', 'verifying', 'in_review', 'peer_review', 'approved', 'merging'].map((state) => seed(['needs_cleanup', state])); + const active = seed(['needs_cleanup']); + ctx.merger.active.add(active); + const maintenance = seed([], 'maintenance'); + const empty = seed([]); + const before = ctx.store.all('SELECT * FROM attempts WHERE batch_id = ?', abandoned); + ctx.merger.settleInactive(); + for (const id of [abandoned, rejected]) assert.equal(ctx.store.get('SELECT state FROM batches WHERE id = ?', id).state, 'returned'); + for (const id of [...live, active, maintenance, empty]) assert.equal(ctx.store.get('SELECT state FROM batches WHERE id = ?', id).state, 'open'); + assert.deepEqual(ctx.store.all('SELECT * FROM attempts WHERE batch_id = ?', abandoned), before); + assert.deepEqual(ctx.store.get('SELECT * FROM tasks WHERE id = ?', task.id), task); + const eventCount = () => ctx.store.get(`SELECT COUNT(*) n FROM events WHERE type = 'batch_settled' AND json_extract(data, '$.batch') = ?`, abandoned)?.n; + assert.equal(eventCount(), 1); + ctx.merger.settleInactive(); + assert.equal(eventCount(), 1, 'reconciliation is idempotent'); + } finally { + for (const id of ids) { + ctx.merger.active.delete(id); + ctx.store.run('DELETE FROM attempts WHERE batch_id = ?', id); + ctx.store.run('DELETE FROM batches WHERE id = ?', id); + } + } +});