From 4b3205ad82fbe93685b49b88d9d511d22e87f461 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 13 Nov 2026 18:08:34 -0800 Subject: [PATCH 1/2] chore(quest): claim quest/m0/routing-evidence/handbacks.md Quest-Claim: f9dd299db267b5d9 From 4d26d19e5fe9c85e0353ab6c6b7cc66270a13416 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 13 Nov 2026 18:11:10 -0800 Subject: [PATCH 2/2] feat(usage): count hand-backs from a PR's earlier bodies Co-Authored-By: Claude Opus 5.5 --- quest/m0/routing-evidence/README.md | 2 +- quest/m0/routing-evidence/handbacks.md | 35 --------------------- quest/m1/model-check.md | 4 +-- src/cli/usage.ts | 9 ++++-- src/github/online-review.ts | 43 +++++++++++++++++++------- test/usage.test.ts | 7 +++++ 6 files changed, 48 insertions(+), 52 deletions(-) delete mode 100644 quest/m0/routing-evidence/handbacks.md diff --git a/quest/m0/routing-evidence/README.md b/quest/m0/routing-evidence/README.md index 91cf031f..9ecdbe7b 100644 --- a/quest/m0/routing-evidence/README.md +++ b/quest/m0/routing-evidence/README.md @@ -1,4 +1,4 @@ -# Routing evidence +# [L] Routing evidence ## Goal diff --git a/quest/m0/routing-evidence/handbacks.md b/quest/m0/routing-evidence/handbacks.md deleted file mode 100644 index d9e0f14b..00000000 --- a/quest/m0/routing-evidence/handbacks.md +++ /dev/null @@ -1,35 +0,0 @@ -# [S] Count hand-backs on merged PRs - -## Goal - -`quest usage` counts a PR as handed back when any version of its body had a -`## Waiting` (or `## Decisions needed`) section while it was a draft, merged -PRs included, so the writer A/B's and the model check's hand-back measure -sees every PR rather than only drafts still open. - -## Plan - -Decided 2026-11-14 (user, /quest-run, from -[moq-dev/quest#721](https://github.com/moq-dev/quest/pull/721)'s -follow-ups): add this quest rather than drop the measure. - -Today `handedBack` in `src/cli/usage.ts` reads only the current body of an -open draft. `quest ready` and hand edits remove `## Waiting` before merge, -so every merged PR reads 0. - -Recommended (agent's choice): read each PR's body history from GitHub -(GraphQL `userContentEdits`, whose entries carry each earlier body) where -`quest usage` already reads the PR, and count a hand-back when an earlier -body had a hand-back section, using `handedBack` from `src/core/claim.ts`. -It works on PRs merged before this lands and needs no change to the skills -or `quest ready`. The alternative, having `quest ready` record hand-backs -in `## Models`, counts only PRs readied after it lands and trusts every -writer to keep the record. Batch the history reads with the existing PR -query so `--since` over a week stays within the GraphQL rate limit. - -Count each PR once, however many times it was handed back, and report the -count in the `accuracy` and per-PR rows as today. - -## Related - -- [Model check](/quest/m1/model-check.md) - charts hand-backs per PR diff --git a/quest/m1/model-check.md b/quest/m1/model-check.md index 0f662abb..4c78e9c1 100644 --- a/quest/m1/model-check.md +++ b/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 / diff --git a/src/cli/usage.ts b/src/cli/usage.ts index c54a38b9..49e70295 100644 --- a/src/cli/usage.ts +++ b/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, diff --git a/src/github/online-review.ts b/src/github/online-review.ts index 734eac8a..34b02a97 100644 --- a/src/github/online-review.ts +++ b/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 & { head: string; committed: string; moved?: boolean }; +export type Activity = Record, 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> { + const connections = [...CONNECTIONS, ...extra]; const pulls = new Map(); const more: { number: number; cursors: Partial> }[] = []; 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> = {}; - 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> = {}; 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> { - const pulls = await readActivity(client, repository, numbers); - return new Map([...pulls].map(([number, pull]) => [number, onlineReviews(pull)])); +): Promise> { + 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] : [], + ), + }, + ]), + ); } diff --git a/test/usage.test.ts b/test/usage.test.ts index fa1dd57f..5e86dd87 100644 --- a/test/usage.test.ts +++ b/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],