Skip to content

docs(pr-review): prep ADO fetcher reliability — Notice Tier doctrine + PRD A/B triage#30

Merged
orioltf merged 9 commits into
developfrom
feature/pr-review/orchestrator-split-03
May 13, 2026
Merged

docs(pr-review): prep ADO fetcher reliability — Notice Tier doctrine + PRD A/B triage#30
orioltf merged 9 commits into
developfrom
feature/pr-review/orchestrator-split-03

Conversation

@orioltf
Copy link
Copy Markdown
Member

@orioltf orioltf commented May 13, 2026

Summary

  • Adds Notice Tier vocabulary (OK / EMPTY-BY-DESIGN / DEGRADED / ABORTED, Notice, Trailer) to apps/claude-code/pr-review/CONTEXT.md
  • Graduates the ADO error-hardening inbox item into two full PRDs: PRD A (pr-review-ado-fetcher-reliability) and PRD B
  • Breaks PRD A into 4 tracer-bullet slices and PRD B into 6, all triaged to ready-for-agent

Why

Prerequisite for /implement-feature pr-review-ado-fetcher-reliability. The implementation worktree branches from develop and the sub-agents need the updated CONTEXT.md terms and the issue files already present on that base.

Test plan

  • No code changes — docs and triage files only
  • pnpm check and pnpm format pass

🤖 Generated with Claude Code

orioltf and others added 4 commits May 13, 2026 18:19
Captures the domain language that emerged from the grilling session
behind the ADO-error-hardening graduation: Notice (the structured
message emitted by orchestration agents), Notice Tier (the four-state
classification — OK / EMPTY-BY-DESIGN / DEGRADED / ABORTED — with no
fifth ASK tier), and Trailer (the mandatory end-of-run line in the
Claude interface).

The relationships section is extended with a new bullet stating that
every orchestration-agent operation terminates in one of the four
Notice Tiers and that DEGRADED and Doc-Context EMPTY-BY-DESIGN
operations emit a Notice that flows through the Review Summary (ADO
modes) or the pre-findings block (Pre-PR mode), with counts also
echoed in the Trailer.

These terms are referenced by the two PRDs created in the follow-up
commit (PRD A: pr-review-ado-fetcher-reliability; PRD B:
pr-review-platform-failure-handling) and by the planned ADRs 0014 and
0015. Capturing them in CONTEXT.md first so the PRDs can reference the
canonical definitions rather than re-litigating.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Splits the deferred items from PR #29's silent-failure-hunter review into
two coherent PRDs after a grilling session that locked the underlying
doctrine, scope, and module shape:

- PRD A — pr-review-ado-fetcher-reliability (foundation): introduces
  the four-tier Notice doctrine (OK / EMPTY-BY-DESIGN / DEGRADED /
  ABORTED), the helper layer under scripts/ado/ (classify-http-error,
  notices, fetch-iterations, fetch-work-items), the canonical
  HTTP-tier mapping (401/403 abort, 5xx/network degrade, no retries
  in v1), the Notice flow from agent result blocks through the
  orchestrator into the Review Summary, the mandatory end-of-run
  Trailer line, and the DIFF_RANGE: full | incremental sentinel in
  ADO_FETCHER_RESULT. Also schedules ADR 0014 (helper-layer
  refinement of ADR 0013), ADR 0015 (HTTP-tier mapping), and an
  in-place ADR 0004 amendment for the γ-downgrade rule.

- PRD B — pr-review-platform-failure-handling (consumer, depends on
  PRD A): applies the doctrine to the Re-review Coordinator (DIFF_RANGE
  consumption with γ-downgrade of addressed/obsolete to pending,
  match-finding try/catch with DEGRADED Notice, PATCH-to-fixed routed
  through the canonical mapping), the ADO Writer (every az devops
  invoke routed through a new parse-write-response helper, H1 inline
  POST retroactively inheriting 401/403 abort, *.err streamed to stderr
  at moment of failure, parseAdoWriterResult discriminated-union
  refactor), and Pre-PR mode (parseChangedFilesFromDiff suspicious-
  shape Notice, Gitflow-aware default-branch fallback chain via a new
  scripts/pre-pr/detect-default-branch.mjs helper).

