hsliuustc0106 · Oct 2, 2026
@@ -1,6 +1,6 @@
 ---
 name: review-pr
-description: Review pull requests and local branches for vllm-project/afd-plugin with a frozen snapshot, module-design ownership, feature overlays, patch-contract enforcement, targeted validation, and concise evidence-backed findings. Use for default, detailed, or repeat reviews; checking correctness, NPU/GPU compatibility, vLLM 0.26.0 patch discipline, connectors, distributed topology, workers and model runners, tests, benchmarks, or model-family additions; and identifying or explicitly requesting code-owner reviewers. Use run-e2e instead to execute E2E suites, and AGENTS.md for authoring rules.
+description: Review pull requests and local branches for vllm-project/afd-plugin with a frozen snapshot, module-design ownership, feature overlays, patch-contract enforcement, targeted validation, and concise evidence-backed findings. Use for default, detailed, or repeat reviews; checking correctness, NPU/GPU compatibility, pinned-upstream patch discipline, connectors, distributed topology, workers and model runners, tests, benchmarks, or model-family additions; and identifying or explicitly requesting code-owner reviewers. Use run-e2e instead to execute E2E suites, and AGENTS.md for authoring rules.
 ---
 
 # Review AFD Plugin Pull Requests
@@ -9,6 +9,17 @@ Review like a maintainer: direct, selective, and focused on issues that CI does
 not prove. Prefer a few high-confidence findings over exhaustive commentary.
 Zero findings is a valid result.
 
+## Target-version evidence
+
+Resolve version-sensitive rules from the frozen target: read
+`afd_plugin/compat/vllm.py`, `docker/Dockerfile.ci`, Python/package metadata,
+Ascend patch provenance, CI commands, pytest markers, and relevant fixtures.
+Version literals in these references describe the baseline they were written
+against; they are not permanent pins for newer or older branches. Compare
+patches to the target's actual upstream ref, preserve AFD-off behavior, and
+verify coordinated pin changes. Do not treat a draft module proposal or a
+hardware-unavailable test as a proved blocker or passing validation.
+
 ## Quality contract
 
 Make every finding:
@@ -98,6 +109,15 @@ drift.
 | [delivery-style.md](references/delivery/delivery-style.md) | Findings are ready for concise maintainer-style delivery. |
 | [review-requests.md](references/delivery/review-requests.md) | The user asks to identify, suggest, request, or ping code-owner reviewers. |
 
+## Optional review workers
+
+For independent defect discovery on a large diff, or when the user selects
+`auto`, `codex`, `claude`, or `none`, read
+[reviewer-backends.md](references/process/reviewer-backends.md). Pass immutable
+base/head SHAs and a bounded read-only scope. The parent owns domain routing,
+verification, deduplication, severity, and any separately authorized posting.
+Missing optional tooling falls back to direct review without blocking it.
+
 ## Workflow
 
 ### 1. Freeze and report the snapshot

