sglang-humanize-review: add PR comprehension pass, exhaustive corpus sweep, and off-diff verification (#69)

Extend the review skill so a review now:

1. Opens with a mandatory PR comprehension pass — a change summary plus a Mermaid
   execution flowchart (changed nodes marked) with a prose walkthrough after each
   diagram, shown before any findings. Includes Mermaid syntax-safety rules
   (quote special-char labels, <br/> not \n) and vertical-stacking guidance so it
   renders on GitHub's pinned Mermaid.

2. Grounds findings in an exhaustive, memory-bounded sweep of the whole review
   corpus (new summarize_sglang_review_corpus.py) instead of the first-N matches,
   with a written historical-review synthesis before the findings.

3. Requires any claim depending on code outside the diff (call-site coverage,
   method shadowing, "no other caller", line numbers) to be verified against the
   PR branch, not a local checkout — preventing confident false positives.

Validated by running the full pipeline on 50 randomly sampled merged PRs from the
last 6 months: surfaced 3 independently confirmed latent bugs plus several
candidates, with no unverified false positives in the final report.

Tests updated accordingly (8 passing).

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: BBuf

README.md

@@ -43,7 +43,7 @@ find it.
 | [`llm-torch-profiler-analysis`](skills/llm-torch-profiler-analysis/) | You need a three-table profiler report that keeps `extend/prefill` and `decode` evidence separate. |
 | [`llm-pipeline-analysis`](skills/llm-pipeline-analysis/) | You need forward-pass, layer, and kernel-level timing from a torch profiler trace, including anchor boundaries and Perfetto ranges. |
 | [`model-compute-simulation`](skills/model-compute-simulation/) | You need operator shapes, FLOPs, MFU estimates, kernel-to-op mapping, or parallelism what-if analysis for an LLM serving shape. |
-| [`sglang-humanize-review`](skills/sglang-humanize-review/) | You need SGLang code-review findings grounded in full human PR review episodes from project start through the latest refresh (June 2026), including inline code context, top-level discussion, review summaries, and multi-round replies. |
+| [`sglang-humanize-review`](skills/sglang-humanize-review/) | You need SGLang code-review findings grounded in full human PR review episodes from project start through the latest refresh (June 2026), including inline code context, top-level discussion, review summaries, and multi-round replies. Every review opens with a PR comprehension pass — a change summary plus a Mermaid execution flowchart with the diff's modified steps marked — so the reviewer sees how the PR runs before the findings. |
 | [`sglang-sota-humanize-loop`](skills/sglang-sota-humanize-loop/) | You want one model-level Humanize RLCR loop that owns gap decisions, profiler triage, required layer-pipeline deep dives, SGLang patches, optional `ncu-report-skill` evidence, and real-model revalidation after the fixed fair benchmark. |
 | [`vllm-sota-humanize-loop`](skills/vllm-sota-humanize-loop/) | You want one model-level Humanize RLCR loop that owns gap decisions, profiler triage, required layer-pipeline deep dives, vLLM patches, optional `ncu-report-skill` evidence, and real-model revalidation after the fixed fair benchmark. |
 | [`sglang-prod-incident-triage`](skills/sglang-prod-incident-triage/) | You need to turn queue growth, timeouts, wrong outputs, crashes, or distributed stalls into a replay and next debug step. |

skills/sglang-humanize-review/SKILL.md

@@ -10,6 +10,11 @@ description: "Perform SGLang code review in the style of human maintainers by co
 Use this skill when the user asks for a human-style SGLang code review or wants
 review feedback that resembles SGLang maintainers instead of generic linting.
 
+Every review opens with a **PR comprehension pass**: a short change summary plus
+a Mermaid execution flowchart (with the PR's added/modified steps marked) so the
+reviewer can see how the diff actually runs before reading any findings. See
+[PR Comprehension Diagram](#pr-comprehension-diagram).
+
 The bundled corpus is collected from `sgl-project/sglang` PRs from the first
 public PR through the latest refresh (June 2026), excluding PRs authored by bots
 or obvious coding-agent accounts. The collector paginates every PR's full
@@ -27,11 +32,38 @@ review episodes, not just individual comments:
 Every episode preserves PR metadata, reviewer identity, original comment text,
 original comment language, timestamps, categories, and multi-round replies when
 GitHub exposes them. Read [references/corpus-summary.md](references/corpus-summary.md)
-first for coverage, counts, top paths, and category distribution. Do not load
-the gzip corpus directly into context; query it with the helper script.
+first for coverage, counts, top paths, and category distribution. Do not paste
+the raw gzip corpus into context; go through the helper scripts, which read it
+in memory-bounded segments and return a digest.
 
 ## Corpus Tools
 
+There are two tools. **For an actual PR review, the exhaustive sweep below is
+mandatory** (see workflow step 3); the first-N query tool is only for follow-up
+drill-downs.
+
+### Exhaustive sweep + synthesis (run this first, for every review)
+
+`summarize_sglang_review_corpus.py` scans the **whole** corpus in
+memory-bounded segments, collects **every** thread relevant to the PR (not just
+the first N), and prints an aggregate over all matches plus the top relevance-
+ranked review opinions. Pass every touched path and the PR's risk keywords;
+`--path` and `--query` are repeatable and OR-combined.
+
+```bash
+python3 skills/sglang-humanize-review/scripts/summarize_sglang_review_corpus.py \
+  --path python/sglang/srt/speculative --path python/sglang/srt/managers \
+  --query eagle --query "cuda graph" --query verify --query logprob \
+  --top 30
+```
+
+It reports `Scanned N threads ... matched M threads across K PRs` so coverage is
+explicit. Read the aggregate and the top-ranked threads, then write a short
+synthesis of the recurring historical review opinions before reviewing. Use
+`--format jsonl` to stream all matched threads when you need to read every one.
+
+### First-N lookup (follow-up drill-down only)
+
 Search the corpus by topic, path, category, or reviewer:
 
 ```bash
@@ -84,15 +116,28 @@ python3 skills/sglang-humanize-review/scripts/collect_sglang_review_corpus.py \
      maintainer-level summaries.
    - Preserve the original language of any relevant corpus examples; do not
      translate user-facing comments unless the user asks.
-3. Query similar review threads.
-   - Search by path first for touched SGLang modules.
-   - Search by risk keyword next, for example `cuda`, `kv cache`,
-     `server_args`, `openai`, `logprob`, `tp`, `dp`, `eagle`, `fp8`,
-     `benchmark`, or `pytest`.
-   - Query at least one non-inline source (`--kind pr_conversation` or
-     `--kind review_submission`) when the PR changes behavior, tests, docs,
-     benchmarking, deployment defaults, or model support.
-   - Prefer evidence from the same subsystem over broad keyword matches.
+3. Exhaustively sweep the corpus, then synthesize the historical review
+   opinions. This step is **mandatory and must finish before you write any
+   findings** — do not review off the first few hits.
+   - Run `summarize_sglang_review_corpus.py` with **every touched path**
+     (`--path`, repeatable) and the PR's risk keywords (`--query`, repeatable:
+     for example `cuda`, `kv cache`, `server_args`, `openai`, `logprob`, `tp`,
+     `dp`, `eagle`, `fp8`, `benchmark`, `pytest`). It scans all threads in
+     memory-bounded segments and aggregates every relevant match, not the first
+     N.
+   - Confirm coverage from its `Scanned N ... matched M across K PRs` line. If
+     `matched` is 0, widen paths/keywords and rerun; if it is very large, read
+     the aggregate plus the top-ranked threads and, when needed, stream the
+     full set with `--format jsonl`.
+   - Read the matched threads — especially at least one non-inline source
+     (`pr_conversation` or `review_submission`) when the PR changes behavior,
+     tests, docs, benchmarking, deployment defaults, or model support — and
+     **write a short synthesis**: the recurring concerns, what reviewers
+     blocked vs. nitpicked, repros/benchmarks they demanded, and the prevailing
+     resolution for this subsystem. Prefer same-subsystem evidence over broad
+     keyword matches. This synthesis is what the findings must be grounded in.
+   - Use `query_sglang_review_corpus.py` only afterward, to drill into a
+     specific thread or reviewer surfaced by the sweep.
 4. Add cross-skill evidence when the diff touches an area covered elsewhere in
    this repository.
    - Model-family implementation or optimization: query
@@ -105,13 +150,34 @@ python3 skills/sglang-humanize-review/scripts/collect_sglang_review_corpus.py \
      expectations for startup logs and capacity accounting.
    - Serving incidents, hangs, or distributed regressions: use
      `sglang-prod-incident-triage` style replay requirements.
-5. Produce a code-review response.
+5. Explain the PR before judging it (PR comprehension pass).
+   - This step is mandatory and always comes **before** any review findings.
+   - Goal: let a reviewer grasp *what changed* and *how the changed code runs*
+     in under a minute, without reading the whole diff.
+   - Produce a short prose summary plus a Mermaid diagram, following the
+     [PR Comprehension Diagram](#pr-comprehension-diagram) contract below.
+   - Trace the actual execution path through the changed lines: entrypoint,
+     control flow, data flow, the modified branches/calls, and where the new
+     behavior diverges from the old one. Highlight changed nodes.
+   - Read enough surrounding code (callers, callees, config wiring) to make the
+     flow correct. Do not invent functions or call edges that are not in the
+     diff or the files it touches.
+6. Produce a code-review response.
    - Lead with concrete findings ordered by severity.
    - Include file and line references from the reviewed diff.
    - Explain the failure mode, not just the preferred style.
    - Suggest a fix or validation step when the issue is actionable.
    - Keep nits separate from correctness, performance, or compatibility risks.
-6. If no issue is found, say so clearly.
+   - **Verify any claim that depends on code outside the diff against the PR
+     branch, not a local checkout.** Findings about call-site coverage, method
+     shadowing/MRO, "no other caller", or exact line numbers are base-sensitive:
+     a local repo on a different commit produces confident false positives
+     (e.g. grepping a stale `EagleVerifyInput` for a method-collision that does
+     not exist on the PR branch). Confirm with `gh pr diff`, `gh api
+     .../contents/<path>?ref=<pr-sha>`, or `git show <pr-sha>:<path>`. If only a
+     mismatched checkout is available, label the finding "needs branch
+     verification" rather than asserting it.
+7. If no issue is found, say so clearly.
    - Mention the main residual risk and the test or benchmark coverage that
      would increase confidence.
 
@@ -144,6 +210,71 @@ threads in the corpus:
 - **Observability**: review metrics, logs, warning levels, traceability, and
   error messages when operational behavior changes.
 
+## PR Comprehension Diagram
+
+Before findings, emit a comprehension block so the reviewer understands the
+PR's principle at a glance. It has two parts:
+
+1. **Change summary (2-6 bullets).** Plain language: what subsystem is touched,
+   the core mechanism the PR changes, and the one or two lines that carry the
+   real behavior change. Name the entrypoint(s) and the touched files.
+2. **A Mermaid flowchart** of the execution logic for the PR-relevant path,
+   with changed steps visually marked, **each diagram immediately followed by a
+   prose walkthrough of its details**.
+
+Diagram rules:
+
+- Use a fenced ` ```mermaid ` block with `flowchart TD` (or `LR` for short
+  linear flows). This renders on GitHub PR comments and most markdown viewers.
+- Model the **runtime execution path**, not the file tree: entrypoint → control
+  flow (branches/loops) → the calls and data transforms the PR adds or changes →
+  return/side-effect. The reader should see how a request/tensor/batch actually
+  flows through the changed code.
+- Mark nodes the PR adds or modifies with the `changed` class and keep
+  untouched context nodes plain, so old vs. new behavior is obvious. Always
+  include the legend node.
+- Annotate edges with the condition or data that flows along them
+  (`-->|fp8 path|`, `-->|cache miss|`) when a branch is where the behavior
+  changes.
+- Keep it to roughly 6-14 nodes. If the PR spans independent code paths, emit
+  one small flowchart per path as **separate ` ```mermaid ` blocks**, stacked
+  vertically (one after another, never two side by side) — do not pack two
+  `subgraph`s into one block, which lays them out horizontally and shrinks each
+  to an unreadable size. For a pure refactor with no control-flow change, show
+  old-vs-new as two short branches and say so.
+- Prefer `flowchart TD` (top-down) so the graph grows vertically and stays
+  legible; reserve `LR` for a genuinely short linear chain.
+- Immediately after each diagram, add a short prose walkthrough of *that*
+  diagram: what the entrypoint is, what each branch/condition means, and which
+  nodes the PR changed and why. The picture orients; the walkthrough is what
+  the reviewer reads. Never drop a diagram without explaining it.
+- Reference real symbols (`function`, `ClassName.method`, file:line) in node
+  labels so the diagram is verifiable against the diff. Do not fabricate edges.
+- Syntax safety, so the block renders on GitHub's pinned Mermaid, not just the
+  latest CLI: wrap any node or edge label containing `()`, `,`, `:`, `=`, `>`,
+  `&`, or `/` in double quotes — `C["down_proj(x, skip_all_reduce=rs)"]`,
+  `B -->|"id < 0 or id >= vocab"| N`. Use `<br/>` for line breaks inside a
+  label, never a literal `\n`. Put `classDef changed stroke-dasharray:5 5,stroke-width:2px;`
+  once at the end. Quoting unconditionally is the safe default.
+
+Skeleton to adapt (replace labels with the PR's real symbols and paths):
+
+```mermaid
+flowchart TD
+    A["Entry: forward / handler / scheduler step"] --> B{"Branch the PR changes"}
+    B -->|"new condition"| C["New/changed call or transform"]:::changed
+    B -->|"existing path"| D["Unchanged path"]
+    C --> E["Downstream effect: KV write / output / metric"]:::changed
+    D --> E
+    E --> F["Return / response / side effect"]
+
+    L["Legend: dashed border = added or modified by this PR"]:::changed
+    classDef changed stroke-dasharray:5 5,stroke-width:2px;
+```
+
+Place this comprehension block first in the response, then the findings. Keep
+it tight; it orients the reader, it is not the review itself.
+
 ## Review Style
 
 Mirror human SGLang review habits:
@@ -164,13 +295,23 @@ Mirror human SGLang review habits:
 
 For a normal review, return:
 
-- Findings first, ordered by severity, with file/line references.
+- A PR comprehension block first: change summary plus the Mermaid execution
+  flowchart(s) with changed nodes marked, each diagram followed immediately by
+  a prose walkthrough of its details (see
+  [PR Comprehension Diagram](#pr-comprehension-diagram)).
+- A short **historical review synthesis** next: state the sweep coverage
+  (`scanned / matched / PRs`) and summarize the recurring review opinions for
+  this subsystem from the exhaustive corpus sweep (workflow step 3) that the
+  findings build on.
+- Findings next, ordered by severity, with file/line references, explicitly
+  grounded in that synthesis where a precedent applies.
 - Open questions or assumptions.
 - Test or benchmark gaps.
 - A short summary only after findings.
 
 For a review-prep pass before the user opens a PR, return:
 
+- the PR comprehension block (change summary plus Mermaid execution flowchart)
 - likely reviewer concerns
 - missing tests or benchmark evidence
 - suggested patch cleanup