Refuse removing annotations of functions not held; new source files join the build list

- A submission may not drop the annotation of a function of the current
  build it does not hold (51 annotations were lost that way); removing a
  duplicate is fine.
- A unit missing from compile.json compiles with the command shape of its
  folder, and the merge adds it to compile.json in the same commit (22
  contributor files were outside the build).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Saty 2026-10-20 04:56:07 +02:00
commit 67965b04f1
4 changed files with 102 additions and 3 deletions

View file

@ -6,6 +6,7 @@ import { hasRole } from './accounts.js';
import { STATUS } from '../shared/compare.js';
import { returnBatch, batchAttempts, settleBatch } from './batches.js';
import { withoutFunctions, functionRegions } from './split.js';
import { addUnitsToCompileCommands } from '../shared/project.js';
import { RANK, bestResults, BIG_BATCH } from './verify.js';
// Merges run in two lanes: one big merge (a header batch that must recompile many units) at a
@ -353,7 +354,12 @@ export class Merger {
(!reviewers.length && !confirmedIds.size ? '\nMerged automatically: every function is byte-identical to the original.' : '') +
(!reviewers.length && confirmedIds.size ? `\nAccepted without human review: at least ${this.peers?.autoAccept ?? 5} peers confirmed the logic.` : '') +
'\n';
const commit = this.repo.commitFiles(merged.files, message, `${author} <${author.replace(/[^A-Za-z0-9_.-]/g, '_')}@users.clank-together.invalid>`);
// New source files join the build list in the same commit (with the flags of their folder).
const listFile = this.project.compileCommands ?? null;
const grown = listFile ? addUnitsToCompileCommands(this.project, this.repo.show(head, listFile), Object.keys(merged.files)) : null;
const filesToCommit = grown ? { ...merged.files, [listFile]: grown.text } : merged.files;
const commit = this.repo.commitFiles(filesToCommit, grown ? `${message}Adds to the build: ${grown.added.join(', ')}\n` : message, `${author} <${author.replace(/[^A-Za-z0-9_.-]/g, '_')}@users.clank-together.invalid>`);
if (grown) this.project.compile.units = grown.units;
this.store.tx(() => {
this.applyCheckToDatabase(check, rec, commit, batch.user_id, { confirmedIds });

View file

@ -221,6 +221,38 @@ export class Verifier {
return errors;
}
/**
* Annotations of functions of the current version that the submission removes although it does
* not hold them (not in the batch, not leased, not in a claimed region). Removing a duplicate is
* fine: the function stays annotated elsewhere on main.
*/
droppedAnnotations(files, baseCommit, attempts, userId) {
const ANN = /\/\/\s*(?:FUNCTION|STUB)\s*:\s*(fn_\d+)/g;
const ids = (t) => new Set([...String(t ?? '').matchAll(ANN)].map((m) => m[1]));
const before = new Set();
const after = new Set();
for (const [f, text] of Object.entries(files)) {
for (const id of ids(this.repo.show(baseCommit, f))) before.add(id);
for (const id of ids(text)) after.add(id);
}
const held = new Set(attempts.map((a) => a.symbol_id));
const v = this.store.currentVersion();
const out = [];
for (const id of before) {
if (after.has(id) || held.has(id)) continue;
if (!this.store.get('SELECT 1 FROM symbol_addresses WHERE symbol_id = ? AND version_id = ?', id, v.id)) continue; // not in this build
const t = this.store.get('SELECT * FROM tasks WHERE symbol_id = ?', id);
if (t && ((t.state === 'leased' && t.lease_user === userId) || this.regions?.covers({ id: userId }, t))) continue;
try {
if (this.repo.git(['grep', '-l', '-w', '-e', `FUNCTION: ${id}`, 'HEAD', '--', '.', ...Object.keys(files).map((f) => `:(exclude)${f}`)]).trim()) continue;
} catch {
// not annotated anywhere else
}
out.push(id);
}
return out;
}
/** Functions the submission declares that nothing defines (undefined.js). */
undefinedGate(files, baseCommit) {
const submitted = Object.keys(files);
@ -412,6 +444,13 @@ export class Verifier {
if (!this.repo.isAncestor(batch.base_commit)) return finishAll('rejected', { summary: `base commit ${batch.base_commit} is not part of main; sync and resubmit` });
const textErrors = this.freeTextGate(submitted, batch.base_commit);
if (textErrors.length) return finishAll('rejected', { summary: 'rejected: the submission adds comments or strings that are not allowed (see lintErrors)', lintErrors: textErrors });
const dropped = this.droppedAnnotations(submitted, batch.base_commit, attempts, batch.user_id);
if (dropped.length) {
return finishAll('needs_cleanup', {
summary: `removes the annotation of ${dropped.join(', ')}, which this submission does not hold: functions of the original lose their match that way. Keep the annotation (and the code), or claim the function too`,
droppedAnnotations: dropped,
});
}
const undefinedNames = this.undefinedGate(submitted, batch.base_commit);
if (undefinedNames.length) {
return finishAll('needs_cleanup', {

View file

@ -35,6 +35,41 @@ export function loadProject(repoDir) {
return project;
}
/**
* The command shape a unit that is not in the build list yet gets: the one most units of its
* folder use (or of the nearest parent folder that has units), else the most common one.
*/
export function inferShape(project, file) {
const units = project.compile?.units;
if (!units) return null;
if (units[file] !== undefined) return units[file];
const tally = (prefix) => {
const n = new Map();
for (const [f, s] of Object.entries(units)) if (f.startsWith(prefix)) n.set(s, (n.get(s) ?? 0) + 1);
return [...n].sort((a, b) => b[1] - a[1] || a[0] - b[0])[0]?.[0];
};
const parts = file.split('/');
for (let i = parts.length - 1; i > 0; i--) {
const s = tally(parts.slice(0, i).join('/') + '/');
if (s !== undefined) return s;
}
return tally('') ?? 0;
}
/**
* The build list with new units added (each with its inferred shape), as the text the repository
* stores it in, or null when nothing is new. `text` is the list as it is on main.
*/
export function addUnitsToCompileCommands(project, text, files) {
if (!project.compile?.units || text === null) return null;
const doc = JSON.parse(text);
const added = files.filter((f) => isUnit(f) && doc.units[f] === undefined && !/(^|\/)Win32\//.test(f));
if (!added.length) return null;
for (const f of added) doc.units[f] = inferShape({ compile: doc }, f);
doc.units = Object.fromEntries(Object.entries(doc.units).sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)));
return { text: JSON.stringify(doc, null, 1), added, units: doc.units };
}
export function loadToolchainLock(repoDir) {
const file = path.join(repoDir, 'toolchain.lock.json');
return fs.existsSync(file) ? JSON.parse(fs.readFileSync(file, 'utf8')) : {};
@ -42,8 +77,9 @@ export function loadToolchainLock(repoDir) {
/** Compiler flags and toolchain for one translation unit. */
export function unitSettings(project, file) {
const shape = project.compile?.units?.[file];
if (shape !== undefined) return { cflags: project.compile.shapes[shape], toolchain: project.toolchain, env: project.compile.env ?? {} };
// A unit the build list does not have yet compiles the way it will once it is added (merge.js).
const shape = project.compile?.units ? inferShape(project, file) : undefined;
if (shape !== undefined && shape !== null) return { cflags: project.compile.shapes[shape], toolchain: project.toolchain, env: project.compile.env ?? {} };
let cflags = project.cflags;
let toolchain = project.toolchain;
for (const o of project.fileOverrides ?? []) {

18
test/units.test.js Normal file
View file

@ -0,0 +1,18 @@
import { test } from 'node:test';
import assert from 'node:assert/strict';
import { inferShape, addUnitsToCompileCommands } from '../src/shared/project.js';
test('new source files get the flags of their folder and join the build list', () => {
const compile = { shapes: [['/a'], ['/b']], units: { 'Engine/Gx/A.cpp': 1, 'Engine/Gx/B.cpp': 1, 'Engine/Base/C.cpp': 0, 'WoW/D.cpp': 0 } };
const project = { compile };
assert.equal(inferShape(project, 'Engine/Gx/New.cpp'), 1, 'its folder');
assert.equal(inferShape(project, 'Engine/Gx/Deep/New.cpp'), 1, 'the nearest folder with units');
assert.equal(inferShape(project, 'Tools/X.cpp'), 0, 'the most common one');
const text = JSON.stringify(compile, null, 1);
const grown = addUnitsToCompileCommands(project, text, ['Engine/Gx/New.cpp', 'Engine/Gx/A.cpp', 'Engine/Gx/x.h', 'WoW/Win32/Old.cpp']);
assert.deepEqual(grown.added, ['Engine/Gx/New.cpp'], 'headers, listed units and Win32 copies are left alone');
const doc = JSON.parse(grown.text);
assert.deepEqual(Object.keys(doc.units), ['Engine/Base/C.cpp', 'Engine/Gx/A.cpp', 'Engine/Gx/B.cpp', 'Engine/Gx/New.cpp', 'WoW/D.cpp']);
assert.equal(doc.units['Engine/Gx/New.cpp'], 1);
assert.equal(addUnitsToCompileCommands(project, text, ['Engine/Gx/A.cpp']), null);
});