@@ -11,7 +11,7 @@ Accept a claim only with base and head runs that hold constant:
 - Hardware pool and placement (CI presets: `l4_1`–`l4_4`, `h100_4`; NPU:
   documented manual environment per `docs/npu/TESTING.md`).
 - Container image and dependency versions (CI image pins
-  `VLLM_BASE_TAG=v0.26.0`).
+  `VLLM_BASE_TAG` from the frozen target).
 - Workload and datasets (`tools/benchmarks/decode_bench.py` or the named E2E
   scenario; GSM8K-7 for accuracy gates).
 - Graph/eager mode, DBO on/off, ubatching mode, and topology (2A2F/2A1F,

@@ -21,7 +21,7 @@ The template requires Purpose / Issue / Scope / Implementation Notes / Test
 Plan / Test Result / Docs Impact, plus the Essential PR Checklist. Verify the
 high-signal items rather than restating them:
 
-- vLLM 0.26.0 compatibility considered; no vLLM source-checkout changes.
+- Target-pinned vLLM compatibility considered; no vLLM source-checkout changes.
 - Plugin-owned classes or dotted paths preferred over monkey patches; any
   shim isolated, idempotent, version-guarded, documented, tested.
 - Imports remain CPU-safe; CUDA-heavy work delayed or GPU-gated.

@@ -17,7 +17,7 @@ Designs: `docs/design/module/attention_runtime.md`,
   or mixed batches; the FULL_DECODE_ONLY gate stays enforced where the
   connector contract declares it.
 - Keep ACL graph patches pinned: `mla_graph` patches
-  `vllm_ascend/attention/mla_v1.py` at commit `80d8c194f` — apply the patch
+  the target's documented Ascend attention source — apply the patch
   contract (markers, upstream source, signature parity) and re-verify the copy
   against that ref.
 - Preserve the eager escape hatch: every graph path has a tested eager

@@ -15,8 +15,8 @@ engine/GPU: `async_dp_engine.py`, `async_dp_forward_context.py`,
   new monkey patch; a new patch needs an architectural-review rationale:
   correct upstream target, minimal scope, understood performance impact, and a
   long-term upstream-or-removal plan.
-- Copy the upstream function wholesale from the pinned ref (vLLM 0.26.0;
-  vLLM-Ascend commit `80d8c194f`) and mark only AFD-specific differences with
+- Copy the upstream function wholesale from the target's pinned vLLM or
+  vLLM-Ascend ref and mark only AFD-specific differences with
   `# ### PATCH START:` / `# ### PATCH END:`; keep the marker text short and
   specific.
 - Keep a patched function's signature and return type identical to upstream;

@@ -16,7 +16,7 @@ mechanics ([compat-patches.md](compat-patches.md)) or worker internals
 - Keep one owner for attention→FFN routing; models and workers must not
   independently select a route or a peer.
 - Keep the async-DP patches externally transparent: non-AFD requests observe
-  upstream vLLM 0.26.0 scheduling, output order, and finish semantics
+  the pinned upstream vLLM scheduling, output order, and finish semantics
   unchanged, on both `EngineCoreProc` and `DPEngineCoreProc` paths.
 - Preserve request identity and ordering across the two roles; correlate every
   connector result, ubatch work item, and DBO yield to the originating

@@ -23,8 +23,8 @@ patch application ([npu-compat.md](npu-compat.md)).
 - Pair `csrc/` kernel changes (a2e/e2a hosts and kernels) with the torch
   adapter/extension updates they require, and state accuracy and performance
   impact; kernel behavior changes need NPU evidence, never simulation.
-- Keep version pins coherent: vLLM 0.26.0 (`TARGET_VLLM_VERSION`) and
-  vLLM-Ascend `80d8c194f` move together across the compat gate, docker base
+- Keep the target's version pins coherent: `TARGET_VLLM_VERSION` and
+  the documented vLLM-Ascend ref move together across the compat gate, docker base
   image, docs, and design-page `upstream_refs`, or the PR states why not.
 - Keep GPU path parity visible: `csrc/gpu/` is reserved — a PR adding GPU
   native ops must state where the GPU counterpart lives.

@@ -16,9 +16,9 @@ model wrappers ([model-integration.md](model-integration.md)).
   fails fast on scheduler-driven calls; attention-side runners never construct
   FFN components and vice versa.
 - Keep runner V1/V2 in pairs: a change to shared runner behavior lands in both
-  variants or states the exclusion; GPU keeps V2 gated off
-  (`VLLM_USE_V2_MODEL_RUNNER=0` required) while NPU V2 exists — flag any change
-  that silently moves that gate.
+  variants or states the exclusion. Resolve GPU/NPU V1/V2 support from the
+  target's feature validation and E2E scenarios; a gate from an older release
+  is not evidence that the reviewed target forbids the combination.
 - Enforce the patch contract on every patched runner method (the NPU
   attention model runner is the heaviest patch site): marked AFD differences,
   upstream-copy fidelity against the pinned ref, identical signatures.

@@ -19,8 +19,8 @@ vLLM engine/config ingress (--additional-config {"afd": ...})
 - Cover the matrix the change can affect: GPU/NPU, graph/eager, DBO on/off,
   2A2F/2A1F, runner V1/V2, colocation/disaggregation, AFD-on/AFD-off.
 - Patched or wrapper code must be transparent when AFD is inactive
-  (`is_afd_active` false): a non-AFD request must keep upstream vLLM 0.26.0
-  behavior exactly.
+  (`is_afd_active` false): a non-AFD request must preserve behavior at the target's pinned upstream
+  version exactly.
 - Search bounded callers and sibling implementations (the GPU vs NPU twin of
   every worker/model file) rather than assuming the changed hunk is the only
   path; the two platforms drift independently.
@@ -40,9 +40,11 @@ rules, not style nits:
    New mutable globals require approval and must be justified in the PR.
 5. Do not split simple functions into helper layers; extract only for real
    complexity, real duplication, or an established local pattern.
-6. Keep imports CPU-safe at module level: importing `afd_plugin` (or any unit
-   test module) must not import torch, torch_npu, or vLLM runtime modules.
-   Defer or `importorskip` them inside functions/tests.
+6. Keep plugin registration and the CPU-selected test lane import-safe.
+   Inspect the target's lazy exports, collection hooks, and runtime guards;
+   defer optional runtime imports or skip them as required by that lane.
+   Runtime-only modules may import their declared dependencies: prove that a
+   changed import reaches a CPU-only boundary before reporting a defect.
 7. New public behavior needs a named owner for its design page and a docs
    impact statement in the PR template.
 8. Performance-affecting changes need a comparable A/B claim or an explicit
@@ -54,7 +56,7 @@ rules, not style nits:
 | --- | --- |
 | Correctness | A reachable input reaches the changed code and produces wrong output, a hang, or a crash. |
 | Patch-contract drift | A patched function's signature, return type, or upstream copy diverges from the pinned ref, or missing/incorrect `# ### PATCH` markers, `Patch reason`/`Patch functionality` comments, or `# Upstream source:` pointers break upgrade comparability. |
-| Version/compat | The change breaks the vLLM 0.26.0 / vLLM-Ascend `80d8c194f` contract, or moves a pin without updating the version gate, docker base, docs, and design pages together. |
+| Version/compat | The change breaks the target's pinned vLLM / vLLM-Ascend contract, or moves a pin without updating the version gate, docker base, docs, and design pages together. |
 | Platform divergence | The GPU or NPU twin (worker/runner/model variant) is not updated and the change is not provably platform-neutral. |
 | CPU-safe imports | A new module-level import of torch/torch_npu/vLLM runtime breaks `pytest -m "not gpu and not vllm_runtime"` or plugin import on a CPU host. |
 | Concurrency/async | Async-DP busy loops, ubatching state machines, DBO yield, or connector transfer states can deadlock, drop, reorder, or leak work items on abort/timeout. |

@@ -0,0 +1,80 @@
+# Reviewer Backends
+
+Use this reference when delegating defect discovery to Codex, Claude, or another reviewer. The parent `review-pr` workflow remains responsible for gates, domain routing, verification, prioritization, and any GitHub writes.
+
+## Backend Selection
+
+| Choice | Behavior |
+|--------|----------|
+| `auto` | Use one available reviewer backend; fall back to direct review |
+| `codex` | Use an available Codex review agent or skill |
+| `claude` | Use an available Claude review agent or CLI |
+| `none` | Review directly without delegation |
+
+Honor an explicit user choice. Do not block when the requested backend is unavailable; report the fallback locally and continue directly unless the user required that backend specifically.
+
+Use one backend for ordinary code changes. Use at most two reviewers for large or high-risk changes, either with non-overlapping path scopes or as independent passes. Do not run multiple reviewers merely to vote on the same conclusion.
+
+## Common Read-Only Handoff
+
+Give every backend the same concrete target:
+
+```yaml
+repository: /absolute/path/to/afd-plugin
+base_ref: origin/main
+base_sha: <resolved-base-sha>
+head_sha: <captured-pr-head-sha>
+merge_base_sha: <resolved-merge-base-sha>
+scope: complete-diff | path-partition
+paths: []
+focus: []
+constraints:
+  read_only: true
+  no_github_writes: true
+  no_delegation: true
+output:
+  finding: "[P0-P3] Imperative title — path:line"
+  exhaustive: true
+  include_assessment: true
+  include_test_gaps: true
+  include_residual_risks: true
+```
+
+Capture immutable SHAs before delegation. Ask the reviewer to:
+
+1. Read the applicable `AGENTS.md`.
+2. Inspect the complete assigned diff and enough surrounding code to understand it.
+3. Check relevant tests and call sites.
+4. Report only concrete regressions introduced by the change.
+5. Cite the smallest changed-line range that demonstrates each finding.
+6. Continue through the full assigned scope after finding the first issue.
+
+The worker must not modify files, create commits, push branches, post comments, submit a review event, or delegate again.
+
+## Backend Adapters
+
+### Codex
+
+Use an available Codex review agent or review skill; do not assume that a named system skill is installed. Give it the common handoff plus selected AFD focus areas. Use it for a complete change or bounded path partition, not for a one-function context lookup.
+
+### Claude
+
+Pass the same handoff and output format to the available Claude review surface. Do not assume a particular command, model, or installation path. Explicitly include the read-only and no-GitHub-write constraints in the prompt.
+
+### Direct fallback
+
+If no backend is available, the parent performs the same checks. Keep the same evidence threshold and comment budget. Do not mention missing tooling in the GitHub review unless the user asks.
+
+## Aggregate Results
+
+The parent reviewer must:
+
+1. Re-read the PR head SHA. If it changed, discard stale line mappings and rerun affected scopes.
+2. Verify each finding against code, tests, and the captured diff.
+3. Drop speculative, pre-existing, intentional, or style-only findings.
+4. Deduplicate by root cause and affected call path rather than by line alone.
+5. Classify `blocking` independently from `P0`-`P3`; priority alone does not determine the verdict.
+6. Apply the AFD comment budget after collecting the complete result.
+7. Preserve material omitted findings, test gaps, and residual risks in the local report.
+
+Only the parent may turn selected findings into GitHub comments, and only with user authorization.