Both PRDs land with Status: needs-triage. Test scope follows the user's
choice during grilling: unit tests for the new deep helpers; MODIFY
helpers and agent prompts are verified by integration smoke test
against a real ADO PR (per ADR 0013's testing posture).

The originating inbox file (docs/inbox/pr-review-ado-error-hardening-
pass.md) is removed in this commit, per the graduation flow documented
in docs/inbox/README.md. Discriminated-union refactors are verified
breaking-change-free (zero consumers outside the pr-review plugin).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each slice is a thin vertical cut through every layer (helper → agent
prompt → orchestrator → tests where applicable) and is demoable on its
own. All 10 slices land with Status: needs-triage and are AFK-grabbable
once their blocker has merged.

PRD A — pr-review-ado-fetcher-reliability (foundation):
  01 — End-to-end Notice pipeline via Doc-Context info Notice + ADR-0014
  02 — classify-http-error + fetch-work-items refactor → DEGRADED tier + ADR-0015
  03 — fetch-iterations refactor → ABORTED tier
  04 — DIFF_RANGE sentinel + ADR-0004 amendment

PRD B — pr-review-platform-failure-handling (consumer):
  01 — parse-write-response helper + Writer applies HTTP-tier mapping to all writes + .err streaming
  02 — parseAdoWriterResult discriminated-union refactor
  03 — Coordinator consumes DIFF_RANGE → γ-downgrade in classify-thread
  04 — Coordinator match-finding throws + DEGRADED Notice on catch
  05 — Coordinator PATCH-to-fixed routed through parse-write-response
  06 — Pre-PR Notice surface (suspicious-shape Notice + Gitflow-aware default-branch fallback)

Dependency graph:
  A1 ──┬── A2 ── A3
       │   └── B1 ── B5
       ├── A4 ── B3
       ├── B2
       ├── B4
       └── B6

After A1 lands, A2 / A4 / B2 / B4 / B6 can be picked up in parallel.
Only A3 (after A2) and B5 (after B1) extend the critical path.

Each slice file follows the to-issues template: Parent (PRD reference),
What to build (end-to-end description), Acceptance criteria (checkbox
list verifiable by pnpm test/check/verify:changelog), Blocked by
(concrete file path of the blocker, or "None" for A1).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each slice was fully grilled to lock in the doctrine, scope, and module
shape during the conversational /grill-with-docs session of 2026-05-13.
All acceptance criteria are precise and verifiable via the standard
pnpm test/check/verify:changelog loop. No outstanding questions on any
slice; no slice needs human implementation (all AFK).

Triage Notes appended per file map each slice to the specific grilling
questions that produced its locked decisions, so an AFK agent picking
up a slice tomorrow can trace the rationale without re-reading the
full conversation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the pr-review plugin’s documentation and issue tracker to prepare for upcoming Azure DevOps (ADO) fetcher reliability and broader platform-failure handling work, by formalizing “Notice Tier” terminology and publishing/triaging two PRDs (PRD A + PRD B) with implementable slices.

Changes:

  • Adds “Platform-failure handling” vocabulary (Notice, Notice Tier, Trailer) to apps/claude-code/pr-review/CONTEXT.md.
  • Publishes two PRDs and their sliced implementation issues for (A) ADO Fetcher reliability and (B) platform-failure handling rollout.
  • Removes the graduated inbox item (docs/inbox/pr-review-ado-error-hardening-pass.md).

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
docs/issues/pr-review-ado-fetcher-reliability/PRD.md PRD A describing Notice Tier doctrine + ADO Fetcher reliability plan.
docs/issues/pr-review-ado-fetcher-reliability/01-end-to-end-notice-pipeline.md Slice A1: end-to-end Notice pipeline scaffolding (notices helper + flow).
docs/issues/pr-review-ado-fetcher-reliability/02-classify-http-error-and-work-items.md Slice A2: canonical HTTP-tier mapping + work-items fetch helper.
docs/issues/pr-review-ado-fetcher-reliability/03-fetch-iterations-aborted-tier.md Slice A3: iterations fetch helper + ABORTED tier wiring.
docs/issues/pr-review-ado-fetcher-reliability/04-diff-range-sentinel.md Slice A4: DIFF_RANGE sentinel + ADR-0004 amendment plan.
docs/issues/pr-review-platform-failure-handling/PRD.md PRD B describing rollout of doctrine to Coordinator/Writer/Pre-PR with new helpers.
docs/issues/pr-review-platform-failure-handling/01-writer-http-tier-mapping.md Slice B1: parse-write-response + Writer HTTP-tier mapping application + err streaming.
docs/issues/pr-review-platform-failure-handling/02-parse-ado-writer-result-discriminated-union.md Slice B2: parseAdoWriterResult discriminated-union refactor plan.
docs/issues/pr-review-platform-failure-handling/03-coordinator-diff-range-gamma-downgrade.md Slice B3: DIFF_RANGE consumption + γ-downgrade in classify-thread.
docs/issues/pr-review-platform-failure-handling/04-coordinator-match-finding-throw.md Slice B4: match-finding throws on parse error + Coordinator Notice on catch.
docs/issues/pr-review-platform-failure-handling/05-coordinator-patch-to-fixed-mapping.md Slice B5: route PATCH-to-fixed via parse-write-response with tier handling.
docs/issues/pr-review-platform-failure-handling/06-pre-pr-notice-surface.md Slice B6: Pre-PR notices + suspicious diff detection + Gitflow-aware default branch fallback.
docs/inbox/pr-review-ado-error-hardening-pass.md Deletes inbox item after graduation into PRD A/B.
apps/claude-code/pr-review/CONTEXT.md Adds canonical definitions for Notice / Notice Tier / Trailer to align future work.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/issues/pr-review-ado-fetcher-reliability/PRD.md Outdated
Comment thread docs/issues/pr-review-platform-failure-handling/06-pre-pr-notice-surface.md Outdated
Comment thread docs/issues/pr-review-platform-failure-handling/PRD.md Outdated
orioltf and others added 5 commits May 13, 2026 19:03
PR #30 had `## ⚠ Notices` in PRD A's Notice-flow prose but `## Notices`
in the slice files — Copilot flagged the inconsistency on PRD A:49.

Reconcile to the bare heading. Justification: a mixed `info` + `warning`
Notices list would force a single heading emoji to misrepresent one
tier; per-item emoji prefixes (`ℹ️` for `info`, `⚠` for `warning`)
correctly distinguish them without polluting the heading.

Add an explicit doctrine sentence to PRD A's "Notice flow" subsection so
future contributors don't relitigate the choice.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pilot K2)

Prettier had collapsed the whitespace around several inline-code fragments
in the End-to-end demoable paragraph of B5 (lines 26), making the example
unreadable. Rewrite the paragraph with prose spacing instead of inline-code
adjacency, so the example renders cleanly in both raw markdown and GitHub.

Copilot caught this on PR #30 line 26 of the slice file.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…opilot K3)

The earlier wording read as if `git remote show origin HEAD branch` were a
single command invocation (an invalid one — `git remote show` takes a
remote name only). Reword to make clear we parse the `HEAD branch:` line
out of `git remote show origin`'s output and pass the parsed branch name
into the helper via a new `remoteHeadBranch` argument. Step A's prose
mirrors the helper signature.

Copilot caught this on PR #30 line 19 of the slice file.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Copilot K4)

PRD B's "Test-scope choice" section stated MODIFY helpers receive no new
unit tests, but slices B2/B3/B4/B6 each explicitly require 2–3 new test
cases pinning the new behaviour branches. Copilot flagged this real
contradiction on PR #30 line 81.

Reconcile by clarifying the doctrine: the user's "NEW deep modules only"
answer during grilling was about not writing whole new test suites for
MODIFY helpers; the minimal branch-verification cases each slice requires
are necessary to confirm the new behaviour landed and do not constitute
new suites. Update Test-scope choice + Modules under test sections and
two Out of Scope bullets in PRD B and its Agent Brief.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t K3 follow-up)

The K3 commit (3a63d78) only fixed slice 06's description of
detect-default-branch.mjs. PRD B itself carried four stale references to
the same helper that contradicted the slice:

- Implementation Decisions: helper signature missing `remoteHeadBranch`
  parameter; fallback chain described as `git remote show origin HEAD`
  (the exact invalid-command wording K3 was meant to remove).
- Testing Decisions: listed test cases referenced `git remote show
  succeeds` without naming the `remoteHeadBranch` injection.
- Agent Brief desired-behaviour: said the helper walks `develop → main
  → master` (omitting the `remote-show` first level).
- Agent Brief acceptance criteria: said orchestrator wires the helper
  via injectable `branchExists` only, omitting `remoteHeadBranch`.

Reconcile all four against slice 06's K3-corrected contract so an AFK
agent picking up B6 reads one consistent signature across PRD and
slice. No effect on the other 9 slices.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@orioltf orioltf merged commit 86370a1 into develop May 13, 2026
4 checks passed
@orioltf orioltf deleted the feature/pr-review/orchestrator-split-03 branch May 13, 2026 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants