diff --git a/.changeset/onboard-review-mining.md b/.changeset/onboard-review-mining.md new file mode 100644 index 00000000..c39b179b --- /dev/null +++ b/.changeset/onboard-review-mining.md @@ -0,0 +1,5 @@ +--- +"@taskless/cli": patch +--- + +Onboarding now covers a busy repository's whole review window. The review-comment source names the 1000-result cap of `gh pr list` and `gh search`, gives ready commands to count the window, list it by date range, and fetch every inline review comment since the window opened through `gh api --paginate`, and says to sample evenly across the window rather than read the newest pull requests. GitLab gets the same whole-window guidance through `glab api --paginate`. The scan step now has the agent record each counted comment under its candidate while mining, so `surfaced` comes from that record instead of a second read. diff --git a/openspec/specs/cli-onboard/spec.md b/openspec/specs/cli-onboard/spec.md index 92a34fc1..d74cb74d 100644 --- a/openspec/specs/cli-onboard/spec.md +++ b/openspec/specs/cli-onboard/spec.md @@ -188,8 +188,8 @@ The recipe SHALL instruct the agent to: 1. Read `.taskless/taskless.json` and respect the `install.onboarded` field. 2. Ask the user exactly one question, "Do you want me to set up Taskless for you?", preceded by a scripted introduction the agent SHALL repeat word for word and SHALL NOT reword or extend, set apart from surrounding text by blank lines, and through the host's structured question tool when the host has one. The introduction SHALL name the sources in general terms (the repository, agent files, documentation, PRs, bug tracking), state that rules are written locally with no login, and state that a report is generated. The recipe SHALL state that a yes covers mining the repository, writing rules, publishing the report, and sending onboarding feedback. On anything but a yes the agent SHALL stop. 3. Establish the routing surface before proposing any candidate, by fetching the `route` topic for the destination criterion and running `taskless detect --json` for the repository's linters, languages, rule styles, host tools, and `agent` hints. -4. Mine every reachable source without asking which: codebase TODOs/FIXMEs, agent-memory files (CLAUDE.md, AGENTS.md, .cursorrules, and similar), review comments through whichever host tool `detect` reports present and applicable (pull requests through `gh`, merge requests through `glab`), the commit history of the window being mined, read for fixes that recur, and any MCP the agent has that reaches a bug tracker, an engineering-context tool, or architectural decisions. Whether to run a hinted command such as `gh auth status`, and which MCPs to use, SHALL be left to the agent. -5. Filter for high-signal candidates: repeated patterns across multiple PRs/files/comments, comments that cite a doc or style guide, and merge-blocking review feedback; drop one-off nits and pure formatting feedback. Commit-history evidence SHALL choose candidates only; the recipe SHALL state that it never counts toward a rule's `surfaced`, which counts review comments alone. +4. Mine every reachable source without asking which: codebase TODOs/FIXMEs, agent-memory files (CLAUDE.md, AGENTS.md, .cursorrules, and similar), review comments through whichever host tool `detect` reports present and applicable (pull requests through `gh`, merge requests through `glab`), the commit history of the window being mined, read for fixes that recur, and any MCP the agent has that reaches a bug tracker, an engineering-context tool, or architectural decisions. Whether to run a hinted command such as `gh auth status`, and which MCPs to use, SHALL be left to the agent. Wherever the review-comment source is offered, the recipe SHALL direct the agent to cover the whole window rather than the most recent pull or merge requests, SHALL give commands that rely on the host CLI's own pagination so the agent writes no pagination loop of its own, and, when the window holds more than the agent can fetch review bodies for, SHALL direct it to sample evenly across the whole window and to report counts from a sample as a floor. For GitHub it SHALL name the 1000-result cap of `gh pr list` and `gh search`, give a command that counts the window's merged pull requests and one that lists them by date range, and give the repository-wide review-comments endpoint filtered by `since` for the window's inline comments. +5. Filter for high-signal candidates: repeated patterns across multiple PRs/files/comments, comments that cite a doc or style guide, and merge-blocking review feedback; drop one-off nits and pure formatting feedback. Commit-history evidence SHALL choose candidates only; the recipe SHALL state that it never counts toward a rule's `surfaced`, which counts review comments alone. The recipe SHALL direct the agent to record each counted pull request and comment under its candidate while mining, scoped to the rule it expects to write, so the counts come from that record rather than a second read. 6. Author every candidate the routing surface sends to a local destination (`legacy`, `sg`, or `vale`) by following the `route` topic, without asking per candidate. 7. Record, and not author, every candidate the agent judges better solved by a runtime rule, for the report's `cloud` tier, noting how many instances of its pattern the agent saw in the code, which the report takes as that candidate's estimate. 8. Once the rules are written, run `taskless onboard --mark-complete` without asking. @@ -238,6 +238,19 @@ The recipe SHALL NOT restate the destination criterion itself; it SHALL referenc - **THEN** it SHALL name codebase TODOs/FIXMEs, agent-memory files, review comments, commit history, and MCP-reachable trackers and engineering context as sources - **AND** SHALL NOT direct the agent to ask the user which sources to include +#### Scenario: A busy review window is covered without hand-written pagination + +- **WHEN** the recipe is rendered for a GitHub repository on a host where `gh` is on `PATH`, or with `gh` unmeasured +- **THEN** the review-comment source SHALL state that `gh pr list` and `gh search` stop at 1000 results +- **AND** SHALL give a command counting the window's merged pull requests, a command listing them by date range, and a `gh api --paginate` command for the repository's review comments `since` the window's first day +- **AND** SHALL direct the agent to write no pagination loop of its own +- **AND** SHALL direct any sample to span the whole window, never the most recent pull requests, and state that counts from a sample are a floor + +#### Scenario: Surfaced is counted while mining + +- **WHEN** the recipe's scan step is read +- **THEN** it SHALL direct the agent to record each counted pull request and comment under its candidate as it mines, at the scope of the rule it expects to write + #### Scenario: A present and applicable tool is offered as present - **WHEN** the recipe is rendered for a GitHub repository on a host where `gh` is on `PATH` diff --git a/packages/cli/src/agent/onboard.md b/packages/cli/src/agent/onboard.md index 96170814..f6e8dea0 100644 --- a/packages/cli/src/agent/onboard.md +++ b/packages/cli/src/agent/onboard.md @@ -1,4 +1,4 @@ -# Topic: onboard (CLI v%(CLI_VERSION)s / topic v11) +# Topic: onboard (CLI v%(CLI_VERSION)s / topic v12) %(ONE_COMMAND_PER_CALL)s @@ -150,6 +150,12 @@ making the case for running them in CI. commit history chooses candidates; it never counts toward these numbers, which count review comments alone. + Count while you mine, not afterwards. As each comment joins a + candidate, note its pull request and comment under that candidate, + scoped to the rule you expect to write rather than a broader theme, + so the counts come from that list instead of a second read. When + `route` narrows a candidate, recount from the list. + 8. **Write the rules.** For every candidate whose destination is `legacy`, `sg`, or `vale`, follow the `%(TASKLESS_CLI)s agent route` recipe you already fetched, with the candidate as the rule diff --git a/packages/cli/src/prompts/recipes.ts b/packages/cli/src/prompts/recipes.ts index 65481ab2..248b89e9 100644 --- a/packages/cli/src/prompts/recipes.ts +++ b/packages/cli/src/prompts/recipes.ts @@ -788,6 +788,89 @@ function toolState(tools: HostTool[] | undefined, name: string): ToolState { return tool.present ? "present" : "absent"; } +/** + * How to cover a busy GitHub window, appended to every review-comment item + * that reads pull requests through `gh`. + * + * WRITTEN FROM RUNS THAT HIT EACH WALL (taskless/cli#674). Onboarding agents + * in busy repositories found the 1000-result cap of `gh pr list` and + * `gh search` only after getting short results, hand-rolled GraphQL + * pagination to export review threads, and one sampled the newest 200 PRs, + * which covered a single week, so its strongest evidence fell outside what it + * counted. Each command below was run against a real repository before it was + * written here; `{owner}` and `{repo}` are `gh api`'s own placeholders, which + * it fills in only inside the endpoint argument, which is why the search + * query rides in the endpoint rather than in `-f`. + */ +const GITHUB_REVIEW_WINDOW = `Cover the whole window, not the newest PRs. In a busy repository the +latest 200 PRs can be a single week, and the strongest evidence then +falls outside what you read. \`gh api --paginate\` follows every page +for you, so write no pagination loop of your own. Below, START is the +window's first day as YYYY-MM-DD. Send bulk output to a file outside +the repository and read it from there rather than into your context. + +- **Count the window, then list it in date chunks.** \`gh pr list\` + and \`gh search\` stop at 1000 results without saying so. Ask for the + total first: + + \`\`\` + gh api "search/issues?q=repo:{owner}/{repo}+is:pr+is:merged+merged:>=START&per_page=1" --jq .total_count + \`\`\` + + Up to 1000, one list covers it. Past that, list one date range at a + time, each small enough to return fewer than 1000, and split any + range that returns exactly 1000: + + \`\`\` + gh pr list --state merged --search "merged:START..END" --limit 1000 --json number,title,mergedAt + \`\`\` + +- **Fetch the window's inline review comments in one call.** The + endpoint covers every pull request in the repository, and \`since\` + filters on when a comment last changed, so keep only comments on a + PR in your list: + + \`\`\` + gh api --paginate "repos/{owner}/{repo}/pulls/comments?since=START&per_page=100" --jq '.[] | {pr: (.pull_request_url | split("/") | last), id, user: .user.login, path, body}' + \`\`\` + +- **Review bodies come one PR at a time**, from + \`gh api --paginate "repos/{owner}/{repo}/pulls/NUMBER/reviews"\`, and + count toward \`surfaced\` as much as inline comments do. When the + window holds more PRs than you can fetch them for, sample evenly + across it: every Nth PR from every chunk, never the most recent N. + Read every inline comment either way, since they arrived in bulk. + Counts from a sample are a floor; say the window was sampled when you + report them.`; + +/** + * The GitLab counterpart of {@link GITHUB_REVIEW_WINDOW}. GitLab's list + * endpoints carry no 1000-result cap to chunk around, so what is left is the + * whole-window rule, `glab api`'s own pagination, and where review notes live. + * `:id` is `glab api`'s placeholder for the current project. + */ +const GITLAB_REVIEW_WINDOW = `Cover the whole window, not the newest MRs. In a busy repository the +latest 200 MRs can be a single week, and the strongest evidence then +falls outside what you read. \`glab api --paginate\` follows every page +for you, so write no pagination loop of your own. Below, START is the +window's first day as YYYY-MM-DD. Send bulk output to a file outside +the repository and read it from there rather than into your context. + +- **List the window's merged MRs.** An older GitLab may ignore + \`merged_after\` and return every merged MR, so still drop any whose + \`merged_at\` falls before START: + + \`\`\` + glab api --paginate "projects/:id/merge_requests?state=merged&merged_after=START&per_page=100" + \`\`\` + +- **Review notes come one MR at a time**, from + \`glab api --paginate "projects/:id/merge_requests/IID/discussions"\`. + When the window holds more MRs than you can fetch them for, sample + evenly across it: every Nth MR across the whole window, never the + most recent N. Counts from a sample are a floor; say the window was + sampled when you report them.`; + /** * The review-comment entry in the onboard recipe's source list, as a whole * list item: pull requests through `gh`, or merge requests through `glab`. @@ -817,14 +900,18 @@ function reviewCommentSource(tools: HostTool[] | undefined): string { Taskless looked for the file and did not run it, so treat that as presence rather than as a working install; \`detect --json\` names the command that checks sign-in under \`agent\`. Scan the last 90 -days of merged PRs for repeated reviewer feedback patterns.`; +days of merged PRs for repeated reviewer feedback patterns. + +${GITHUB_REVIEW_WINDOW}`; } if (glab === "present") { return `- **Recent MR review comments**: \`glab\` is on your PATH. Taskless looked for the file and did not run it, so treat that as presence rather than as a working install; \`detect --json\` names the command that checks sign-in under \`agent\`. Scan the last 90 -days of merged merge requests for repeated reviewer feedback patterns.`; +days of merged merge requests for repeated reviewer feedback patterns. + +${GITLAB_REVIEW_WINDOW}`; } if (gh === "absent") { return `- **Recent PR review comments** are not a source here: \`gh\` is @@ -845,7 +932,9 @@ line rather than passing over the source in silence.`; return `- **Recent PR review comments**: reading merged PRs needs the GitHub CLI (\`gh\`), or the GitLab CLI (\`glab\`) for merge requests, or something equivalent. Scan the last 90 days of merged PRs for -repeated reviewer feedback patterns.`; +repeated reviewer feedback patterns. With \`gh\`: + +${GITHUB_REVIEW_WINDOW}`; } /** diff --git a/packages/cli/test/host-tools.test.ts b/packages/cli/test/host-tools.test.ts index 7fd90245..313934f8 100644 --- a/packages/cli/test/host-tools.test.ts +++ b/packages/cli/test/host-tools.test.ts @@ -197,6 +197,79 @@ describe("onboard recipe renders host-tool state", () => { expect(rendered).not.toContain("`gh` is available"); }); + // taskless/cli#674: busy-repository runs hit the 1000-result cap, wrote + // their own pagination, and sampled the newest PRs, which in one repository + // covered a single week of a 90-day window. + it("covers a busy window without the agent writing its own pagination", () => { + const rendered = onboardWith({ present: true, applicable: true }); + const flat = rendered.replaceAll(/\s+/g, " "); + + expect(flat).toContain("stop at 1000 results"); + expect(flat).toContain("write no pagination loop of your own"); + expect(rendered).toContain( + 'gh api "search/issues?q=repo:{owner}/{repo}+is:pr+is:merged+merged:>=START&per_page=1" --jq .total_count' + ); + expect(rendered).toContain( + 'gh pr list --state merged --search "merged:START..END" --limit 1000' + ); + expect(rendered).toContain( + 'gh api --paginate "repos/{owner}/{repo}/pulls/comments?since=START&per_page=100"' + ); + expect(flat).toContain("never the most recent N"); + expect(flat).toContain("Counts from a sample are a floor"); + }); + + it("gives the busy-window guidance when gh was not measured", () => { + const rendered = getRecipe("onboard") ?? ""; + + expect(rendered).toContain("pulls/comments?since=START"); + expect(rendered.replaceAll(/\s+/g, " ")).toContain( + "never the most recent N" + ); + }); + + it("gives merge requests the whole-window rule through glab", () => { + const rendered = + getRecipe("onboard", { + hostTools: [ + { + name: "gh", + present: true, + applicable: false, + reason: "this repository has no GitHub origin", + }, + { name: "glab", present: true, applicable: true }, + ], + }) ?? ""; + + expect(rendered).toContain("**Recent MR review comments**: `glab` is"); + expect(rendered).toContain("glab api --paginate"); + // Filter on the merge date itself; `updated_after` would lean on the + // assumption that a merge always bumps `updated_at`. + expect(rendered).toContain("state=merged&merged_after=START"); + expect(rendered).not.toContain("updated_after"); + expect(rendered.replaceAll(/\s+/g, " ")).toContain( + "never the most recent N" + ); + // GitHub's cap and endpoints have nothing to say to a GitLab repository. + expect(rendered).not.toContain("pulls/comments"); + }); + + it("keeps the busy-window guidance inside the review-comment item", () => { + const rendered = onboardWith({ present: true, applicable: true }); + const start = rendered.indexOf("**Recent PR review comments**"); + const end = rendered.indexOf(" - **Commit history**"); + const item = rendered.slice(start, end); + + expect(item).toContain("pulls/comments?since=START"); + // Every continuation line sits at the item's content column or deeper, + // so the guidance renders as part of the bullet rather than ending it. + for (const line of item.split("\n").slice(1)) { + if (line === "") continue; + expect(line).toMatch(/^ {5}/); + } + }); + it("omits PR review comments with a reason when gh is not on PATH", () => { const rendered = onboardWith({ present: false, applicable: true }); diff --git a/packages/cli/test/onboard.test.ts b/packages/cli/test/onboard.test.ts index a454e8a2..c4f136a5 100644 --- a/packages/cli/test/onboard.test.ts +++ b/packages/cli/test/onboard.test.ts @@ -402,8 +402,8 @@ describe("onboard recipe asks one question and finishes on its own", () => { : stdout.slice(start, start + title.length + next); } - it("is topic v11", () => { - expect(stdout).toMatch(/^# Topic: onboard {5}\(CLI v.+ \/ topic v11\)/m); + it("is topic v12", () => { + expect(stdout).toMatch(/^# Topic: onboard {5}\(CLI v.+ \/ topic v12\)/m); }); // v10 (#554): a worktree holds only commits, and `init` commits nothing, so @@ -433,6 +433,18 @@ describe("onboard recipe asks one question and finishes on its own", () => { expect(scan).not.toMatch(/\bfixed\b/); }); + // taskless/cli#674: mining grouped themes more broadly than the rules + // finally written, so runs re-read every comment to count them. + it("records each counted comment under its candidate, at the rule's scope", () => { + const scan = step("Scan with high-signal filtering").replaceAll( + /\s+/g, + " " + ); + expect(scan).toContain("Count while you mine, not afterwards"); + expect(scan).toContain("scoped to the rule you expect to write"); + expect(scan).toContain("recount from the list"); + }); + it("mines commit history for recurring fixes, but never counts it as surfaced", () => { const mine = step("Mine every source you can reach"); expect(mine).toContain("**Commit history**");