diff --git a/apps/claude-code/pr-review/docs/adr/0013-orchestrator-split-for-review-pr.md b/apps/claude-code/pr-review/docs/adr/0013-orchestrator-split-for-review-pr.md index 1a8ebd8..b50287e 100644 --- a/apps/claude-code/pr-review/docs/adr/0013-orchestrator-split-for-review-pr.md +++ b/apps/claude-code/pr-review/docs/adr/0013-orchestrator-split-for-review-pr.md @@ -12,7 +12,7 @@ The root cause is architectural: `review-pr.md` conflates orchestration (which mode are we in? what agents to launch?) with platform integration (fetch ADO threads, post inline comments) and re-review state management (classify threads, match findings, reply). -The GitHub PR review workflow (`.claude/prompts/pr-review-workflow.prompt.md`) solves the same orchestration problem in ~80 lines by staying a thin coordinator and delegating everything else to focused agents. That pattern is the right model for `review-pr.md`. +The right model for `review-pr.md` is a thin coordinator: prerequisites block, mode detection block, and one delegation block per mode. The three focused agents own all data-fetch and write-back ADO operations. The one allowed inline ADO call is the mode-detection `az repos pr thread list` in the mode detection block — an orchestration concern, not a data-fetch or write-back operation; no `az devops invoke` commands remain in the orchestrator. ## Decision @@ -25,12 +25,12 @@ Refactor `review-pr.md` into a **thin orchestrator** of ~200 lines that: Three focused agents live in the plugin's `.agents/` directory (not in `pr-review-toolkit`, which is a read-only dependency): - **`pr-review:ado-fetcher`** — fetches PR metadata, iterations, changed files, and raw diff from ADO. Used by first-review and re-review modes. -- **`pr-review:re-review-coordinator`** — owns Steps 3.5–10-Path-B: prior thread detection, partial-run check, thread classification, finding matching, and reply posting to classified threads. Used only in re-review mode. +- **`pr-review:re-review-coordinator`** — owns prior thread detection, partial-run check, thread classification, finding matching, and reply posting to classified threads. Used only in re-review mode. - **`pr-review:ado-writer`** — owns the ADO write-back pipeline: posting inline threads, patching thread status, and posting the summary comment. Used by first-review and re-review modes. Pre-PR mode skips the ADO fetcher and writer entirely; it goes straight from the orchestrator to the `pr-review-toolkit` review agents and presents findings locally. -**Compact sub-agent output.** Review agents (`pr-review-toolkit:code-reviewer`, etc.) are asked via the Step 8 prompt in `review-pr.md` to return structured findings (`severity`, `filePath`, `startLine`, `endLine`, `title`, `body`) rather than prose with embedded code quotes. This keeps what flows back into the parent context small. This guidance stays in `review-pr.md`'s prompt, not in the toolkit agent definitions, because `pr-review-toolkit` is not owned by this plugin. +**Compact sub-agent output.** Review agents (`pr-review-toolkit:code-reviewer`, etc.) are asked via the review-agent launch step in `review-pr.md` to return structured findings (`severity`, `filePath`, `startLine`, `endLine`, `title`, `body`) rather than prose with embedded code quotes. This keeps what flows back into the parent context small. This guidance stays in `review-pr.md`'s prompt, not in the toolkit agent definitions, because `pr-review-toolkit` is not owned by this plugin. **Re-review logic ownership.** The four Node.js modules in `scripts/re-review/` are already algorithmically platform-agnostic; only their input shapes are ADO-specific. When a second write-back platform (GitHub) is built, normalising to a canonical thread shape and lifting these modules to `pr-review-toolkit` is the correct move. That work is deferred until a second platform consumer exists. @@ -53,5 +53,3 @@ _Option B: re-review coordinator as a procedural agent_ — keep re-review logic - `docs/issues/pr-review-orchestrator-split/PRD.md` for the feature PRD and implementation issues that deliver this split - ADR 0008 (soft dependency on `pr-review-toolkit`) -- `.claude/prompts/pr-review-workflow.prompt.md` (the GitHub orchestrator pattern this - mirrors) diff --git a/docs/issues/pr-review-orchestrator-split/01-create-ado-fetcher-agent.md b/docs/issues/pr-review-orchestrator-split/01-create-ado-fetcher-agent.md index ec854cf..783dfe6 100644 --- a/docs/issues/pr-review-orchestrator-split/01-create-ado-fetcher-agent.md +++ b/docs/issues/pr-review-orchestrator-split/01-create-ado-fetcher-agent.md @@ -9,11 +9,11 @@ ## What to build -Create a new plugin agent (`pr-review:ado-fetcher`) that encapsulates all Azure DevOps read operations required for a PR review. The agent receives a PR URL (org, project, PR ID) and returns a structured context block containing: PR metadata (title, description, source/target branches, repo ID), latest iteration ID and its commit SHA, prior commit SHA (passed in for re-review, empty for first-review), changed files list, raw diff, and work-item IDs linked to the PR. +Create a new plugin agent (`pr-review:ado-fetcher`) that encapsulates all Azure DevOps read operations required for a PR review. The agent receives a PR URL (org, project, PR ID) and an optional prior iteration ID (passed in for re-review, empty string for first-review), and returns a structured context block containing: PR metadata (title, description, source/target branches, repo ID), latest iteration ID and its commit SHA, changed files list, raw diff, and work-item IDs linked to the PR. This agent replaces the inline ADO shell commands currently scattered across Steps 2–5 of the `review-pr` command. It is invoked by first-review and re-review modes; pre-PR mode never calls it. -The ADO Fetcher and the Doc Context Orchestrator agent must be invocable concurrently — the ADO Fetcher provides the work-item IDs that the Doc Context Orchestrator needs, so the Fetcher runs first, but the Fetcher and Doc Context Orchestrator may overlap in wall-clock time. +The ADO Fetcher is a prerequisite for the Doc Context Orchestrator — the Fetcher must complete first because its output (work-item IDs) is the input the Doc Context Orchestrator needs. Once the Fetcher returns, the Doc Context Orchestrator and review aspect agents launch concurrently with each other. ## Acceptance criteria @@ -45,7 +45,7 @@ A new plugin agent (`pr-review:ado-fetcher`) accepts PR URL components and retur **Key interfaces:** -- Input: org URL, project, PR ID, optional prior commit SHA (passed in for re-review) +- Input: org URL, project, PR ID, optional prior iteration ID (passed in for re-review) - Output: structured context block — PR metadata, latest iteration ID, latest commit SHA, changed files list, raw diff, work-item IDs list - The agent must handle zero-iteration PRs and already-merged PRs gracefully diff --git a/docs/issues/pr-review-orchestrator-split/03-create-re-review-coordinator-agent.md b/docs/issues/pr-review-orchestrator-split/03-create-re-review-coordinator-agent.md index cda8ab6..7337d4d 100644 --- a/docs/issues/pr-review-orchestrator-split/03-create-re-review-coordinator-agent.md +++ b/docs/issues/pr-review-orchestrator-split/03-create-re-review-coordinator-agent.md @@ -9,17 +9,17 @@ ## What to build -Create a new plugin agent (`pr-review:re-review-coordinator`) that owns the full re-review state machine. The agent receives the ADO Fetcher context block, the raw prior-threads JSON, and the diff hunks file path. +Create a new plugin agent (`pr-review:re-review-coordinator`) that owns the full re-review state machine. The agent receives the ADO Fetcher context block (which includes the raw diff) and the raw full PR threads JSON (the unfiltered ADO thread list). It parses the raw diff into diff hunks internally before calling `classify-thread` — the hunks file is a temp artefact managed inside the agent, not an input from the orchestrator. It performs in order: 1. Calls the `detect-prior-review` Node.js module to identify prior bot threads and locate the summary thread. 2. Runs the partial-run check (looks for the completion marker for the prior iteration in the summary thread). Falls back to first-review mode if the marker is absent. -3. If no new commits exist since the prior review (prior commit SHA equals latest commit SHA), prints outstanding pending threads to the console and exits early — no ADO writes. +3. If no new revisions exist since the prior review (prior iteration ID equals latest iteration ID), prints outstanding pending threads to the console and exits early — no ADO writes. 4. Calls `classify-thread` on each prior thread against the diff hunks. 5. For each new finding passed in, calls `match-finding` to look for a matching prior thread. 6. Based on classification, posts replies to prior threads: acknowledges disputes, confirms resolutions (and PATCHes thread status to fixed), adds new evidence to pending threads with new information, skips pending threads with no new evidence, ignores obsolete threads. -7. Returns the classification counts (new, addressed, disputed, pending) and the updated findings list (unmatched findings pass through as fresh; matched findings are consumed). +7. Returns the classification counts (addressed, disputed, pending, obsolete), the fresh findings list (`freshFindings[]` — only unmatched findings; matched findings are consumed and not returned), and an `earlyExit` flag. `earlyExit` is `true` only on the no-new-revisions path (step 3); it is `false` on all other paths including normal completion with zero fresh findings. The four Node.js modules (`detect-prior-review`, `classify-thread`, `match-finding`, `parse-signature`) remain in `scripts/re-review/` unchanged. This agent calls them via `node --input-type=module` inline scripts, exactly as the current `review-pr.md` does. @@ -27,13 +27,13 @@ The four Node.js modules (`detect-prior-review`, `classify-thread`, `match-findi - [ ] The agent correctly detects prior bot threads using the `detect-prior-review` module - [ ] The agent falls back to first-review mode when no completion marker is found for the prior iteration -- [ ] The agent exits early (console output only, no ADO writes) when prior and latest commit SHAs are identical +- [ ] The agent exits early (console output only, no ADO writes) when prior and latest iteration IDs are identical, and returns `earlyExit: true` - [ ] The agent classifies all prior threads using the `classify-thread` module - [ ] The agent matches new findings to prior threads using the `match-finding` module with ±3-line drift tolerance - [ ] The agent posts a dispute acknowledgement reply to disputed threads including the ADO nudge - [ ] The agent posts a resolution confirmation reply and PATCHes status to fixed for addressed threads - [ ] The agent posts a new-evidence reply to pending threads that have new analysis; skips pending threads with no new evidence -- [ ] The agent returns classification counts and the unmatched (fresh) findings list +- [ ] The agent returns classification counts (addressed, disputed, pending, obsolete), the unmatched (fresh) findings list, and the `earlyExit` flag - [ ] The existing re-review module unit tests (`detect-prior-review`, `classify-thread`, `match-finding`, `parse-signature`) pass unchanged ## Blocked by @@ -53,24 +53,25 @@ None — can start immediately. The re-review state machine (prior thread detection, partial-run check, Thread Classification, finding matching, reply/resolution posting) lives inline in `review-pr.md` across Steps 3.5–10-Path-B. It is loaded on every invocation regardless of mode. **Desired behavior:** -A new plugin agent (`pr-review:re-review-coordinator`) receives the ADO Fetcher context block, raw prior-threads JSON, and diff hunks. It runs the full re-review state machine, posts classified replies directly to ADO, and returns classification counts plus the list of unmatched (fresh) findings for the ADO Writer to post as new threads. The four existing Node.js modules (`detect-prior-review`, `classify-thread`, `match-finding`, `parse-signature`) are called from this agent unchanged. +A new plugin agent (`pr-review:re-review-coordinator`) receives the ADO Fetcher context block (which includes the raw diff) and the raw full PR threads JSON (unfiltered). It calls `detect-prior-review` internally to identify bot threads, parses the raw diff into diff hunks internally, then runs the full re-review state machine, posts classified replies directly to ADO, and returns classification counts plus the list of unmatched (fresh) findings for the ADO Writer to post as new threads. The four existing Node.js modules (`detect-prior-review`, `classify-thread`, `match-finding`, `parse-signature`) are called from this agent unchanged. **Key interfaces:** -- Input: ADO Fetcher context block, prior-threads JSON (from `detect-prior-review`), diff hunks JSON, new findings list, Bot Signature prefix constant -- Output: `{ addressed, disputed, pending, freshFindings[] }` — fresh findings are those with no matching prior thread +- Input: ADO Fetcher context block (includes raw diff), raw full PR threads JSON (captured by the orchestrator during mode detection via `az repos pr thread list` — not re-fetched; `detect-prior-review` filters this list inside the Coordinator), new findings list, Bot Signature prefix constant +- The Coordinator parses the raw diff into diff hunks internally; this is not an orchestrator concern +- Output: `{ addressed, disputed, pending, obsolete, freshFindings[], earlyExit }` — fresh findings are those with no matching prior thread; `earlyExit: true` signals the no-new-revisions path to the orchestrator - The agent calls the four Node.js modules via `node --input-type=module` inline scripts (same pattern as current `review-pr.md`) -- Early-exit path: when prior commit SHA equals latest commit SHA, prints pending threads to console and returns with empty fresh findings — no ADO writes +- Early-exit path: when prior iteration ID equals latest iteration ID, prints pending threads to console and returns `{ earlyExit: true, freshFindings: [], addressed: 0, disputed: 0, pending: N, obsolete: 0 }` — all count fields are always present; the orchestrator must skip the ADO Writer entirely when `earlyExit: true` **Acceptance criteria:** - [ ] The agent correctly detects prior bot threads using the `detect-prior-review` module - [ ] The agent falls back to first-review mode when no completion marker is found for the prior iteration -- [ ] The agent exits early when prior and latest commit SHAs are identical (console output only, no ADO writes) +- [ ] The agent exits early when prior and latest iteration IDs are identical (console output only, no ADO writes), returning `earlyExit: true` - [ ] The agent classifies all prior threads using the `classify-thread` module - [ ] The agent matches new findings to prior threads using `match-finding` with ±3-line drift tolerance - [ ] The agent posts dispute acknowledgement, resolution confirmation, and new-evidence replies appropriately -- [ ] The agent returns classification counts and unmatched fresh findings +- [ ] The agent returns classification counts (addressed, disputed, pending, obsolete), the unmatched fresh findings list, and the `earlyExit` flag - [ ] The four re-review module unit tests pass unchanged after this issue is implemented **Out of scope:** diff --git a/docs/issues/pr-review-orchestrator-split/04-refactor-orchestrator.md b/docs/issues/pr-review-orchestrator-split/04-refactor-orchestrator.md index 9345505..ce09dee 100644 --- a/docs/issues/pr-review-orchestrator-split/04-refactor-orchestrator.md +++ b/docs/issues/pr-review-orchestrator-split/04-refactor-orchestrator.md @@ -13,13 +13,13 @@ Refactor `review-pr.md` into a thin orchestrator of approximately 200 lines. The 1. Validates prerequisites in a mode-aware way: always checks `git` availability and `pr-review-toolkit`; checks Azure CLI and `azure-devops` extension only when a PR URL is present (Pre-PR mode requires no ADO credentials). 2. Parses `$ARGUMENTS` for a PR URL. If absent, sets mode to Pre-PR; if present, proceeds to detection. -3. For PR URL cases: makes a lightweight ADO thread-list call directly (not via the ADO Fetcher) to check for a prior Bot Signature, determining mode and extracting the prior commit SHA if found; then invokes the ADO Fetcher agent (passing the prior commit SHA for re-review runs). +3. For PR URL cases: makes a lightweight ADO thread-list call directly (not via the ADO Fetcher) using `az repos pr thread list` (not `az devops invoke`) to check for a prior Bot Signature, determining mode and extracting the prior iteration ID if found. The full thread list from this call is captured and passed forward to the Re-review Coordinator in step 6 — no second ADO thread-list call is made. 4. Logs the detected mode clearly before delegating. -5. For First-review: runs Doc Context Orchestrator + review aspect agents in parallel, collects compact findings, delegates write-back to the ADO Writer agent. -6. For Re-review: runs Doc Context Orchestrator + review aspect agents in parallel, passes findings and prior-thread data to the Re-review Coordinator agent (which handles replies), then passes remaining fresh findings to the ADO Writer agent. +5. For First-review: invokes the ADO Fetcher agent (passing org URL, project, PR ID), then runs Doc Context Orchestrator + review aspect agents in parallel, collects compact findings, delegates write-back to the ADO Writer agent. +6. For Re-review: invokes the ADO Fetcher agent (passing org URL, project, PR ID, and prior iteration ID), then runs Doc Context Orchestrator + review aspect agents in parallel. Once all review aspect agents return their findings, passes the complete findings list and prior-thread data to the Re-review Coordinator agent (which handles replies). If the Coordinator returns `earlyExit: true` (no new revisions), the orchestrator stops — ADO Writer is not called. Otherwise passes fresh findings to the ADO Writer agent. 7. Pre-PR mode is a stub at this slice — it detects the mode and prints a "Pre-PR mode not yet implemented" message. Full Pre-PR behaviour is delivered in issue 05. -The `review-pr.md` file must contain no `az devops invoke` shell commands after this refactor — all ADO operations live in the three focused agents. The Bot Signature constants and detection prefix are unchanged. All existing re-review module unit tests must pass. +The `review-pr.md` file must contain no `az devops invoke` shell commands after this refactor — the three focused agents own all data-fetch and write-back ADO operations. The one allowed inline ADO call is the mode-detection `az repos pr thread list` in step 3, which is an orchestration concern, not a data-fetch or write-back operation. The Bot Signature constants and detection prefix are unchanged. All existing re-review module unit tests must pass. ## Acceptance criteria @@ -57,12 +57,12 @@ The `review-pr.md` file must contain no `az devops invoke` shell commands after **Key interfaces:** -- Mode detection sequence: no URL → Pre-PR; URL → orchestrator makes a lightweight ADO thread-list call → no Bot Signature → First-review; Bot Signature found → extract prior commit SHA → Re-review +- Mode detection sequence: no URL → Pre-PR; URL → orchestrator calls `az repos pr thread list` (not `az devops invoke`) → no Bot Signature → First-review; Bot Signature found → extract prior iteration ID → Re-review - Bot Signature detection prefix: `🤖 *Reviewed by Claude Code*` — must not change -- ADO Fetcher agent invocation: passes org URL, project, PR ID -- Re-review Coordinator agent invocation (re-review only): passes ADO Fetcher context + new findings list -- ADO Writer agent invocation: passes PR context + fresh findings list + mode flag -- The GitHub prompt (`.claude/prompts/pr-review-workflow.prompt.md`) is the structural reference for what the orchestrator should look like +- ADO Fetcher agent invocation: passes org URL, project, PR ID (plus prior iteration ID in re-review) +- Re-review Coordinator agent invocation (re-review only): called after all review aspect agents complete; passes ADO Fetcher context + full PR threads JSON (captured from mode detection in step 3, not re-fetched) + new findings list; returns `{ earlyExit, freshFindings[], addressed, disputed, pending, obsolete }` +- If Coordinator returns `earlyExit: true`, orchestrator stops — ADO Writer is not called +- ADO Writer agent invocation: passes PR context + fresh findings list + mode (`"first-review"` | `"re-review"`); the mode determines whether ADO Writer posts a new Review Summary (first-review) or a delta reply (re-review) **Acceptance criteria:** diff --git a/docs/issues/pr-review-orchestrator-split/06-compact-subagent-output.md b/docs/issues/pr-review-orchestrator-split/06-compact-subagent-output.md index 1b81bef..06293bf 100644 --- a/docs/issues/pr-review-orchestrator-split/06-compact-subagent-output.md +++ b/docs/issues/pr-review-orchestrator-split/06-compact-subagent-output.md @@ -1,4 +1,4 @@ -# Add compact sub-agent output guidance to Step 8 prompt +# Add compact sub-agent output guidance to the review-agent launch step **Status:** ready-for-agent **Category:** enhancement @@ -9,7 +9,7 @@ ## What to build -Update the Step 8 prompt in the thin orchestrator to instruct `pr-review-toolkit` review aspect agents to return compact structured findings rather than prose with embedded code quotes. +Update the step in the thin orchestrator that launches `pr-review-toolkit` review aspect agents to instruct them to return compact structured findings rather than prose with embedded code quotes. (The thin orchestrator produced by issue 04 will have a different step numbering than the pre-refactor monolith — find the step by its purpose: the one that spawns the parallel review agents.) The prompt addition instructs each agent to return a JSON array where each element has: `severity` (critical / important / minor), `filePath` (leading `/`, forward slashes), `startLine` (integer), `endLine` (integer), `title` (one line, ≤ 80 chars), `body` (one paragraph — the exact text to post as the ADO or local-interface comment, no code quotes, no repeated context). The reasoning and supporting analysis should stay inside the agent's own context, not appear in the return value. @@ -17,7 +17,7 @@ No changes are made to `pr-review-toolkit` agent definitions — this guidance l ## Acceptance criteria -- [ ] The Step 8 prompt explicitly requests structured JSON findings with the six required fields +- [ ] The review-agent launch step explicitly requests structured JSON findings with the six required fields - [ ] The prompt instructs agents to omit code quotes and prose reasoning from the return value - [ ] The ADO Writer agent correctly receives and processes the structured finding schema - [ ] Pre-PR mode findings are also presented using the same structured schema @@ -35,23 +35,23 @@ No changes are made to `pr-review-toolkit` agent definitions — this guidance l > _This was generated by AI during triage._ **Category:** enhancement -**Summary:** Update the orchestrator's Step 8 prompt to request compact structured findings from review aspect agents. +**Summary:** Update the review-agent launch step in the thin orchestrator to request compact structured findings from review aspect agents. **Current behavior:** Review aspect agents return prose findings with embedded code quotes and explanatory text. The full prose flows back into the parent context as tool call results, contributing to token budget pressure. **Desired behavior:** -The Step 8 prompt in the thin orchestrator explicitly instructs each review aspect agent to return a JSON array of findings. Each element: `severity` (critical / important / minor), `filePath`, `startLine`, `endLine`, `title` (≤ 80 chars), `body` (one paragraph, no code quotes). Reasoning stays inside the agent's own context. No `pr-review-toolkit` agent definition files are modified. +The step in the thin orchestrator that spawns parallel review agents explicitly instructs each `pr-review-toolkit` review aspect agent to return a JSON array of findings. Locate this step by purpose — it is the one that launches the review agents in parallel — not by number (step numbering changed after the issue 04 refactor). Each element: `severity` (critical / important / minor), `filePath`, `startLine`, `endLine`, `title` (≤ 80 chars), `body` (one paragraph, no code quotes). Reasoning stays inside the agent's own context. No `pr-review-toolkit` agent definition files are modified. **Key interfaces:** - The structured finding schema: `{ severity, filePath, startLine, endLine, title, body }` -- Guidance location: orchestrator Step 8 prompt only — not in toolkit agent definitions +- Guidance location: the review-agent launch step in the orchestrator only — not in toolkit agent definitions - Both ADO modes and Pre-PR mode use this schema **Acceptance criteria:** -- [ ] The Step 8 prompt requests structured JSON findings with all six required fields +- [ ] The review-agent launch step requests structured JSON findings with all six required fields - [ ] The prompt instructs agents to omit code quotes and prose reasoning from the return value - [ ] The ADO Writer agent correctly receives and processes the structured schema - [ ] Pre-PR mode findings are presented using the same schema diff --git a/docs/issues/pr-review-orchestrator-split/07-version-bump-and-release.md b/docs/issues/pr-review-orchestrator-split/07-version-bump-and-release.md index 016e545..e25ce91 100644 --- a/docs/issues/pr-review-orchestrator-split/07-version-bump-and-release.md +++ b/docs/issues/pr-review-orchestrator-split/07-version-bump-and-release.md @@ -13,11 +13,14 @@ Bump the `pr-review` plugin version (minor bump — new features added) and add Run `pnpm --filter pr-review bump minor` to update both `plugin.json` and `marketplace.json`. Add a `[Unreleased]` → versioned entry to `CHANGELOG.md` following the existing format. Run `pnpm --filter pr-review verify:changelog` to confirm the entry passes validation. +Update `CLAUDE.md` to reflect the new architecture: remove the claim that "the entire behaviour of the plugin lives in `commands/review-pr.md`", add the `.agents/` directory to the repository layout, and update the command conventions section to note that ADO calls now live in the three focused agents (ADO Fetcher, Re-review Coordinator, ADO Writer) rather than inline in the command. + ## Acceptance criteria - [ ] `plugin.json` and `marketplace.json` both reflect the new minor version - [ ] `CHANGELOG.md` has a dated entry for the new version describing the orchestrator split, three new agents, pre-PR mode, and compact output guidance - [ ] `pnpm --filter pr-review verify:changelog` passes +- [ ] `CLAUDE.md` updated: "entire behaviour lives in `commands/review-pr.md`" claim removed, `.agents/` directory added to layout, command conventions updated to reflect ADO calls live in the focused agents - [ ] `pnpm format` produces no diff ## Blocked by @@ -51,6 +54,7 @@ Run `pnpm --filter pr-review bump minor` to update both version files atomically - [ ] `plugin.json` and `marketplace.json` both reflect the new minor version - [ ] `CHANGELOG.md` has a dated entry for the new version covering all four feature areas - [ ] `pnpm --filter pr-review verify:changelog` passes +- [ ] `CLAUDE.md` updated to reflect the three-agent architecture - [ ] `pnpm format` produces no diff **Out of scope:** diff --git a/docs/issues/pr-review-orchestrator-split/PRD.md b/docs/issues/pr-review-orchestrator-split/PRD.md index b21024d..220545e 100644 --- a/docs/issues/pr-review-orchestrator-split/PRD.md +++ b/docs/issues/pr-review-orchestrator-split/PRD.md @@ -28,7 +28,7 @@ Refactor `review-pr.md` into a thin orchestrator of ~200 lines that detects the 6. As a developer on a large PR, I want review-agent findings returned as compact structured records rather than prose with embedded code quotes, so that the parent context stays within budget. -7. As a developer, I want the structured finding to include severity, file path, line range, a short title, and one-paragraph comment body, so that the ADO Writer has everything it needs to post the Inline Comment without re-querying the agent. +7. As a developer, I want the structured finding to include severity, file path, start line, end line, a short title, and one-paragraph comment body, so that the ADO Writer has everything it needs to post the Inline Comment without re-querying the agent. 8. As a developer, I want the ADO Fetcher to encapsulate all ADO API calls needed to retrieve PR metadata, iterations, changed files, and the raw diff, so that the orchestrator does not contain any platform-specific shell commands. @@ -48,9 +48,9 @@ Refactor `review-pr.md` into a thin orchestrator of ~200 lines that detects the 16. As a developer reading the codebase, I want each agent to have a single clearly named responsibility, so that I know exactly which file to open when debugging an ADO write error versus a thread-classification error. -17. As a developer running a first-review, I want the ADO Fetcher and the Doc Context Orchestrator to run concurrently as before, so that the split does not increase wall-clock time. +17. As a developer running a first-review, I want the ADO Fetcher to complete first (providing work-item IDs), then the Doc Context Orchestrator and review aspect agents to run concurrently with each other, so that the split does not increase wall-clock time. -18. As a developer, I want the guidance for compact review-agent output to live in the orchestrator's Step 8 prompt rather than in the `pr-review-toolkit` agent definitions, so that the toolkit remains an unmodified read-only dependency. +18. As a developer, I want the guidance for compact review-agent output to live in the orchestrator's review-agent launch step rather than in the `pr-review-toolkit` agent definitions, so that the toolkit remains an unmodified read-only dependency. 19. As a plugin operator, I want the existing test suite for the four re-review modules to continue passing after the split with no changes, so that I have confidence the refactor is behaviour-preserving. @@ -125,8 +125,6 @@ The existing test structure mirrors `packages/release-tools/scripts/verify-chang **CONTEXT.md** has already been updated with the three operating modes, three orchestration agent terms, and their relationships. -**GitHub prompt as reference.** The `.claude/prompts/pr-review-workflow.prompt.md` file is the model for what the thin orchestrator should look like — it coordinates review activities in ~80 lines by staying a pure coordinator. The refactored `review-pr.md` should be structurally similar. - --- ## Agent Brief @@ -148,13 +146,13 @@ The existing test structure mirrors `packages/release-tools/scripts/verify-chang Each of the three new agents lives in the plugin's own `.agents/` directory. `pr-review-toolkit` is not modified (it is a read-only dependency). The four existing re-review Node.js modules (`detect-prior-review`, `classify-thread`, `match-finding`, `parse-signature`) remain in the plugin's `scripts/re-review/` directory and are called from the Re-review Coordinator agent. -Review aspect agents are instructed via the orchestrator's Step 8 prompt to return compact structured findings (severity, file path, start line, end line, one-line title, one-paragraph body) rather than prose with embedded code quotes. This guidance lives in the orchestrator prompt only. +Review aspect agents are instructed via the review-agent launch step in the orchestrator to return compact structured findings (severity, file path, start line, end line, one-line title, one-paragraph body) rather than prose with embedded code quotes. This guidance lives in the orchestrator prompt only. **Key interfaces:** - `review-pr` command orchestrator — validates prerequisites, detects mode within first ~50 lines, delegates entirely; carries no ADO shell commands -- ADO Fetcher agent — returns a structured context block: PR metadata, latest iteration ID, prior commit ID (re-review only), changed files list, raw diff, and work-item IDs for Doc Context -- Re-review Coordinator agent — receives the ADO Fetcher context and prior-threads data; produces classified thread list and executes reply/resolution actions; delegates to `detect-prior-review`, `classify-thread`, and `match-finding` modules +- ADO Fetcher agent — accepts org URL, project, PR ID, and optional prior iteration ID (re-review only); returns a structured context block: PR metadata, latest iteration ID, changed files list, raw diff, and work-item IDs for Doc Context +- Re-review Coordinator agent — receives the ADO Fetcher context and prior-threads data; produces classified thread list and executes reply/resolution actions; delegates to `detect-prior-review`, `classify-thread`, and `match-finding` modules; returns `{ addressed, disputed, pending, obsolete, freshFindings[], earlyExit }` — when `earlyExit: true`, the orchestrator skips the ADO Writer entirely - ADO Writer agent — receives the findings list and PR context; posts all Inline Comment threads, patches thread statuses, posts the Review Summary or delta reply, posts the completion marker - Compact finding schema: `{ severity, filePath, startLine, endLine, title, body }` - Bot Signature constant: `🤖 *Reviewed by Claude Code*` prefix — must remain unchanged @@ -167,7 +165,7 @@ Review aspect agents are instructed via the orchestrator's Step 8 prompt to retu - [ ] Running with a URL where prior Bot Signature exists enters Re-review mode; the Re-review Coordinator correctly classifies threads and posts replies - [ ] The orchestrator logs the detected mode (Pre-PR / First-review / Re-review) before delegating - [ ] The four existing re-review module unit tests pass unchanged after the refactor -- [ ] The ADO Fetcher and Doc Context Orchestrator still run concurrently (no wall-clock regression for first-review) +- [ ] The ADO Fetcher completes before the Doc Context Orchestrator is launched; the Doc Context Orchestrator and review aspect agents then run concurrently with each other (no wall-clock regression for first-review) - [ ] The Bot Signature format and detection prefix are unchanged - [ ] `pnpm test` passes; `pnpm format` produces no diff