fix(pr-review): correct spec inconsistencies in orchestrator-split issues#28
Merged
Merged
Conversation
…sues Found and fixed ten spec issues during grill-with-docs session: - Mode detection must use `az repos pr thread list`, not `az devops invoke` - Step 8 references replaced with purpose-based description (step numbering changes after refactor) - ADO Fetcher is a prerequisite for Doc Context Orchestrator, not concurrent - Re-review Coordinator returns `earlyExit` flag; orchestrator skips ADO Writer entirely on early exit - Coordinator receives full unfiltered thread list, not pre-filtered prior-threads JSON (`detect-prior-review` runs inside the Coordinator) - Thread list is captured during mode detection and passed forward — no second ADO call - Diff-to-hunks parsing moved inside the Coordinator (was incorrectly an orchestrator input) - All references to `.claude/prompts/pr-review-workflow.prompt.md` removed (untracked temp file, wrong analogy) - Stale monolith step reference removed from ADR 0013 - CLAUDE.md update added to issue 07 scope Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the spec set for the pr-review “orchestrator split” work (docs/issues/pr-review-orchestrator-split/) to remove inconsistencies discovered during review, so implementing agents have unambiguous acceptance criteria and interface contracts before any code changes land.
Changes:
- Clarifies orchestration sequencing (ADO Fetcher prerequisite; Doc Context + review aspect agents run concurrently after Fetcher completes).
- Refines re-review orchestration contracts (raw threads JSON input;
earlyExitflag semantics; orchestrator skips writer on early exit). - Removes references to an untracked “GitHub prompt” model and updates naming from “Step 8” to “review-agent launch step” in several places.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/issues/pr-review-orchestrator-split/PRD.md | Updates user stories + acceptance criteria and removes the untracked prompt-model reference. |
| docs/issues/pr-review-orchestrator-split/01-create-ado-fetcher-agent.md | Clarifies that Fetcher must complete before Doc Context runs; concurrency occurs after Fetcher. |
| docs/issues/pr-review-orchestrator-split/03-create-re-review-coordinator-agent.md | Updates Coordinator inputs/outputs (raw threads JSON, internal hunks parsing, earlyExit). |
| docs/issues/pr-review-orchestrator-split/04-refactor-orchestrator.md | Updates mode detection method and specifies early-exit behavior for re-review orchestration. |
| docs/issues/pr-review-orchestrator-split/06-compact-subagent-output.md | Renames “Step 8” to purpose-based “review-agent launch step” and clarifies schema guidance placement. |
| docs/issues/pr-review-orchestrator-split/07-version-bump-and-release.md | Brings CLAUDE.md update into scope and adds it to acceptance criteria. |
| apps/claude-code/pr-review/docs/adr/0013-orchestrator-split-for-review-pr.md | Removes the untracked prompt-model analogy; reframes the “thin coordinator” rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Issue 04: add explicit ADO Fetcher invocation to steps 5 and 6 - Issue 04: clarify inline ADO prohibition (no az devops invoke; mode-detection az repos pr thread list is allowed) - Issue 03: drop "new" from Coordinator return contract count list - PRD: replace remaining "Step 8" reference with "review-agent launch step" - ADR 0013: align "no inline ADO shell commands" with allowed mode-detection exception Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… docs - Replace "prior commit SHA" with "prior iteration ID" throughout: the parse-signature module extracts an iteration ID, not a commit SHA; the no-new-revisions check compares iteration IDs - Add `obsolete` to Re-review Coordinator return schema; all four classification states are now surfaced (addressed, disputed, pending, obsolete) - Clarify issue 03 step 7: "fresh findings list" replaces "updated findings list" ambiguity; early-exit return now shows all fields always present - Issue 04: Coordinator is explicitly called after all review agents complete; ADO Writer mode flag defined with its two values and semantics - Issue 01: prior iteration ID is an input, not an output of the context block - PRD: User Story 7 names all six finding fields; Re-review Coordinator Key interface now includes return schema and earlyExit skip-writer contract; ADO Fetcher interface clarified as input/output Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A
grill-with-docssession over the orchestrator-split feature set (docs/issues/pr-review-orchestrator-split/) found ten spec inconsistencies that would have caused an implementing agent to either fail acceptance criteria or produce incorrect behaviour. This PR fixes all of them before any code is written.What changed
az devops invoke" but the implementation usedaz devops invokeaz repos pr thread listfor mode detectionearlyExitflag to Coordinator return value; orchestrator skips ADO Writer entirely when setdetect-prior-reviewruns inside the Coordinator.claude/prompts/pr-review-workflow.prompt.mdreferenced as structural model in four placesCLAUDE.mdupdate not in scope of any issueFiles changed
docs/issues/pr-review-orchestrator-split/PRD.mddocs/issues/pr-review-orchestrator-split/01-create-ado-fetcher-agent.mddocs/issues/pr-review-orchestrator-split/03-create-re-review-coordinator-agent.mddocs/issues/pr-review-orchestrator-split/04-refactor-orchestrator.mddocs/issues/pr-review-orchestrator-split/06-compact-subagent-output.mddocs/issues/pr-review-orchestrator-split/07-version-bump-and-release.mdapps/claude-code/pr-review/docs/adr/0013-orchestrator-split-for-review-pr.mdAll seven issues remain
ready-for-agent.Test plan
🤖 Generated with Claude Code