trevato/forge

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

More
Forks1g fMapg aRuns14g rEventsFlows

Live triage gives an open issue with no pillar a pillar or Later within an hour #3451

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/3266-live-triage-gives-an-open-issue-with-no → main 3 commits fork

  • Merged. scout/3266-live-triage-gives-an-open-issue-with-no was merged into main as 48a95b7.

Review

Sign in to review this change.
4 files changed +375 −12
README.md +10
@@ -1065,6 +1065,16 @@ so its comment says "Closed by the crew's live triage". Reopening the issue
10651065 undoes it and the reopen stands: an issue that already has a close note is
10661066 never closed by triage again. The tick line counts them as `triaged=N`.
10671067
1068 **Live triage files issues.** On the same tick, an open issue an hour old
1069 that serves no pillar is filed: under the pillar its title, else its body,
1070 names by its short form (`P0`) or whole label (`P0 Trust`) — a bare word like
1071 "trust" does not count — otherwise behind the Later gate. Each gets a comment
1072 marked automatic. Directions, umbrellas and the gate are skipped, as is a
1073 repository with neither pillars nor a gate. To undo it, name a pillar (link
1074 its umbrella to wait for the issue) or unlink the gate; the comment stays, so
1075 triage does not file that issue again. The tick line counts `labelled=N`, and
1076 shows `triageMiss=N` for old issues it could not file (no gate to defer to).
1077
10681078 **A red pull.** The decider stops a red check only when it is at least 70%
10691079 sure; below that the pull gets another feedback run. A stop parks the pull: it
10701080 closes as *parked*, not done, with a comment that says why. The pull page's
src/crew/autonomy.ts +3 −3
@@ -50,7 +50,7 @@ import { certain } from "./decide.ts";
5050 import { decideAndRecord } from "./ledger.ts";
5151 import { sentinelTick } from "./sentinel.ts";
5252 import { triageTick } from "./triage.ts";
53 import { liveTriage, none as noTriage } from "./livetriage.ts";
53 import { liveTriage, none as noTriage, tickFields } from "./livetriage.ts";
5454 import { wallOf, wallSentence } from "../contract/policy.ts";
5555 import {
5656 getIssue,
@@ -1156,7 +1156,7 @@ export async function autonomyTick(
11561156 const requested = await moveRequested(app, current, freeSlots(app, current, now), now);
11571157 const did = await driveRepo(app, current, now);
11581158 // Rules only, no run: an issue noted already done on the base closes.
1159 const triaged = await liveTriage(app, current).catch((error) => {
1159 const triaged = await liveTriage(app, current, now).catch((error) => {
11601160 log.error("live triage failed", {
11611161 repo: `${repo.ownerName}/${repo.name}`,
11621162 error: String(error),
@@ -1178,7 +1178,7 @@ export async function autonomyTick(
11781178 log.info("tick", {
11791179 repo: `${repo.ownerName}/${repo.name}`,
11801180 did,
1181 triaged: triaged.closed,
1181 ...tickFields(triaged),
11821182 ready: ready.length,
11831183 // What the builders could start after the filters (#2143), and
11841184 // what holds the rest, per filter (#2244).
src/crew/livetriage.ts +142 −6
@@ -1,5 +1,14 @@
11 /**
2 * Live triage (#3242), first slice: an issue the crew found already done on
2 * Live triage (#3242). Two rules, no run, no spend.
3 *
4 * Filing (#3266): an open issue an hour old that serves no pillar gets one —
5 * the pillar its title, else its body, names — or else waits behind the
6 * Later gate, with a one-line comment saying so. A person undoes it by
7 * naming a pillar, or by unlinking the gate: the note stays, so it is not
8 * done twice. Directions, umbrellas and the gate itself are left alone, and
9 * a repository with no pillars and no gate has nothing to file under.
10 *
11 * Closing: an issue the crew found already done on
312 * the base branch stops waiting for a person to close it. Rules only, no
413 * run, no spend. Only the crew's own note (`ALREADY_NOTE`, read through
514 * `alreadyDone`) with every commit it names reachable from the default
@@ -13,11 +22,13 @@
1322 */
1423
1524 import type { App } from "../app.ts";
16 import { setIssueState } from "../actions/issues.ts";
25 import { linkIssue } from "../actions/dependencies.ts";
26 import { addComment, setIssueState } from "../actions/issues.ts";
1727 import { alreadyDone, CLOSED_NOTE, DONE_LIKE } from "../core/done_on_main.ts";
18 import { getIssueById } from "../core/issues.ts";
28 import { getIssueById, getIssuesByIds, type Issue } from "../core/issues.ts";
29 import { isStructural, pillarsOf, servedBy, type Pillar, type Pillars } from "../core/pillars.ts";
1930 import type { Repo } from "../core/repos.ts";
20 import { getUserById } from "../core/users.ts";
31 import { getUserById, type User } from "../core/users.ts";
2132 import { repoDir } from "../git/repos.ts";
2233 import { isMirror } from "../origins/write-through.ts";
2334 import { createLog } from "../kernel/log.ts";
@@ -31,9 +42,125 @@ export const MOST_CHANGES = 25;
3142 /** What a pass did. */
3243 export interface Triaged {
3344 closed: number;
45 /** Issues given a pillar or put behind the Later gate. */
46 labelled: number;
47 /** Open issues over an hour old that serve no pillar and that triage has yet to file. */
48 missed: number;
3449 }
3550
36 export const none = (): Triaged => ({ closed: 0 });
51 export const none = (): Triaged => ({ closed: 0, labelled: 0, missed: 0 });
52
53 /** How old an issue is before triage files it. */
54 export const FILE_AFTER_MS = 60 * 60 * 1000;
55
56 /** The start of the one-line comment a filing leaves. */
57 export const FILED_NOTE = "Live triage (automatic, not written by a person):";
58
59 const escapeRe = (text: string): string => text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
60
61 /**
62 * Whether `text` names this pillar by its short form (`P0`) or its whole
63 * label (`P0 Trust`). A bare name is not enough: names are common words.
64 */
65 function names(text: string, pillar: Pillar): boolean {
66 for (const word of [pillar.short, pillar.name])
67 if (new RegExp(`(?<![\\w])${escapeRe(word)}(?![\\w])`, "i").test(text)) return true;
68 return false;
69 }
70
71 /** The pillar the title names, else the body; the first in order wins. */
72 export function namedPillar(set: Pillars, issue: Issue): Pillar | undefined {
73 return (
74 set.pillars.find((p) => names(issue.title, p)) ??
75 set.pillars.find((p) => names(issue.body, p))
76 );
77 }
78
79 /** Ids of the repository's issues triage has already filed once. */
80 function filedIds(app: App, repo: Repo): Set<string> {
81 return new Set(
82 app.db
83 .query<{ id: string }, [string, string]>(
84 `SELECT DISTINCT c.issue_id AS id FROM comment c
85 JOIN issue i ON i.id = c.issue_id
86 WHERE i.repo_id = ?1 AND c.body LIKE ?2`,
87 )
88 .all(repo.id, `${FILED_NOTE}%`)
89 .map((r) => r.id),
90 );
91 }
92
93 /**
94 * Open issues old enough to file that serve nothing yet and that triage has
95 * not filed before, oldest first. One a person took back out stays out.
96 */
97 function unfiled(app: App, repo: Repo, set: Pillars, now: Date): Issue[] {
98 const before = new Date(now.getTime() - FILE_AFTER_MS).toISOString();
99 const ids = app.db
100 .query<{ id: string }, [string, string, string | null]>(
101 `SELECT i.id AS id FROM issue i
102 WHERE i.repo_id = ?1 AND i.kind = 'issue' AND i.state = 'open'
103 AND i.created_at <= ?2 AND (?3 IS NULL OR i.id != ?3)
104 ORDER BY i.number`,
105 )
106 .all(repo.id, before, repo.directionIssueId);
107 if (ids.length === 0) return [];
108 const served = servedBy(app.db, set);
109 const filed = filedIds(app, repo);
110 return [...getIssuesByIds(app.db, ids.map((r) => r.id)).values()]
111 .filter((i) => !served.has(i.id) && !isStructural(set, i.id) && !filed.has(i.id))
112 .sort((a, b) => a.number - b.number);
113 }
114
115 /**
116 * File old, unplaced issues; returns how many were, and how many were left
117 * unplaced. The link comes first: if the note then fails the issue is still
118 * placed, which is the part that matters, and the failure is logged.
119 */
120 function file(app: App, repo: Repo, by: User, now: Date): [number, number] {
121 const set = pillarsOf(app.db, repo);
122 if (set.pillars.length === 0 && set.gate === undefined) return [0, 0];
123 const todo = unfiled(app, repo, set, now);
124 let filed = 0;
125 for (const issue of todo) {
126 if (filed >= MOST_CHANGES) break;
127 const pillar = namedPillar(set, issue);
128 if (pillar === undefined && set.gate === undefined) continue;
129 try {
130 const linked =
131 pillar !== undefined
132 ? linkIssue(app, by, repo, pillar.umbrella, issue)
133 : linkIssue(app, by, repo, issue, set.gate!);
134 if (!linked.ok) continue;
135 filed++;
136 addComment(
137 app,
138 by,
139 repo,
140 issue,
141 pillar !== undefined
142 ? `${FILED_NOTE} filed under ${pillar.name}, which it names.`
143 : `${FILED_NOTE} no pillar named, so it waits behind the Later gate; name a pillar to move it.`,
144 );
145 } catch (error) {
146 log.error("filing failed", {
147 repo: `${repo.ownerName}/${repo.name}`,
148 issue: issue.number,
149 error: String(error),
150 });
151 }
152 }
153 return [filed, todo.length - filed];
154 }
155
156 /** What the tick line says of a pass: `triageMiss` only when something was missed. */
157 export function tickFields(did: Triaged): Record<string, number> {
158 return {
159 triaged: did.closed,
160 labelled: did.labelled,
161 ...(did.missed === 0 ? {} : { triageMiss: did.missed }),
162 };
163 }
37164
38165 /**
39166 * Open issues that carry an already-done note, oldest first. One that
@@ -56,12 +183,21 @@ function candidates(app: App, repo: Repo): string[] {
56183 }
57184
58185 /** One repository's pass. */
59 export async function liveTriage(app: App, repo: Repo): Promise<Triaged> {
186 export async function liveTriage(
187 app: App,
188 repo: Repo,
189 now = new Date(),
190 ): Promise<Triaged> {
60191 const did = none();
61192 if (repo.archivedAt !== null || repo.autonomyBy === null) return did;
62193 if (isMirror(app, repo)) return did;
63194 const by = getUserById(app.db, repo.autonomyBy);
64195 if (!by) return did;
196 try {
197 [did.labelled, did.missed] = file(app, repo, by, now);
198 } catch (error) {
199 log.error("filing failed", { repo: `${repo.ownerName}/${repo.name}`, error: String(error) });
200 }
65201 const ids = candidates(app, repo);
66202 const noted = alreadyDone(app.db, ids);
67203 if (noted.size === 0) return did;
test/livetriage.test.ts +220 −3
@@ -9,11 +9,11 @@ import { join } from "node:path";
99 import { createApp, type App } from "../src/app.ts";
1010 import { addComment, openIssue, setIssueState } from "../src/actions/issues.ts";
1111 import { commitToBranch, createRepo, updateRepoSettings } from "../src/actions/repos.ts";
12 import { getIssueById, listComments, type Issue } from "../src/core/issues.ts";
12 import { getIssueById, labelsFor, listComments, type Issue } from "../src/core/issues.ts";
1313 import { getRepoById, type Repo } from "../src/core/repos.ts";
1414 import { createUser, type User } from "../src/core/users.ts";
1515 import { ALREADY_NOTE } from "../src/crew/already.ts";
16 import { liveTriage, MOST_CHANGES } from "../src/crew/livetriage.ts";
16 import { liveTriage, MOST_CHANGES, FILE_AFTER_MS, tickFields } from "../src/crew/livetriage.ts";
1717
1818 describe("live triage", () => {
1919 let dir: string;
@@ -77,7 +77,7 @@ describe("live triage", () => {
7777 const before = closedEvents();
7878
7979 const first = await liveTriage(app, fresh());
80 expect(first).toEqual({ closed: 1 });
80 expect(first.closed).toBe(1);
8181 expect(state(done).state).toBe("closed");
8282 expect(state(done).closeReason).toBe("done");
8383 expect(listComments(app.db, done.id).some((c) => c.body.includes(oid.slice(0, 7)))).toBe(true);
@@ -148,3 +148,220 @@ describe("live triage", () => {
148148 expect((await liveTriage(app, fresh())).closed).toBe(3);
149149 });
150150 });
151
152 describe("live triage files unplaced issues", () => {
153 let dir: string;
154 let app: App;
155 let alice: User;
156 let repo: Repo;
157 let umbrella: Issue;
158 let gate: Issue;
159 const fresh = () => getRepoById(app.db, repo.id)!;
160 const open = (title: string, body = "", age = 2 * FILE_AFTER_MS): Issue => {
161 const opened = openIssue(app, alice, fresh(), { title, body });
162 if (!opened.ok) throw new Error(String(opened.error));
163 app.db
164 .query(`UPDATE issue SET created_at = ?2 WHERE id = ?1`)
165 .run(opened.value.id, new Date(Date.now() - age).toISOString());
166 return getIssueById(app.db, opened.value.id)!;
167 };
168 const waitsFor = (a: Issue, b: Issue) =>
169 app.db
170 .query<{ n: number }, [string, string]>(
171 `SELECT COUNT(*) AS n FROM issue_dependency WHERE issue_id = ?1 AND blocked_by_id = ?2`,
172 )
173 .get(a.id, b.id)!.n === 1;
174 const link = (placed: Issue) =>
175 app.db
176 .query(`INSERT INTO issue_dependency (issue_id, blocked_by_id) VALUES (?1, ?2)`)
177 .run(umbrella.id, placed.id);
178 const notes = (on: Issue) => listComments(app.db, on.id).filter((c) => c.body.startsWith("Live triage (automatic"));
179
180 beforeAll(async () => {
181 dir = await mkdtemp(join(tmpdir(), "forge-livetriage-file-"));
182 app = createApp({ dataDir: dir, port: 0 });
183 const a = await createUser(app.db, {
184 origin: "seed",
185 trusted: true,
186 username: "alice",
187 email: "[email protected]",
188 password: "a-good-passphrase",
189 adminIfFirst: true,
190 });
191 if (!a.ok) throw new Error(a.error);
192 alice = a.value;
193 const made = await createRepo(app, alice, { name: "engine", visibility: "private" });
194 if (!made.ok) throw new Error(String(made.error));
195 repo = made.value;
196 const set = await updateRepoSettings(app, alice, repo, {
197 autonomy: "autonomous",
198 autonomyDirection: "Keep it small.",
199 autonomyCeilingUsd: 100,
200 autonomyBy: alice.id,
201 });
202 if (!set.ok) throw new Error(JSON.stringify(set.error));
203 umbrella = open("P0 Trust: be safe");
204 gate = open("Later gate: not yet");
205 });
206 afterAll(async () => {
207 app.close();
208 await rm(dir, { recursive: true, force: true });
209 });
210
211 test("an issue whose title names a pillar is filed under it, and counted", async () => {
212 const work = open("Harden P0 sessions");
213 const did = await liveTriage(app, fresh());
214 expect(did.labelled).toBe(1);
215 expect(waitsFor(umbrella, work)).toBe(true);
216 expect(labelsFor(app.db, work.id).map((l) => l.name)).toContain("P0 Trust");
217 expect(notes(work)).toHaveLength(1);
218 });
219
220 test("a pillar named only in the body files it", async () => {
221 const work = open("Sessions", "This serves p0 trust.");
222 await liveTriage(app, fresh());
223 expect(waitsFor(umbrella, work)).toBe(true);
224 });
225
226 test("the title wins when title and body name different pillars", async () => {
227 const other = open("P1 Speed: be quick");
228 const work = open("Cache P1 pages", "Also touches P0.");
229 await liveTriage(app, fresh());
230 expect(waitsFor(other, work)).toBe(true);
231 expect(waitsFor(umbrella, work)).toBe(false);
232 });
233
234 test("a bare pillar name in passing does not file it: it goes to Later", async () => {
235 const work = open("Tell the story of trust", "A story about trust, in passing.");
236 await liveTriage(app, fresh());
237 expect(waitsFor(umbrella, work)).toBe(false);
238 expect(waitsFor(work, gate)).toBe(true);
239 });
240
241 test("an issue naming none goes behind the Later gate with a comment, once", async () => {
242 const work = open("Add a colour picker");
243 await liveTriage(app, fresh());
244 expect(waitsFor(work, gate)).toBe(true);
245 expect(labelsFor(app.db, work.id).map((l) => l.name)).toContain("Later");
246 expect(notes(work)[0]!.body).toContain("Later gate");
247 const again = await liveTriage(app, fresh());
248 expect(again.labelled).toBe(0);
249 expect(again.missed).toBe(0);
250 expect(notes(work)).toHaveLength(1);
251 });
252
253 test("an issue under an hour old is left, and a placed one is not moved", async () => {
254 const young = open("Brand new idea", "", 60_000);
255 const placed = open("Already placed");
256 link(placed);
257 await liveTriage(app, fresh());
258 expect(waitsFor(young, gate)).toBe(false);
259 expect(notes(young)).toHaveLength(0);
260 expect(notes(placed)).toHaveLength(0);
261 expect(waitsFor(umbrella, placed)).toBe(true);
262 expect(waitsFor(placed, gate)).toBe(false);
263 });
264
265 test("directions, umbrellas and the gate are skipped", async () => {
266 const direction = open("Direction: keep it small");
267 app.db.query(`UPDATE repo SET direction_issue_id = ?2 WHERE id = ?1`).run(repo.id, direction.id);
268 await liveTriage(app, fresh());
269 for (const furniture of [direction, umbrella, gate]) expect(notes(furniture)).toHaveLength(0);
270 expect(waitsFor(direction, gate)).toBe(false);
271 expect(waitsFor(gate, gate)).toBe(false);
272 });
273
274 test("a person who unlinks the gate is not overruled", async () => {
275 const work = open("Needs thought");
276 await liveTriage(app, fresh());
277 app.db.query(`DELETE FROM issue_dependency WHERE issue_id = ?1`).run(work.id);
278 const did = await liveTriage(app, fresh());
279 expect(waitsFor(work, gate)).toBe(false);
280 expect(did.labelled).toBe(0);
281 expect(did.missed).toBe(0);
282 });
283 });
284
285 describe("live triage where there is little to file under", () => {
286 let dir: string;
287 let app: App;
288 let alice: User;
289 beforeAll(async () => {
290 dir = await mkdtemp(join(tmpdir(), "forge-livetriage-bare-"));
291 app = createApp({ dataDir: dir, port: 0 });
292 const a = await createUser(app.db, {
293 origin: "seed",
294 trusted: true,
295 username: "alice",
296 email: "[email protected]",
297 password: "a-good-passphrase",
298 adminIfFirst: true,
299 });
300 if (!a.ok) throw new Error(a.error);
301 alice = a.value;
302 });
303 afterAll(async () => {
304 app.close();
305 await rm(dir, { recursive: true, force: true });
306 });
307
308 /** A repository with the given furniture; `count` old issues naming no pillar. */
309 const world = async (name: string, furniture: string[], count: number) => {
310 const made = await createRepo(app, alice, { name, visibility: "private" });
311 if (!made.ok) throw new Error(String(made.error));
312 const set = await updateRepoSettings(app, alice, made.value, {
313 autonomy: "autonomous",
314 autonomyDirection: "Keep it small.",
315 autonomyCeilingUsd: 100,
316 autonomyBy: alice.id,
317 });
318 if (!set.ok) throw new Error(JSON.stringify(set.error));
319 const repo = () => getRepoById(app.db, made.value.id)!;
320 const old = new Date(Date.now() - 2 * FILE_AFTER_MS).toISOString();
321 const open = (title: string): Issue => {
322 const o = openIssue(app, alice, repo(), { title });
323 if (!o.ok) throw new Error(String(o.error));
324 app.db.query(`UPDATE issue SET created_at = ?2 WHERE id = ?1`).run(o.value.id, old);
325 return getIssueById(app.db, o.value.id)!;
326 };
327 furniture.forEach(open);
328 const work = Array.from({ length: count }, (_, n) => open(`Plain work ${n}`));
329 return { repo, work };
330 };
331 const comments = (on: Issue) => listComments(app.db, on.id).length;
332
333 test("pillars but no gate: an unnamed issue is left alone and counted as missed", async () => {
334 const w = await world("nogate", ["P0 Trust: be safe"], 1);
335 const did = await liveTriage(app, w.repo());
336 expect(did.labelled).toBe(0);
337 expect(did.missed).toBe(1);
338 expect(comments(w.work[0]!)).toBe(0);
339 expect(labelsFor(app.db, w.work[0]!.id)).toHaveLength(0);
340 expect(app.db.query<{ n: number }, []>(`SELECT COUNT(*) AS n FROM issue_dependency`).get()!.n).toBe(0);
341 });
342
343 test("neither pillars nor gate: nothing is filed, counted or written", async () => {
344 const w = await world("plain", [], 2);
345 expect(await liveTriage(app, w.repo())).toEqual({ closed: 0, labelled: 0, missed: 0 });
346 for (const issue of w.work) expect(comments(issue)).toBe(0);
347 });
348
349 test("at most 25 issues are filed per tick; the rest are missed, then filed", async () => {
350 const w = await world("many", ["Later gate: not yet"], MOST_CHANGES + 1);
351 const first = await liveTriage(app, w.repo());
352 expect(first.labelled).toBe(MOST_CHANGES);
353 expect(first.missed).toBe(1);
354 const second = await liveTriage(app, w.repo());
355 expect(second.labelled).toBe(1);
356 expect(second.missed).toBe(0);
357 });
358
359 test("the tick line carries labelled always and triageMiss only when something was missed", () => {
360 expect(tickFields({ closed: 2, labelled: 3, missed: 0 })).toEqual({ triaged: 2, labelled: 3 });
361 expect(tickFields({ closed: 0, labelled: 0, missed: 4 })).toEqual({
362 triaged: 0,
363 labelled: 0,
364 triageMiss: 4,
365 });
366 });
367 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs