feat(usage): count hand-backs on merged PRs from earlier bodies

quest/m0/routing-evidence/README.md

@@ -1,4 +1,4 @@
-# Routing evidence
+# [L] Routing evidence
 
 ## Goal
 

quest/m1/model-check.md

@@ -71,9 +71,7 @@ checkpoints and outages to count by hand:
     on its own chart. The measures are first-round findings per PR
     (`firstRounds.findings` over `firstRounds.pulls`), review rounds per
     reviewed PR (`tally.rounds / reviewed`), hand-backs per PR
-    (`handedBack / pulls`, which counts only drafts still open until
-    [hand-backs on merged PRs](/quest/m0/routing-evidence/handbacks.md)
-    lands), and later
+    (`handedBack / pulls`, merged PRs included), and later
     `## Caused by` escapes per merged PR (`fixes / merged`).
   - Local reviewers, one point per `reviewers` row (PR size, harness,
     model, effort, shadow): cost per PR reviewed, `spent.cost /

src/cli/usage.ts

@@ -77,7 +77,7 @@ export interface PullRow {
 	repository: string | null;
 	state: "open" | "closed" | "merged";
 	draft: boolean;
-	/** Its body has a `## Waiting` section, so it was handed back to the maintainer. */
+	/** Its body, as a draft now or any earlier version, had a `## Waiting` section, so it was handed back to the maintainer. */
 	handedBack: boolean;
 	size: Size | null;
 	implementers: { harness: string; model: string; effort: string; size: Size | null; arm: Arm | null }[];
@@ -1007,7 +1007,12 @@ export async function usage(root: string, options: UsageOptions, deps: UsageDepe
 		repository,
 		rows.map((row) => row.number),
 	);
-	for (const row of rows) row.online = online.get(row.number) ?? [];
+	for (const row of rows) {
+		const read = online.get(row.number);
+		row.online = read?.online ?? [];
+		// `quest ready` and hand edits drop the section before merge, so an earlier body counts too.
+		if (read?.bodies.some(handedBack)) row.handedBack = true;
+	}
 	return {
 		repository: key(repository),
 		since: options.since,

src/github/online-review.ts

@@ -60,32 +60,43 @@ const FIELDS = {
 		"id isResolved resolvedBy { __typename login } " +
 		`comments(first: 100) { pageInfo { hasNextPage endCursor } nodes { ${THREAD_COMMENT} } }`,
 	comments: "author { __typename login } authorAssociation body createdAt url",
+	// Each edit's `diff` is the whole body as that edit left it, so the earliest one is the body the PR opened with.
+	userContentEdits: "diff",
 } as const;
 type Connection = keyof typeof FIELDS;
-const CONNECTIONS = Object.keys(FIELDS) as Connection[];
+/** The connections every read brings; a caller adds `userContentEdits` when it needs the PR's earlier bodies. */
+const CONNECTIONS: Connection[] = ["reviews", "reviewThreads", "comments"];
 const page = (name: Connection, after: string) =>
 	`${name}(first: ${name === "reviewThreads" ? 50 : 100}${after}) { pageInfo { hasNextPage endCursor } nodes { ${FIELDS[name]} } }`;
 
-/** PRs read per query; each brings at most 100 reviews, 50 threads of 100 comments, and 100 comments. */
+/** PRs read per query; each brings at most 100 reviews, 50 threads of 100 comments, 100 comments, and 100 body edits when asked. */
 const BATCH = 10;
 
 /**
  * A PR's last head and its committer date, and its reviews, review threads, and comments as GitHub's GraphQL API gives
  * them; `moved` when a later page found a newer head, so the pages may mix two heads' activity.
  */
-export type Activity = Record<Connection, ObjectValue[]> & { head: string; committed: string; moved?: boolean };
+export type Activity = Record<Exclude<Connection, "userContentEdits">, ObjectValue[]> & {
+	/** The PR's body edits, read only when asked. */
+	userContentEdits?: ObjectValue[];
+	head: string;
+	committed: string;
+	moved?: boolean;
+};
 
 /** Each PR's reviews, review threads, and comments, and its head, read in batches and then in pages. */
 export async function readActivity(
 	client: GitHub,
 	repository: Repository,
 	numbers: number[],
+	extra: "userContentEdits"[] = [],
 ): Promise<Map<number, Activity>> {
+	const connections = [...CONNECTIONS, ...extra];
 	const pulls = new Map<number, Activity>();
 	const more: { number: number; cursors: Partial<Record<Connection, string>> }[] = [];
 	for (let start = 0; start < numbers.length; start += BATCH) {
 		const batch = numbers.slice(start, start + BATCH);
-		const fields = `headRefOid commits(last: 1) { nodes { commit { oid committedDate } } } ${CONNECTIONS.map((name) => page(name, "")).join(" ")}`;
+		const fields = `headRefOid commits(last: 1) { nodes { commit { oid committedDate } } } ${connections.map((name) => page(name, "")).join(" ")}`;
 		const data = await client.graphql(
 			`query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { ${batch
 				.map((number) => `p${number}: pullRequest(number: ${number}) { ${fields} }`)
@@ -103,7 +114,7 @@ export async function readActivity(
 				throw new Error(`the head of PR #${number} changed while it was read; retry`);
 			const nodes = { head: String(pull.headRefOid), committed: String(commit.committedDate) } as Activity;
 			const cursors: Partial<Record<Connection, string>> = {};
-			for (const name of CONNECTIONS) {
+			for (const name of connections) {
 				const connection = object(pull[name]);
 				nodes[name] = array(connection.nodes);
 				const info = object(connection.pageInfo);
@@ -116,7 +127,7 @@ export async function readActivity(
 	for (const { number, cursors } of more) {
 		const nodes = pulls.get(number)!;
 		for (let left = cursors; Object.keys(left).length;) {
-			const names = CONNECTIONS.filter((name) => left[name] !== undefined);
+			const names = connections.filter((name) => left[name] !== undefined);
 			const data = await client.graphql(
 				`query($owner: String!, $name: String!, $number: Int!, ${names.map((name) => `$${name}: String!`).join(", ")}) { ` +
 					`repository(owner: $owner, name: $name) { pullRequest(number: $number) { headRefOid ${names
@@ -130,7 +141,7 @@ export async function readActivity(
 			const next: Partial<Record<Connection, string>> = {};
 			for (const name of names) {
 				const connection = object(pull[name]);
-				nodes[name].push(...array(connection.nodes));
+				nodes[name]!.push(...array(connection.nodes));
 				const info = object(connection.pageInfo);
 				if (info.hasNextPage === true) next[name] = String(info.endCursor);
 			}
@@ -288,12 +299,22 @@ export function onlineReviews(pull: Activity): OnlineReview[] {
 	return [...records.values()].sort((a, b) => a.reviewer.localeCompare(b.reviewer));
 }
 
-/** Each PR's online reviews, by number. */
+/** Each PR's online reviews, and every body its edits left, by number. */
 export async function readOnlineReviews(
 	client: GitHub,
 	repository: Repository,
 	numbers: number[],
-): Promise<Map<number, OnlineReview[]>> {
-	const pulls = await readActivity(client, repository, numbers);
-	return new Map([...pulls].map(([number, pull]) => [number, onlineReviews(pull)]));
+): Promise<Map<number, { online: OnlineReview[]; bodies: string[] }>> {
+	const pulls = await readActivity(client, repository, numbers, ["userContentEdits"]);
+	return new Map(
+		[...pulls].map(([number, pull]) => [
+			number,
+			{
+				online: onlineReviews(pull),
+				bodies: (pull.userContentEdits ?? []).flatMap((edit) =>
+					typeof edit.diff === "string" ? [edit.diff] : [],
+				),
+			},
+		]),
+	);
 }

test/usage.test.ts

@@ -695,6 +695,10 @@ describe("usage", () => {
 					: [],
 			),
 			comments: connection([]),
+			// #10 was handed back as a draft before `quest ready` dropped its `## Waiting` section.
+			userContentEdits: connection(
+				number === 10 ? [{ diff: "Body.\n\n## Waiting\n\n- Maintainer: pick one\n" }, { diff: null }] : [],
+			),
 		});
 		const fetch = async (input: string | URL | Request, init?: RequestInit) => {
 			const url = new URL(String(input));
@@ -825,6 +829,9 @@ describe("usage", () => {
 		// #10's writing spend: the iterate's Sol session to its own line though the efforts differ, and the Haiku
 		// sub-agent, which no line names, to the PR's main writer; the risk classifier's session is not writing. Its
 		// other reviewers record no first round, so no PR counts toward the writers' first rounds.
+		// Only #10's earlier body hands it back, which counts for both of its writers.
+		expect(report.pulls.filter((p) => p.handedBack).map((p) => p.number)).toEqual([10]);
+		expect(report.accuracy.map((a) => a.handedBack)).toEqual([1, 1]);
 		expect(report.accuracy.map((a) => [a.spent.pulls, a.spent.cost, a.firstRounds.pulls])).toEqual([
 			[1, 2, 0],
 			[1, 5.5, 0],