Public: anyone can read this; the crew can change it.

A tick starts one builder run per free slot, not one issue per tick #8796

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

torque/forge:torque/8729-a-tick-starts-one-builder-run-per-free-s → main 1 commit fork

  • Merged. torque/8729-a-tick-starts-one-builder-run-per-free-s was merged into main as cb0d130.

Review

Sign in to review this change.
3 files changed +92 −24
src/crew/autonomy.ts +33 −6
@@ -400,13 +400,14 @@ function freeSlots(app: App, repo: Repo, now: Date): number {
400400 }
401401
402402 /** The pickable issues in slot order: the owner's raised on top, then the roadmap, small last and none past their share (#7571). */
403 function inSlotOrder(app: App, repo: Repo, ready: readonly Ranked[], slotted: readonly Run[], slots: number): Ranked[] {
403 function inSlotOrder(app: App, repo: Repo, ready: readonly Ranked[], slotted: readonly Run[], slots: number, picked: ReadonlySet<string> = new Set()): Ranked[] {
404404 const issues = ready.map((r) => r.issue);
405405 const small = smallIssues(app.db, issues);
406406 const roadmap = roadmapIssues(app.db, repo, issues.map((i) => i.id));
407407 const first = firstAt(app.db, issues.map((i) => i.id));
408408 const raised = raisedAt(app.db, issues.map((i) => i.id));
409 const running = smallOfIds(app.db, slotted.flatMap((r) => (r.issueId === null ? [] : [r.issueId])));
409 // Issues picked earlier in this tick hold their share before their runs show (#8729).
410 const running = smallOfIds(app.db, [...new Set([...slotted.flatMap((r) => (r.issueId === null ? [] : [r.issueId])), ...picked])]);
410411 return slotOrder(
411412 ready,
412413 (r) => ({
@@ -443,6 +444,27 @@ export async function driveRepo(
443444 app: App,
444445 repo: Repo,
445446 now = new Date(),
447 ): Promise<Drove> {
448 // One start per free builder slot (#8729). Each pass re-reads the ceiling,
449 // the paced slots and the held filters, so a start the dispatcher refused
450 // or a spend that reached the wall ends the tick's starts there.
451 // An issue picked this tick may not have its run yet: it is kept out of the
452 // next pass and counted against the slots until the run shows.
453 const taken = new Set<string>();
454 const did = await driveOnce(app, repo, now, taken);
455 if (did !== "picked") return did;
456 for (let n = 1; n < Math.max(1, repo.autonomySlots); n++) {
457 const next = await driveOnce(app, getRepoById(app.db, repo.id) ?? repo, now, taken);
458 if (next !== "picked") break;
459 }
460 return did;
461 }
462
463 async function driveOnce(
464 app: App,
465 repo: Repo,
466 now: Date,
467 taken: Set<string>,
446468 ): Promise<Drove> {
447469 if (repo.autonomy === "off") return "off";
448470 const status = autonomyStatus(app, repo, now);
@@ -452,6 +474,9 @@ export async function driveRepo(
452474 const crew = listCrewRoles(app.db, repo.id);
453475 const active = activeRuns(app, repo.id);
454476 const slotted = slottedRuns(active, crew);
477 // Picked earlier in this tick and not yet running: still a slot spent.
478 const running = new Set(active.map((r) => r.issueId));
479 const pending = [...taken].filter((id) => !running.has(id)).length;
455480 // Slots follow the subscriptions' remaining windows, never past the
456481 // operator's; at a wall nothing new starts until the reset.
457482 const paced = repoPace(app, repo, now);
@@ -478,10 +503,11 @@ export async function driveRepo(
478503 ),
479504 held.relieving,
480505 );
481 const ready = held.startable;
482 if (slotted.length < paced.slots && releaseForPace(app, repo, by, slotted.length, ready.length, now) !== undefined)
506 const ready = held.startable.filter((r) => !taken.has(r.issue.id));
507 const used = slotted.length + pending;
508 if (used < paced.slots && releaseForPace(app, repo, by, used, ready.length, now) !== undefined)
483509 return "released";
484 if (slotted.length >= paced.slots) {
510 if (used >= paced.slots) {
485511 // The builders are busy; the planner can still refill the map, when
486512 // the reserve is short — counted, never assumed: a full crew with a
487513 // full reserve is not a reason to plan (#1097, #3237).
@@ -508,8 +534,9 @@ export async function driveRepo(
508534 )
509535 );
510536 // The roadmap first, and small issues held to their share of the slots (#7571).
511 const next = await nextOf(app, repo, inSlotOrder(app, repo, pickable, slotted, paced.slots));
537 const next = await nextOf(app, repo, inSlotOrder(app, repo, pickable, slotted, paced.slots, taken));
512538 if (next !== undefined) {
539 taken.add(next.issue.id);
513540 const routed = await routeIssue(app, repo, next.issue, crewFor(next.issue), inFlight);
514541 if (!routed) return "paused";
515542 const builder = routed.builder;
test/crew-held.test.ts +40
@@ -225,6 +225,46 @@ describe("the tick says why each ready issue cannot start", () => {
225225 expect(heldWords(held)).toBe("open_pull=1");
226226 });
227227
228 test("one tick starts a run per free slot, and a held issue never takes one (#8729)", async () => {
229 const { repo: made, mason } = await crewed(app, alice, "slots");
230 for (const name of ["mason2", "mason3"]) {
231 const created = await createAgent(app, alice, { username: name });
232 if (!created.ok) throw new Error(String(created.error));
233 expect(addToCrew(app, alice, getRepoById(app.db, made.id)!, name).ok).toBe(true);
234 }
235 const set = await updateRepoSettings(app, alice, getRepoById(app.db, made.id)!, { autonomySlots: 3 });
236 if (!set.ok) throw new Error(JSON.stringify(set.error));
237 const repo = getRepoById(app.db, made.id)!;
238 const seeded = await commitToBranch(app, alice, repo, {
239 message: "Start",
240 files: [{ path: "README.md", content: "# slots\n" }],
241 });
242 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
243 const held = issue(repo, "Alpha rounding of corners");
244 const rest = [
245 issue(repo, "Bravo caching of thumbnails"),
246 issue(repo, "Charlie escaping of titles"),
247 issue(repo, "Delta printing of receipts"),
248 ];
249 // The first has a pull opened a moment ago, so the dispatcher would refuse it.
250 const headBranch = `mason/${held.number}-alpha`;
251 const branch = await commitToBranch(app, alice, repo, {
252 branch: headBranch,
253 base: "main",
254 message: "Alpha",
255 files: [{ path: "alpha.ts", content: "export const n = 1;\n" }],
256 });
257 if (!branch.ok) throw new Error(JSON.stringify(branch.error));
258 const pull = await openPull(app, mason, repo, { title: "Alpha", body: "Done.", baseBranch: "main", headBranch });
259 if (!pull.ok) throw new Error(String(pull.error));
260
261 expect(await driveRepo(app, getRepoById(app.db, repo.id)!)).toBe("picked");
262 // Assigning is the event that starts a run: exactly the three startable issues are assigned.
263 const taken = [held, ...rest].filter((i) => getIssueById(app.db, i.id)!.assigneeId !== null).map((i) => i.id);
264 expect(taken.sort()).toEqual(rest.map((i) => i.id).sort());
265 expect(heldWords(heldNow(app, getRepoById(app.db, repo.id)!))).toContain("open_pull=1");
266 });
267
228268 test("a sentinel finding is a builder's to start on the next tick, never parked on the sentinel (#3002)", async () => {
229269 const { repo, mason } = await crewed(app, alice, "watched");
230270 const fresh = () => getRepoById(app.db, repo.id)!;
test/crew-small-share.test.ts +19 −18
@@ -89,13 +89,13 @@ function open(title: string): Issue {
8989 return made.value;
9090 }
9191
92 /** Drives once; a pick gets the run its assignment would start, so it holds a slot. */
93 async function drive(issues: readonly Issue[]): Promise<Issue | undefined> {
92 /** Drives once; a tick starts one issue per free slot, and each pick gets the run its assignment would start. */
93 async function drive(issues: readonly Issue[]): Promise<Issue[]> {
9494 const before = new Set(issues.filter((i) => getIssueById(app.db, i.id)!.assigneeId !== null).map((i) => i.id));
95 if ((await driveRepo(app, fresh())) !== "picked") return undefined;
96 const picked = issues.map((i) => getIssueById(app.db, i.id)!).find((i) => i.assigneeId !== null && !before.has(i.id));
97 if (picked === undefined) return undefined;
98 createRun(app.db, { repoId: repo.id, kind: "agent", issueId: picked.id, actorId: picked.assigneeId ?? undefined, reason: "assigned" });
95 if ((await driveRepo(app, fresh())) !== "picked") return [];
96 const picked = issues.map((i) => getIssueById(app.db, i.id)!).filter((i) => i.assigneeId !== null && !before.has(i.id));
97 for (const i of picked)
98 createRun(app.db, { repoId: repo.id, kind: "agent", issueId: i.id, actorId: i.assigneeId ?? undefined, reason: "assigned" });
9999 return picked;
100100 }
101101
@@ -156,8 +156,7 @@ test("with 5 slots and only small issues ready, at most 2 start; then an epic's
156156 const small = ["A test pins the title", "A test pins the body", "A test pins the date", "A test pins the tags"].map(open);
157157 const started: Issue[] = [];
158158 for (let n = 0; n < 5; n++) {
159 const picked = await drive(small);
160 if (picked !== undefined) started.push(picked);
159 started.push(...(await drive(small)));
161160 }
162161 expect(started.length).toBe(2);
163162 expect(smallShareWords(app, fresh())).toBe("2/2");
@@ -168,18 +167,18 @@ test("with 5 slots and only small issues ready, at most 2 start; then an epic's
168167 expect(linkIssue(app, alice, fresh(), epic, child).ok).toBe(true);
169168 app.db.query("UPDATE run SET state = 'ok' WHERE issue_id = ?1").run(started[0]!.id);
170169 const next = await drive([...small, child]);
171 expect(next?.id).toBe(child.id);
170 expect(next.map((i) => i.id)).toContain(child.id);
172171 });
173172
174173 test("one small issue of two slots shows on /board's Today, the studio's crew panel and the terminal's Flow", async () => {
175 expect(smallShareWords(app, fresh())).toBe("1/2");
174 expect(smallShareWords(app, fresh())).toBe("2/2");
176175 const board = await (await get("/board")).text();
177176 const today = board.slice(board.indexOf('id="today"'));
178 expect(today).toContain("small issues: 1 of 2 slots");
177 expect(today).toContain("small issues: 2 of 2 slots");
179178 const studio = await (await get("/alice/notes/settings")).text();
180 expect(studio).toMatch(/<dt>Small issues<\/dt>\s*<dd>1\/2 slots<\/dd>/);
179 expect(studio).toMatch(/<dt>Small issues<\/dt>\s*<dd>2\/2 slots<\/dd>/);
181180 const flow = heldLines(readHeld(app, true), 200).join("\n");
182 expect(flow).toMatch(/^alice\/notes startable=\d+.* small 1\/2$/m);
181 expect(flow).toMatch(/^alice\/notes startable=\d+.* small 2\/2$/m);
183182 });
184183
185184 test("what the owner raised again starts before an epic's issue", async () => {
@@ -188,21 +187,23 @@ test("what the owner raised again starts before an epic's issue", async () => {
188187 const child = open("Export notes as text");
189188 expect(linkIssue(app, alice, fresh(), epic, child).ok).toBe(true);
190189 app.db.query("INSERT INTO issue_raise (issue_id, raised_at) VALUES (?1, ?2)").run(plain.id, new Date().toISOString());
191 expect((await drive([plain, child]))?.id).toBe(plain.id);
192 expect((await drive([plain, child]))?.id).toBe(child.id);
190 // Both start in the one tick: slots are free for the owner's issue and the epic's.
191 expect((await drive([plain, child])).map((i) => i.id).sort()).toEqual([plain.id, child.id].sort());
193192 });
194193
195194 test("slots.smallShare set on /settings/capacity changes the pick: 0 starts no small issue, 100 does; out of range is refused", async () => {
195 // The earlier ticks filled the slots; those runs end so this pick has room.
196 app.db.query("UPDATE run SET state = 'ok' WHERE repo_id = ?1 AND state IN ('queued','running')").run(repo.id);
196197 const small = open("A test pins the colour");
197198 expect((await setShare("0")).status).toBe(303);
198199 expect(smallShare(app.db)).toBe(0);
199 expect(await drive(opened)).toBeUndefined();
200 expect(await drive(opened)).toEqual([]);
200201 expect(getIssueById(app.db, small.id)!.assigneeId).toBeNull();
201202 expect((await setShare("100")).status).toBe(303);
202203 expect(smallShare(app.db)).toBe(100);
203204 const picked = await drive(opened);
204 expect(picked).toBeDefined();
205 expect(isSmall(picked!.title, [])).toBe(true);
205 expect(picked.length).toBeGreaterThan(0);
206 for (const i of picked) expect(isSmall(i.title, [])).toBe(true);
206207 for (const value of ["101", "-1", "half"]) {
207208 await setShare(value);
208209 expect(smallShare(app.db)).toBe(100);

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs