This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/doris-skills.git
The following commit(s) were added to refs/heads/main by this push:
new a535621 feat: add doris-repo-review skill and reorganize the README
(#6)
a535621 is described below
commit a535621ae6907965d34056192596da86745cb2e5
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Mon Aug 17 14:11:56 2026 +0800
feat: add doris-repo-review skill and reorganize the README (#6)
* feat: add doris-repo-review skill
Adds a contributor-side skill that reviews an apache/doris Pull Request
against a local source clone, running the same flow as the repository's CI
Code Review Runner (.github/workflows/code-review-runner.yml):
- align the current worktree to the PR head, refusing to switch when tracked
files are modified (never commits, stashes, resets or deletes for the
user);
- build one authoritative context: three-dot diff, new-side changed line
ranges, required AGENTS.md guides, existing inline review threads;
- main-agent risk scan, then 1-3 full-review subagents plus risk-focused
subagents converging over a shared ledger for at most 3 rounds;
- emit an English and a Chinese review document under review-docs/.
A local run has no GitHub inline comments, so every finding carries a
`path:line` anchor plus a verbatim snippet, and scripts/verify-anchors.py
proves the anchors resolve and that both documents expose the same finding
IDs.
The review is read-only: no build, no tests, no repository source changes
and
nothing posted to GitHub. Unlike the other skills here it works on a Doris
source checkout rather than a running cluster, which the README now states.
The README layout tree is also brought back in sync with skills/.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* docs: reorganize README around the two families of skills
The repository is no longer only cluster-diagnosis knowledge, so the docs no
longer read as if it were.
README:
- open with what a skill is and the two kinds of work covered (working with
a
cluster / working on Doris itself) instead of the deployment-only framing;
- split the Skills table by family and give every row a "Use it when"
column,
plus a link to the skill directory;
- add a "You say / what loads" table so a reader can see how skills are
actually reached;
- move the doriscli setup out of the opening paragraphs into "Install ->
Optional: doriscli", where it belongs - it is a cluster-side dependency,
and
doris-repo-review needs git/gh/jq/python3 instead;
- state which skills have no L1/L2 coverage and that adding cases for a new
skill is welcome but not a merge requirement;
- add "Contributing a skill": the directory layout, what the description
has to
do for the router, and the existing brand-neutrality / CLI-contract rules,
which were previously filed under "Authoring rules" with no surrounding
context.
CLI-CONTRACT.md and verify/README.md: scope both to the cluster-side
skills, so
"these skills" no longer implies coverage of skills they do not describe.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
---------
Co-authored-by: morningman <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
CLI-CONTRACT.md | 7 +-
README.md | 147 +++++--
skills/doris-repo-review/SKILL.md | 372 ++++++++++++++++++
.../doris-repo-review/references/doc-templates.md | 227 +++++++++++
skills/doris-repo-review/references/prompts.md | 142 +++++++
skills/doris-repo-review/scripts/align-to-pr.sh | 243 ++++++++++++
.../scripts/prepare-review-context.sh | 437 +++++++++++++++++++++
skills/doris-repo-review/scripts/verify-anchors.py | 172 ++++++++
verify/README.md | 4 +-
9 files changed, 1704 insertions(+), 47 deletions(-)
diff --git a/CLI-CONTRACT.md b/CLI-CONTRACT.md
index d526cbb..9ec6ee5 100644
--- a/CLI-CONTRACT.md
+++ b/CLI-CONTRACT.md
@@ -1,8 +1,9 @@
# doriscli contract
-These skills are written directly against **`doriscli`** (the Apache Doris
CLI, shipped
-in the companion `doris-cli` repository). The runtime-diagnosis logic in
-`doris-best-practices` hard-codes the command names and JSON field names below.
+The cluster-side skills are written directly against **`doriscli`** (the
Apache Doris CLI,
+shipped in the companion `doris-cli` repository). The runtime-diagnosis logic
in
+`doris-best-practices` hard-codes the command names and JSON field names
below. Skills that
+never talk to a cluster (for example `doris-repo-review`) are outside this
contract.
**If doriscli renames a command or output field, update both repositories
together.**
There is no render/adapter layer to absorb the change, and a renamed field
silently
diff --git a/README.md b/README.md
index a32a5ee..598efc7 100644
--- a/README.md
+++ b/README.md
@@ -1,38 +1,49 @@
# Agent Skills for Apache Doris
-Apache Doris agent skills in the open Agent Skills (`SKILL.md`) format —
usable by
-Claude Code, Cursor, Codex, Cline, Amp, and other agent tools. This repository
contains the
-**kernel knowledge** for any Apache Doris deployment: table design, sizing,
query
-investigation, and architecture decisions.
-
-The skills pair with **`doriscli`**, the Apache Doris CLI (companion
[`doris-cli`](https://github.com/apache/doris-cli)
-repository). doriscli is the "hands" — it runs SQL, fetches and parses query
profiles,
-and analyzes tablet distribution, all as structured JSON; these skills are the
"brain" —
-the decision logic that reads that JSON and recommends table designs, sizing,
and fixes.
-doriscli is optional: when it is not installed the skills fall back to plain
-MySQL-protocol SQL plus the FE HTTP API, so they work with or without it (see
-`skills/doris-best-practices/references/cli-investigation.md`).
-
-The exact doriscli commands and JSON fields the skills depend on are listed in
-[CLI-CONTRACT.md](CLI-CONTRACT.md) — keep that file and doriscli in sync.
+Agent skills for Apache Doris in the open Agent Skills (`SKILL.md`) format —
usable by
+Claude Code, Cursor, Codex, Cline, Amp, and other agent tools.
+
+A skill is the decision logic an agent is missing: what evidence to collect,
how to read it,
+which conclusion it does and does not support. Everything here is **Apache
Doris kernel
+knowledge** — vendor-neutral, no downstream distribution's branding.
+
+The repository covers two kinds of work:
+
+- **Working with a cluster** — designing tables, sizing, diagnosing production
incidents,
+ reading query profiles.
+- **Working on Doris itself** — contributor workflows against an
`apache/doris` source checkout.
Cluster-lifecycle, billing, and networking operations are managed-service
specific and
-intentionally out of scope here; use your platform's cluster-management
console for those.
+intentionally out of scope; use your platform's cluster-management console for
those.
## Skills
-| Skill | Description |
-|---|---|
-| `doris-best-practices` | Apache Doris table design, sizing, and runtime
query investigation (37 rules, 7 use-case templates, 4 sizing guides) |
-| `doris-architecture-advisor` | Workload-aware architecture design (8
decision rules, 10 worked industry examples) |
-| `doris-debug` | Production diagnostic suite: symptom router + 10 domain
skills (query, import, compaction, node, MV, tablet, deployment, data-lake,
resource-isolation, cloud), 16 case files, 45 case patterns |
-| `doris-profile-reader` | Query runtime profile interpretation and bottleneck
triage (counter semantics, join-order / runtime-filter diagnosis, 9 reference
guides) |
+Every skill is self-contained — install all of them or only the ones you need.
Agents select a
+skill from its `description`, so in practice you describe the problem and the
right one loads.
+
+### Working with a cluster
+
+| Skill | What it does | Use it when |
+|---|---|---|
+| [`doris-best-practices`](skills/doris-best-practices/) | Table design,
sizing, and runtime query investigation (37 rules, 7 use-case templates, 4
sizing guides) | Writing or reviewing `CREATE TABLE`, choosing a data model,
partition/bucket strategy, or cluster configuration |
+| [`doris-architecture-advisor`](skills/doris-architecture-advisor/) |
Workload-aware architecture design (8 decision rules, 10 worked industry
examples) | Turning a business workload into a Doris design — model choice,
ingestion strategy, sizing-first planning |
+| [`doris-debug`](skills/doris-debug/) | Production diagnostic suite: symptom
router + 10 domain skills (query, import, compaction, node, MV, tablet,
deployment, data-lake, resource-isolation, cloud), 16 case files, 45 case
patterns | Something is broken — slow queries, failing imports, `-235`
compaction errors, OOM or crashing nodes, an MV that will not rewrite, degraded
tablets |
+| [`doris-profile-reader`](skills/doris-profile-reader/) | Query runtime
profile interpretation and bottleneck triage (counter semantics, join-order /
runtime-filter diagnosis, 9 reference guides) | You have a profile, query id,
or profile URL and need to know what actually made the query slow |
-## How to use
+### Working on Doris itself
-Install with the open [`skills`](https://github.com/vercel-labs/skills) CLI —
it works
-across agent tools (Claude Code, Cursor, Codex, Cline, Amp, …), auto-discovers
both
-skills in this repo, and bundles their `references/`:
+| Skill | What it does | Use it when |
+|---|---|---|
+| [`doris-repo-review`](skills/doris-repo-review/) | Contributor-side PR
review of a local `apache/doris` clone, running the same flow as the CI Code
Review Runner: worktree alignment, risk scan, subagent fan-out, shared ledger,
anchored EN + ZH review documents | Reviewing an `apache/doris` pull request
locally, the way the pipeline would |
+
+These skills read a Doris **source** checkout and never touch a cluster; the
cluster-side skills
+never touch repository source. Nothing here builds Doris, runs tests, or
writes to GitHub.
+
+## Install
+
+The open [`skills`](https://github.com/vercel-labs/skills) CLI works across
agent tools
+(Claude Code, Cursor, Codex, Cline, Amp, …), auto-discovers every skill in
this repository, and
+bundles their `references/` and `scripts/`:
```bash
npx skills add apache/doris-skills
@@ -41,26 +52,53 @@ npx skills add apache/doris-skills
Or copy them in manually:
```bash
-cp -r skills/* ~/.claude/skills/
+cp -r skills/* ~/.claude/skills/ # all of them
+cp -r skills/doris-debug ~/.claude/skills/ # or just one
```
-For the structured-diagnosis path (query profiles, tablet health), also install
-doriscli. The recommended way is from **npm**, which ships prebuilt binaries
(no
-Rust toolchain, no compile step):
+Then describe your problem — the agent loads the matching skill on its own:
+
+| You say | What loads |
+|---|---|
+| "Design tables for a 2 TB/day clickstream workload" |
`doris-architecture-advisor` → `doris-best-practices` |
+| "Review this DDL before I ship it" | `doris-best-practices` |
+| "Imports keep failing with -235" | `doris-debug` → its compaction domain
skill |
+| "Why is this query slow? here's the profile" | `doris-profile-reader` |
+| "/doris-repo-review https://github.com/apache/doris/pull/66807"; |
`doris-repo-review` |
+
+### Optional: doriscli
+
+The cluster-side skills pair with **`doriscli`**, the Apache Doris CLI
(companion
+[`doris-cli`](https://github.com/apache/doris-cli) repository). doriscli is
the "hands" — it runs
+SQL, fetches and parses query profiles, and analyzes tablet distribution, all
as structured JSON;
+the skills are the "brain" — the decision logic that reads that JSON and
recommends table
+designs, sizing, and fixes.
+
+The recommended install is from **npm**, which ships prebuilt binaries (no
Rust toolchain, no
+compile step):
```bash
npm install -g @apache-doris/doriscli
doriscli --version
```
-For other platforms, or to build from source, see the companion
[`doris-cli`](https://github.com/apache/doris-cli)
-repository. Without doriscli, the skills use the SQL + HTTP fallback
automatically.
+For other platforms, or to build from source, see the
+[`doris-cli`](https://github.com/apache/doris-cli) repository.
+
+doriscli is optional: without it the skills fall back automatically to plain
MySQL-protocol SQL
+plus the FE HTTP API (see
+[`cli-investigation.md`](skills/doris-best-practices/references/cli-investigation.md)).
The exact
+commands and JSON fields the skills depend on are listed in
[CLI-CONTRACT.md](CLI-CONTRACT.md) —
+keep that file and doriscli in sync.
+
+`doris-repo-review` does not use doriscli; it needs `git`, an authenticated
`gh` CLI, `jq`, and
+`python3`.
## Verification
-The factual claims in these skills are regression-tested — against a real
Apache Doris
-cluster, against `doriscli`, and against the skills' own runtime behavior — so
they don't
-drift from reality. See [`verify/`](verify/README.md); it runs in three layers:
+The factual claims in the cluster-side skills are regression-tested — against
a real Apache Doris
+cluster, against `doriscli`, and against the skills' own runtime behavior — so
they do not drift
+from reality. See [`verify/`](verify/README.md); it runs in three layers:
- **L1 — knowledge**: every DDL template and gotcha in `doris-best-practices`
is accepted
or rejected by a live cluster exactly as the skill claims (`mysql` client).
@@ -70,27 +108,52 @@ drift from reality. See [`verify/`](verify/README.md); it
runs in three layers:
an end-to-end advisor→DDL→live-cluster loopback, and skill-router triggering
— exercised
through a nested `claude -p`.
-## Authoring rules
+A skill that makes no DDL or doriscli claim (`doris-debug`,
`doris-profile-reader`,
+`doris-repo-review`) has no L1/L2 coverage today. Adding cases for a new skill
is welcome but not
+a merge requirement.
+
+## Contributing a skill
+
+A skill is one directory under `skills/`:
+
+```
+skills/<skill-name>/
+├── SKILL.md # required — frontmatter + the workflow the agent follows
+├── references/ # optional — depth the agent loads on demand
+├── scripts/ # optional — deterministic work better done in code than
by the model
+└── <domain>/ # optional — nested sub-skills, each with its own
SKILL.md (see doris-debug)
+```
+
+`SKILL.md` starts with `name` and `description` frontmatter. The `description`
is the only thing
+the router sees, so it must state **what the skill does and when to reach for
it**, in the words a
+user would actually type. Keep `SKILL.md` itself to the decision logic and
push depth into
+`references/` — the agent reads those on demand.
+
+Rules:
-- Write `doriscli` and `Apache Doris` directly. **Never** introduce a
downstream
- distribution's brand strings — CLI names, console URLs, env-var prefixes,
hostnames.
+- Write `doriscli` and `Apache Doris` directly. **Never** introduce a
downstream distribution's
+ brand strings — CLI names, console URLs, env-var prefixes, hostnames.
- Cloud-mode behavior (storage-compute separation, `replication_num=1`, file
cache) is
**Doris kernel architecture** — describe it neutrally as "cloud mode" /
"storage-compute".
Managed-service behavior (suspend/resume, automatic node-count management,
billing) is
out of scope; point at the user's cluster-management console rather than
naming a product.
- If you rely on a new doriscli command or output field, add it to
[CLI-CONTRACT.md](CLI-CONTRACT.md) so the two repositories stay in sync.
+- Add a row to the matching table above so the skill is discoverable from this
README.
## Layout
```
doris-skills/
├── README.md
-├── CLI-CONTRACT.md # doriscli commands + JSON fields the skills
depend on
+├── CLI-CONTRACT.md # doriscli commands + JSON fields the
skills depend on
├── skills/
-│ ├── doris-best-practices/
-│ └── doris-architecture-advisor/
-└── verify/ # regression suite for the skills' claims — L1
DDL, L2 CLI, L3 behavior
+│ ├── doris-best-practices/ # cluster-side: table design, sizing,
query investigation
+│ ├── doris-architecture-advisor/ # cluster-side: workload → architecture
+│ ├── doris-debug/ # cluster-side: symptom router + 10 nested
domain skills
+│ ├── doris-profile-reader/ # cluster-side: query profile → bottleneck
+│ └── doris-repo-review/ # contributor-side: reviews an
apache/doris PR, not a cluster
+└── verify/ # regression suite — L1 DDL, L2 CLI
contract, L3 behavior
```
## License
diff --git a/skills/doris-repo-review/SKILL.md
b/skills/doris-repo-review/SKILL.md
new file mode 100644
index 0000000..6667d22
--- /dev/null
+++ b/skills/doris-repo-review/SKILL.md
@@ -0,0 +1,372 @@
+---
+name: doris-repo-review
+description: Given a PR URL (`/doris-repo-review
https://github.com/apache/doris/pull/66807`), first check whether the current
directory's branch and commit match that PR, and if they do not, align the
current directory to the PR head without disturbing local work (refuse to
switch when tracked files are modified and hand the decision back to the user);
then review it with the same pipeline apache/doris CI runs (Code Review Runner)
- main-agent risk scan, 1-3 full-review subagents plus r [...]
+---
+
+# Local pipeline-style Doris code review
+
+This skill reviews a Pull Request against a **local clone of the Apache Doris
source repository**.
+It is a contributor-side workflow, not a cluster-operations skill: it never
talks to a running
+cluster and never needs one.
+
+Usage:
+
+```
+/doris-repo-review https://github.com/apache/doris/pull/66807
+/doris-repo-review https://github.com/apache/doris/pull/66807 focus on class
loading and compatibility
+/doris-repo-review 66807 # a bare number defaults to apache/doris
+```
+
+This ports the review flow of
`apache/doris/.github/workflows/code-review-runner.yml` to a local
+machine: **first align the current directory to the PR head**, take the diff
from local git, and
+produce one English and one Chinese document under the current directory's
`review-docs/`.
+Everything else - required reading, main-agent risk scan, subagent split,
shared ledger, at most
+3 convergence rounds, an explicit conclusion per checkpoint - matches CI.
+
+**The scope is the one worktree you are standing in.** No other worktree is
scanned, chosen, or
+created; no environment variable selects a different directory; nothing is
assumed about the
+machine's directory layout.
+
+Let `$S` be this skill's own `scripts/` directory (with a default Claude Code
install that is
+`~/.claude/skills/doris-repo-review/scripts`):
+
+```bash
+$S/align-to-pr.sh <PR> --check # diagnosis only, changes
nothing
+$S/align-to-pr.sh <PR> --out "$CTX/align.env" # step 1: align the
current directory
+$S/prepare-review-context.sh --ctx "$CTX" --align "$CTX/align.env" # step 2:
gather context
+python3 $S/verify-anchors.py --ctx "$CTX" --doc <en> --doc <zh> # step 9:
verify anchors
+```
+
+| File | Purpose |
+|---|---|
+| `scripts/align-to-pr.sh` | Resolve the PR, diagnose how the current
directory relates to it, align it to the PR head |
+| `scripts/prepare-review-context.sh` | Produce the authoritative diff,
new-side line ranges, required AGENTS.md list, existing comments, ledger
skeleton |
+| `scripts/verify-anchors.py` | Check that every `path:line` anchor really
exists and that both documents expose the same finding IDs |
+| `references/prompts.md` | Subagent prompt templates (CI wording, carried
over verbatim) |
+| `references/doc-templates.md` | Templates for both documents, anchor format,
verdict rule |
+
+Requirements: `git`, an authenticated `gh` CLI, `jq`, and `python3`. The clone
must have full
+history (`git fetch --unshallow` on a shallow one), because the authoritative
diff is a three-dot
+diff from the merge base.
+
+---
+
+## 0. Ground rules
+
+1. **Touch only the current directory, and never disturb local work.** When
the current directory
+ has **modified tracked files**, refuse to switch, report the situation, and
let the user commit
+ or stash it themselves - **never stash, reset, or delete a branch on the
user's behalf**, and
+ never go hunting for some other directory to work in. Every switch is
+ `git checkout --detach`, so branch refs stay exactly where they were.
+2. **Read-only review.** Do not build, do not run tests, do not modify any
source file in the
+ repository. The only writes allowed are the two documents under
`review-docs/` and the context
+ directory `$CTX`. **Post nothing to GitHub.**
+3. **The diff has exactly one source**: `$CTX/pr.diff` and
`$CTX/pr_changed_files.txt`. Do not
+ reach for `gh pr diff`, the web UI, or a hand-rolled `git diff` to get the
change list - a
+ different way of fetching it means a different base.
+4. **Confirm a path before reading it.** If a path is not already confirmed by
+ `pr_changed_files.txt`, `pr.diff`, or the output of an earlier successful
command, run
+ `rg --files` to confirm it first.
+5. **Do not stop at the first blocking issue.** Work through the changed
files, the related
+ control flow, the tests, and the parallel or special-case paths.
+6. **Every suspicion must reach a conclusion**: it becomes a finding, is
excluded as "already
+ covered by an existing comment", or is ruled out with concrete code
evidence and written into
+ "Considered and Dismissed". Silently dropping a suspicion means the review
is not finished.
+7. **There are no inline comments locally**, so an anchor is the reader's only
way in. Every
+ finding must carry a `path:line` anchor (new-side line numbers) plus a
verbatim snippet, and
+ the documents must pass `verify-anchors.py` at the end.
+
+---
+
+## 1. Align the current directory
+
+```bash
+CTX="<session scratchpad>/review-ctx"
+$S/align-to-pr.sh <PR> --out "$CTX/align.env"
+```
+
+The script does four things, **all of them against the current worktree only**:
+
+1. **Resolve the PR**: a URL, `owner/repo#N`, or a bare number are all
accepted; `gh api` supplies
+ title, state, head ref/repo/sha, and base ref/sha.
+2. **Diagnose consistency** (the two checks the user explicitly wants):
+ - `branch check`: is the current branch the PR's head branch?
+ - `commit check`: `same` / `ahead:N` / `behind:N` / `diverged:a/b` /
`unrelated`
+ **Relay both lines to the user verbatim.** In particular `ahead:N` means
there are local commits
+ that were never pushed, and **they are not part of the review**.
+3. **Fetch the PR head**: `git fetch <upstream>
+refs/pull/N/head:refs/doris-review/pr-N`. It goes
+ through `refs/pull/*`, so a fork PR needs no extra remote. The fetched sha
must equal the head
+ sha from the API; if it does not, the PR was pushed to while the context
was being prepared -
+ the script re-reads the API once and errors out asking for a re-run if it
still differs (this
+ mirrors CI's "PR changed while review context was being prepared" guard).
+4. **Align the current directory**:
+
+ | State of the current directory | Action |
+ |---|---|
+ | HEAD already equals the PR head | `ALIGN_MODE=already`, nothing to do |
+ | Clean | `git checkout --detach <PR head>`, `ALIGN_MODE=switched` |
+ | Modified tracked files / a rebase, merge, or cherry-pick in progress |
`ALIGN_MODE=blocked`, **refuse and exit** |
+
+ "Clean" means **no modified tracked files**. Untracked files do not count
(`git checkout` never
+ deletes them, and refuses outright when it would overwrite one), so an
untracked directory such
+ as `review-docs/` does not affect the verdict and survives the switch.
+
+**What to do when BLOCKED**: relay the dirty-file list the script printed,
then ask the user to
+pick - (a) commit it themselves, (b) stash it themselves, or (c) start over in
a different
+directory. **Do not perform any of those for them**; once they have chosen and
acted, re-run this
+skill. To see the diagnosis without changing anything, use `--check` (it still
prints the full
+branch check / commit check before reporting BLOCKED).
+
+Once aligned, `$CTX/align.env` supplies `WORKDIR` (= the current directory),
`DOCS_ROOT`
+(= `WORKDIR`), `PR_HEAD_SHA` / `PR_BASE_SHA`, `PREV_REF` (the restore point),
+`BRANCH_MATCHES_PR`, and `LOCAL_VS_PR`.
+
+---
+
+## 2. Prepare the context
+
+```bash
+$S/prepare-review-context.sh \
+ --ctx "$CTX" --align "$CTX/align.env" [--focus "<user focus>"] [--fresh]
+```
+
+`--align` carries over `WORKDIR`, the PR number, the upstream repo, and the PR
base sha, and
+verifies the checkout really is parked on the PR head (if it is not, the
script errors out and
+asks you to redo step 1). `--fresh` wipes and rebuilds everything including
the ledger; without it
+the ledger is preserved so a multi-round run can continue. Whatever free text
the user typed after
+the PR URL on the command line is the `--focus` value.
+
+Output (under `$CTX`):
+
+| File | Contents |
+|---|---|
+| `meta.env` | PR info, base/head sha, diff range, `DOCS_ROOT`, `ALIGN_MODE`,
date, dirty-file count |
+| `pr.diff` / `pr_changed_files.txt` / `pr_changed_files_status.txt` | The
authoritative diff and change list |
+| `changed_line_ranges.txt` / `.tsv` | **New-side (post-change) line ranges**
- the source for anchors |
+| `pr_commits.txt` / `pr_diffstat.txt` | Commit list and change size |
+| `pr_review_threads.md` | Existing inline comments (at most 30 threads, each
body truncated at 1200 chars) |
+| `required_agents.txt` / `required_agents_prompt.txt` | Required AGENTS.md
files, derived from the changed files' ancestor directories |
+| `review_focus.txt` | The user's focus points |
+| `worktree_status.txt` | Uncommitted changes in the review tree (**out of
review scope**; call them out in the documents) |
+| `ledger/` | Shared-ledger skeleton |
+
+If `BASE_SOURCE` is not `PR base sha (matches CI)`, the baseline differs from
CI's and the
+documents must say so.
+
+---
+
+## 3. Required reading (in this order)
+
+Before looking at any code, the main agent reads, in order:
+
+1. `$WORKDIR/.claude/skills/code-review/SKILL.md` - the Doris review
checklist; follow it strictly
+ throughout. On an older branch that does not have it, fetch it with
+ `gh api
repos/apache/doris/contents/.claude/skills/code-review/SKILL.md?ref=master`.
+2. **Every** AGENTS.md listed in `$CTX/required_agents.txt`, each one read in
full. Listing the
+ directory or grepping the path does not count.
+3. `$CTX/pr_review_threads.md` - treat every existing comment as already-known
context. **Do not
+ raise** the same or a substantially similar issue again, even phrased
differently. Raise a
+ similar concern only when this PR introduces a genuinely different instance
somewhere else that
+ the existing comments do not cover, and say why it is distinct.
+4. `$CTX/review_focus.txt` - do the full review as usual and pay extra
attention to these points;
+ the final documents must respond to each one, even if the conclusion is "no
additional issue
+ found for this point".
+5. All of `$CTX/pr.diff` plus `$CTX/pr_commits.txt`. When the diff is large (>
5000 lines), read it
+ in chunks by directory or module until it is covered, and describe how you
read it in "Coverage
+ and Limits" - do not pretend you read it all.
+
+---
+
+## 4. Main-agent initial risk scan (before spawning any subagent)
+
+**This must be finished before any subagent is spawned.** Read the whole
change, understand every
+mechanism it touches, then answer: if this PR is wrong, where is it most
likely to be wrong? Which
+points look risky to you?
+
+Write the result into `$CTX/ledger/00-main-risk-scan.md`, each entry carrying:
an ID, the changed
+files/lines involved, the related mechanism to inspect, why it is suspicious,
the upstream or
+downstream files that must be read alongside it, and the **specific question**
a risk-focused
+subagent has to answer.
+
+An empty risk scan means you have not understood the PR yet - go back and read
it; do not skip
+this step.
+
+---
+
+## 5. Spawn the subagents
+
+Split along the coverage the code-review skill requires, and **send them all
in one message so
+they run concurrently** (general-purpose subagents):
+
+| Type | Count | Responsibility |
+|---|---|---|
+| Full-review subagent | 1-3 (depending on PR complexity) | Each owns one
slice; **their union must cover the whole PR** and every checkpoint the skill
requires |
+| Risk-focused subagent | one per suspicious mechanism | Answers exactly one
question from the risk scan, independently of the full-review subagents |
+
+Split by **module or mechanism** (for example "FE SPI interface and load
path", "Ranger plugin
+implementation and class loading", "build scripts and packaging/deployment"),
not by dividing the
+file count evenly - a split that lands in the middle of a mechanism leaves
both halves unable to
+see the bug. Give every subagent its own ledger file,
`$CTX/ledger/sub-<round>-<id>.md`.
+
+Take the prompts from `references/prompts.md`: section A (shared preamble)
plus section B
+(full review) or section C (risk-focused), substituting `{CTX}` /
`{REPO_ROOT}` (= `$WORKDIR`) /
+`{BASE_SHA}` / `{HEAD_SHA}` / `{ROUND}` / `{AGENT_ID}` / `{FOCUS}`. **Every
subagent prompt must
+state the ledger directory and the path of that subagent's own file.**
+
+> **One implementation difference from CI**: CI uses a single
`subagent_review_findings.md` with
+> sections; locally, concurrent writes to one file collide on patch, so this
becomes **one file per
+> owner** under `ledger/`. The semantics are unchanged - a single shared
source of truth, everyone
+> reads all of it, each writes only their own, the main agent merges.
+
+---
+
+## 6. Main agent merges, verifies, deduplicates
+
+**Re-read the entire `ledger/` directory every time a subagent returns**, then
process each
+candidate into `$CTX/ledger/main-merged.md`:
+
+- **Verify**: read the code yourself to confirm it; do not take a subagent's
conclusion at face
+ value. The common failure modes are treating a local pattern that an
upstream guarantee already
+ covers as a bug, and an "if A then B" whose A has no concrete scenario.
+- **Deduplicate**: against the existing candidates and against the existing
comments in
+ `pr_review_threads.md`.
+- **Assign a status**: `accepted` / `dismissed_with_evidence` / `duplicated`.
A dismissal must come
+ with concrete code evidence, which goes verbatim into the document's
"Considered and Dismissed".
+- **Fill the risk items back in**: update `Status` and `Final conclusion` for
every entry in
+ `00-main-risk-scan.md`.
+
+When this step ends, **no candidate may be left without a status**.
+
+---
+
+## 7. Convergence loop (at most 3 rounds)
+
+One round = step 5 + step 6. Record the outcome in the `Convergence Rounds`
table of
+`main-merged.md`.
+
+- Every subagent returned `NO_NEW_VALUABLE_FINDINGS` → go to step 8.
+- New valuable candidates remain → start another round: **re-slice the
coverage** based on what
+ this round taught you (do not re-dispatch the same split unchanged), and add
risk-focused
+ subagents for any newly suspicious mechanism.
+- **Cap of 3 rounds.** If the cap is reached and new candidates still appear,
finish normally but
+ state in the verdict and in "Coverage and Limits" that **this review did not
converge**.
+
+---
+
+## 8. Final sweep
+
+Before writing the documents, walk explicitly through the changed-file list
and the open-candidate
+list:
+
+- Was every changed file covered by at least one subagent? Cover the rest
yourself.
+- Does every suspicion have a conclusion?
+- Does every applicable item in Part 1.3 of the code-review skill have an
explicit conclusion?
+- Is there anywhere you are still unsure about, or that may not have been
investigated deeply
+ enough? Investigate it now.
+
+Only after the sweep may you write the documents.
+
+---
+
+## 9. Produce the two documents
+
+Write them into `review-docs/` in the **current directory** (`mkdir -p` it if
needed):
+
+```
+review-docs/pr-<N>-review.en.md
+review-docs/pr-<N>-review.zh.md
+```
+
+`references/doc-templates.md` holds the templates, the anchor format, and the
verdict rule
+(`Blocker`/`Major` → REQUEST_CHANGES; only `Minor`/`Nit` → APPROVE). The
essentials:
+
+- The two documents are **equivalent in content**: same finding IDs, same
order, same anchors. The
+ Chinese one is a real Chinese review, not a word-for-word translation;
identifiers, paths, log
+ messages, config names, code snippets, and the severity words stay in their
original form.
+- The document header must state: the PR link and state, the **head sha
actually reviewed**, the
+ diff range, the review directory (`WORKDIR`), and the `branch check` /
`commit check` results
+ from step 1.
+- Every finding must carry: a severity, a `path:line` anchor, a verbatim
snippet, and the four
+ parts **what is wrong / why it happens / when it bites / suggested fix**.
Give a diff patch when
+ the fix is small and self-contained; use prose only for architectural
problems.
+- The "Critical Checkpoints" table needs a conclusion per row; mark a
non-applicable one `n/a` with
+ a half-line reason - **never delete the row**.
+- "Response to Review Focus" answers each of the user's focus points.
+- "Considered and Dismissed" lists every excluded suspicion together with its
evidence.
+- "Coverage and Limits" states: what was read in depth versus skimmed, how the
subagents were
+ split, what was not verified locally (builds, tests, anything needing a real
cluster), and the
+ uncommitted changes from `worktree_status.txt` that were excluded.
+
+When they are written, running the verifier is **mandatory**; if it fails, fix
the documents and
+re-run until it passes:
+
+```bash
+python3 $S/verify-anchors.py --ctx "$CTX" \
+ --doc review-docs/pr-<N>-review.en.md \
+ --doc review-docs/pr-<N>-review.zh.md
+```
+
+It checks that anchor paths exist, that line numbers are inside the file, that
every finding has at
+least one anchor, and that the EN and ZH finding-ID sets match; it also flags
anchors pointing at
+unchanged context lines - which is usually where a miscomputed line number
shows up.
+
+---
+
+## 10. Closing report
+
+Tell the user:
+
+1. The paths of both documents, the verdict (REQUEST_CHANGES / APPROVE), the
finding count per
+ severity, and whether the rounds converged.
+2. The `branch check` / `commit check` results - especially `ahead:N`
(unpushed commits that were
+ not reviewed).
+3. **Where the current directory now stands**: with `ALIGN_MODE=switched` it
is detached on the PR
+ head, and `git checkout <PREV_REF>` restores it. **Do not switch back
automatically** - the user
+ may still want to read the code.
+4. `review-docs/` **is not gitignored in the doris repository**, so **do not
commit it
+ automatically**; leave that to the user.
+
+---
+
+## 11. Mapping to the CI pipeline
+
+| CI (code-review-runner.yml) | Local |
+|---|---|
+| `codex exec --goal` (gpt-5.6-sol, xhigh) | The main agent of this session |
+| checkout the PR head sha | `align-to-pr.sh` detaches **the current
directory** to the same sha |
+| `git diff BASE...HEAD` as the authoritative diff | Same, produced by
`prepare-review-context.sh` |
+| "PR changed while preparing" guard | The fetched sha must equal the API head
sha, otherwise a re-run is required |
+| `prepare_review_agents.py` collecting AGENTS.md | Same ancestor-directory
algorithm, built into the script |
+| Fetch existing inline threads (30 threads / 1200 chars) | Same jq |
+| Text after `/review` = review focus | Free text after the PR URL → `--focus`
|
+| Single-file ledger with sections | A `ledger/` directory, one file per owner
|
+| Main risk scan → 1-3 full-review subagents + risk-focused → merge → ≤3
rounds | Identical |
+| `gh pr review` / Reviews API posting inline comments | **Two `review-docs/`
documents (EN + ZH) with `path:line` anchors** |
+| 60-minute timeout | No hard timeout, but likewise do not let one round turn
into unbounded digging |
+
+---
+
+## 12. Common traps
+
+- **The local checkout is ahead of the PR head.** `commit check: ahead:N`
means there are unpushed
+ commits and **they are not part of the review**. This is the easiest thing
for a reader to
+ misread, so state it both in the document header and in the closing report.
+- **Anchoring to the wrong side.** `changed_line_ranges.txt` gives
**post-change** line numbers,
+ which match what a file read shows in the current directory. A pure deletion
has no line on the
+ new side - anchor the seam line and note `(deleted, base line N)`.
+- **Switching invalidates the current directory's incremental build.**
`output/`, `be/build_*`, and
+ `*/target/` are all built per commit, so switching to the PR head and back
usually forces a
+ rebuild. Warn the user before switching.
+- **Uncommitted changes are not in the diff.** `git diff BASE...HEAD` only
sees committed content.
+ Alignment already guarantees no dirty tracked files at switch time, but the
user may have edited
+ something afterwards; anything in `worktree_status.txt` must be named in the
documents' "Coverage
+ and Limits".
+- **Subagents racing on the ledger.** One file per owner exists precisely for
this; always hard-code
+ each subagent's own file name in its prompt.
+- **Re-raising an issue that already has a comment.** CI forbids it, and so
does a local run - read
+ `pr_review_threads.md` first.
+- **A subagent's conclusion cannot be taken at face value.** The main agent
must read the code back
+ to confirm it; any "if A then B" that cannot name a concrete scenario for A
is downgraded or
+ dropped.
+- **Do not commit `review-docs/`.** The doris repository does not ignore it,
and an automatic commit
+ would slip it into the PR.
diff --git a/skills/doris-repo-review/references/doc-templates.md
b/skills/doris-repo-review/references/doc-templates.md
new file mode 100644
index 0000000..c1a1c1d
--- /dev/null
+++ b/skills/doris-repo-review/references/doc-templates.md
@@ -0,0 +1,227 @@
+# Review document templates
+
+Two documents, same content, same finding IDs, same order:
+
+- `review-docs/pr-<N>-review.en.md`
+- `review-docs/pr-<N>-review.zh.md`
+
+When the branch has no PR, use `review-docs/<branch-slug>-review.en.md` /
`.zh.md`.
+
+The ZH document is a real Chinese review, not a machine translation of the EN
one: same facts,
+same anchors, same IDs, but idiomatic Chinese. Identifiers, file paths, log
messages, config
+names, code snippets, and the severity words (`Blocker` / `Major` / `Minor` /
`Nit`) stay in
+their original form in both documents.
+
+## Anchor format (mandatory, both documents)
+
+Local review has no inline comments, so an anchor is the only way a reader
finds the code.
+
+- Write anchors inside backticks: ``
`fe/fe-core/src/main/java/org/apache/doris/X.java:412` ``
+ or `` `path:412-430` ``. `scripts/verify-anchors.py` only recognises this
form.
+- Numbers are NEW-SIDE (post-change) line numbers of the worktree at HEAD,
cross-checked
+ against `changed_line_ranges.txt` and confirmed by reading the file.
+- Every finding carries at least one anchor plus a fenced snippet copied
verbatim from it.
+- Pure deletion: anchor the surviving seam line and add `(deleted, base line
N)`.
+- Missing thing (no test / no metric / no compat path): anchor the most
relevant existing line
+ and state what should be there.
+- Anchors inside fenced code blocks are ignored by the verifier — never hide a
real anchor there.
+
+## Verdict rule
+
+| Highest severity present | Verdict |
+|---|---|
+| `Blocker` or `Major` | `REQUEST_CHANGES` |
+| only `Minor` / `Nit` / none | `APPROVE` |
+
+Severity meanings: `Blocker` = correctness, data loss, deadlock, crash,
incompatible change
+without a compat path, or a broken build/test contract. `Major` = real defect
or a missing
+guarantee that will bite in production or during upgrade. `Minor` = worth
fixing, not urgent.
+`Nit` = style or wording.
+
+---
+
+## English template
+
+```markdown
+# Code Review — PR #<N>: <title>
+
+| | |
+|---|---|
+| PR | <url> (state: <open/merged/closed>) |
+| PR head | `<PR_HEAD_SHA>` on `<PR_HEAD_REF>` of `<PR_HEAD_REPO>` |
+| Diff range | `<BASE_SHA>...<HEAD_SHA>` (three-dot, merge base
`<MERGE_BASE>`) |
+| Scope | <n> files, +<a>/-<b> lines, <c> commits |
+| Reviewed in | `<WORKDIR>` (<ALIGN_MODE>), on <YYYY-MM-DD> |
+| Local alignment | branch check: <…>; commit check: <same / ahead:N /
behind:N / diverged:a/b> |
+| Verdict | **REQUEST_CHANGES** / **APPROVE** |
+| Rounds | <r> of max 3, converged / not converged |
+
+<!-- Only when commit check was ahead:N before aligning: -->
+> `<N>` local commit(s) on `<branch>` are ahead of the PR head and are **not**
covered by
+> this review.
+
+## Summary
+
+<3-8 sentences: what the PR does, whether it achieves it, and the headline
concerns.>
+
+## Findings
+
+<Ordered by severity, then by file. Omit the section entirely if there are
none.>
+
+### F-01 · <one-line title>
+
+- **Severity**: Blocker
+- **Where**: `path/to/File.java:412-430`
+- **Category**: correctness / concurrency / lifecycle / compatibility / config
/ performance / observability / test coverage
+
+```java
+<verbatim snippet from the anchor>
+```
+
+**What is wrong.** <the defect, stated once>
+
+**Why it happens.** <call chain with `path:line` citations, the invariant that
breaks>
+
+**When it bites.** <a concrete trigger scenario — who calls what, in which
order, with which data>
+
+**Suggested fix.**
+
+```diff
+<a precise patch when the fix is small and self-contained; prose only when it
is architectural>
+```
+
+## Critical Checkpoints
+
+<Part 1.3 of the repo code-review skill requires an individual conclusion per
applicable item.
+One row per checkpoint. Mark non-applicable items `n/a` with a half-line
reason — do not drop them.>
+
+| Checkpoint | Conclusion |
+|---|---|
+| Goal achieved, proven by a test | <conclusion, with `path:line` where
relevant> |
+| Change is minimal, clear, focused | |
+| Concurrency: threads, shared state, locks, lock order | |
+| Lifecycle / static init order | |
+| New config: dynamic? observed without restart? | |
+| Incompatible change: rolling-upgrade path | |
+| Parallel code paths updated | |
+| Special conditionals explained | |
+| Test coverage (e2e, negative, unit) | |
+| Test results added/modified and correct | |
+| Observability (logs, VLOG, metrics) | |
+| Transaction / persistence / EditLog replay | |
+| Data write atomicity, crash safety | |
+| New FE↔BE variables on every send path | |
+| Performance and anti-patterns | |
+| Anything else | |
+
+## Response to Review Focus
+
+<Answer each user focus point from review_focus.txt, including "no additional
issue found for
+this point". Write "No additional user-provided review focus." when there was
none.>
+
+## Considered and Dismissed
+
+<Every suspicious point that did NOT become a finding, with the code evidence
that rules it
+out. This is what makes the review auditable — do not silently drop a concern.>
+
+| # | Concern | Where | Why it is not an issue |
+|---|---|---|---|
+| D-01 | | `path:line` | |
+
+## Coverage and Limits
+
+- Files reviewed in depth: <...>
+- Files skimmed and why: <...>
+- Subagents: <round/id/coverage table>
+- Not verified locally: <builds, tests, and anything that needs a running
cluster>
+- Uncommitted worktree paths excluded from this review: <from
worktree_status.txt, or "none">
+```
+
+---
+
+## Chinese template
+
+```markdown
+# 代码评审 — PR #<N>：<标题>
+
+| | |
+|---|---|
+| PR | <url>（状态：<open/merged/closed>） |
+| PR head | `<PR_HEAD_SHA>`，来自 `<PR_HEAD_REPO>` 的 `<PR_HEAD_REF>` |
+| Diff 范围 | `<BASE_SHA>...<HEAD_SHA>`（三点，merge base `<MERGE_BASE>`） |
+| 规模 | <n> 个文件，+<a>/-<b> 行，<c> 个 commit |
+| 评审位置 | `<WORKDIR>`（<ALIGN_MODE>），<YYYY-MM-DD> |
+| 本地一致性 | 分支：<…>；commit：<一致 / 领先 N / 落后 N / 已分叉 a/b> |
+| 结论 | **REQUEST_CHANGES** / **APPROVE** |
+| 轮次 | 共 <r> 轮（上限 3），已收敛 / 未收敛 |
+
+<!-- 仅当对齐前 commit check 是领先 N 时保留： -->
+> `<branch>` 上有 `<N>` 个本地提交领先于 PR head，**不在**本次评审范围内。
+
+## 总体结论
+
+<3-8 句：这个 PR 做了什么、有没有做到、最主要的问题是什么。>
+
+## 问题清单
+
+### F-01 · <一句话标题>
+
+- **等级**：Blocker
+- **位置**：`path/to/File.java:412-430`
+- **类别**：正确性 / 并发 / 生命周期 / 兼容性 / 配置 / 性能 / 可观测性 / 测试覆盖
+
+```java
+<锚点处原样摘录的代码>
+```
+
+**问题是什么。** <一句话把缺陷说清楚>
+
+**为什么会这样。** <调用链，带 `path:line` 引用，说明被破坏的不变式>
+
+**什么时候会踩到。** <具体触发场景：谁在什么顺序上调用了什么、数据长什么样>
+
+**修改建议。**
+
+```diff
+<小而自洽的改动给出精确 patch；架构性问题只用文字说明>
+```
+
+## 关键检查项
+
+| 检查项 | 结论 |
+|---|---|
+| 目标是否达成、是否有测试证明 | <结论，必要时带 `path:line`> |
+| 改动是否足够小、清晰、聚焦 | |
+| 并发：线程、共享变量、锁与锁序 | |
+| 生命周期 / 静态初始化顺序 | |
+| 新增配置：是否该动态、是否真能动态生效 | |
+| 不兼容改动：滚动升级路径 | |
+| 平行代码路径是否同步修改 | |
+| 特殊条件判断是否有注释说明 | |
+| 测试覆盖（e2e、负例、单测） | |
+| 测试结果是否补充/更新且正确 | |
+| 可观测性（日志、VLOG、Metrics） | |
+| 事务 / 持久化 / EditLog 回放 | |
+| 数据写入原子性与崩溃安全 | |
+| 新增 FE↔BE 变量的所有传递路径 | |
+| 性能与反模式 | |
+| 其它 | |
+
+## 对关注点的回应
+
+<逐条回应 review_focus.txt 中用户提出的关注点，没发现问题也要明确说明。用户没提的话写“用户未指定额外关注点”。>
+
+## 已排查并排除的疑点
+
+| # | 疑点 | 位置 | 为什么不是问题 |
+|---|---|---|---|
+| D-01 | | `path:line` | |
+
+## 覆盖范围与局限
+
+- 深入看过的文件：<...>
+- 只扫了一眼的文件及原因：<...>
+- 子 agent 分工：<轮次 / id / 覆盖面 表格>
+- 本地未验证：<编译、用例，以及任何需要真实集群才能确认的部分>
+- 未纳入本次评审的未提交改动：<取自 worktree_status.txt，没有就写“无”>
+```
diff --git a/skills/doris-repo-review/references/prompts.md
b/skills/doris-repo-review/references/prompts.md
new file mode 100644
index 0000000..4c15f6a
--- /dev/null
+++ b/skills/doris-repo-review/references/prompts.md
@@ -0,0 +1,142 @@
+# Subagent prompt templates
+
+Ported from `apache/doris/.github/workflows/code-review-runner.yml` (the
`review_prompt.txt`
+heredoc) so that a local run gives the subagents the same instructions CI
does. Keep them in
+English — the CI wording is authoritative; only the transport (local `Agent`
tool instead of
+Codex goal mode) and the output (review documents instead of GitHub inline
comments) differ.
+
+Substitute before use:
+
+| Placeholder | Value |
+|---|---|
+| `{CTX}` | absolute review context directory |
+| `{REPO_ROOT}` | absolute repo root |
+| `{BASE_SHA}` / `{HEAD_SHA}` | from `{CTX}/meta.env` |
+| `{ROUND}` | 1-based round number |
+| `{AGENT_ID}` | short slug, e.g. `r1-fe-spi` |
+| `{FOCUS}` | this subagent's assigned coverage |
+
+---
+
+## A. Shared preamble (goes into EVERY subagent prompt)
+
+```
+You are one review subagent in an automated code review of a local Apache
Doris worktree.
+The repository root is {REPO_ROOT}; work there. This environment is for review
only: do not
+build, do not run tests, and do not modify any repository file. Temporary
scripts you write
+for review purposes under {CTX} are the only exception. You must not post
anything to GitHub.
+
+Authoritative PR context (do not obtain the diff or the changed-path list any
other way):
+- Aggregate PR diff : {CTX}/pr.diff
+- Changed files : {CTX}/pr_changed_files.txt
+- Changed files with status : {CTX}/pr_changed_files_status.txt
+- New-side changed line ranges : {CTX}/changed_line_ranges.txt
+- Commits in this PR : {CTX}/pr_commits.txt
+- Existing inline review threads : {CTX}/pr_review_threads.md
+- User review focus : {CTX}/review_focus.txt
+- Required AGENTS.md files : {CTX}/required_agents.txt
+- Shared review ledger directory : {CTX}/ledger/
+- Diff range : {BASE_SHA}...{HEAD_SHA} (three-dot, from
the merge base)
+
+These were generated with `git diff {BASE_SHA}...{HEAD_SHA}` in this worktree.
The base SHA
+identifies the target-branch snapshot and is not necessarily the diff's left
endpoint.
+
+Before reading any file whose exact path is not already confirmed by
pr_changed_files.txt,
+pr.diff, or a previous successful command output, you MUST first run `rg
--files` to confirm
+the actual path of the target file.
+
+Before reviewing any code you MUST read, in this order:
+1. {REPO_ROOT}/.claude/skills/code-review/SKILL.md — the Doris review
checklist; follow it strictly.
+2. Every file listed in {CTX}/required_agents.txt — read each one directly;
listing or
+ grepping the paths is not sufficient.
+3. Every file in {CTX}/ledger/ — so you do not duplicate
an existing candidate.
+4. {CTX}/pr_review_threads.md — treat every existing
thread and reply as
+ already-known review context. Do NOT propose the same or a substantially
similar issue again,
+ even phrased differently. Raise a similar concern only when this PR
introduces a genuinely
+ different instance in another location, and say why it is distinct.
+
+Ledger rules:
+- Write ONLY to {CTX}/ledger/sub-{ROUND}-{AGENT_ID}.md. Never touch another
subagent's file,
+ 00-main-risk-scan.md, main-merged.md, or any repository source file.
+- Use the candidate format documented in {CTX}/ledger/README.md. Candidate IDs
must be
+ globally unique: prefix them with your agent id, e.g. `{AGENT_ID}-01`.
+- If a candidate overlaps one that already exists in the ledger, record it in
your own file
+ with a duplicate note naming the existing candidate ID instead of restating
it.
+
+Line-number rule (this run has no GitHub inline comments, so anchors are the
only pointer):
+- Every candidate MUST carry `Path:` plus `Line:` using NEW-SIDE (post-change)
line numbers,
+ cross-checked against {CTX}/changed_line_ranges.txt, and verified by
actually reading the
+ file at that line. A range is written `start-end`.
+- Include a verbatim 1-8 line snippet copied from that location so the main
agent can confirm it.
+- For a pure deletion, cite the surviving new-side seam line and note
"(deleted, base line N)".
+- For a missing-thing finding (no test, no metric, no compatibility handling),
anchor to the
+ most relevant existing line and say what should be there.
+
+Evidence rule: for any claimed error you must give the concrete path or logic
where it occurs.
+"If A then B" is only acceptable when you name a concrete scenario in which A
actually happens.
+
+Do NOT stop after finding the first blocking issue. Keep reviewing changed
files, related
+control flow, tests, and parallel or special-case paths until all plausible
correctness,
+lifecycle, configuration, compatibility, performance, and coverage bugs have
been investigated
+and every bug you can substantiate has been reported.
+
+Before returning, think once more: what problem might I have missed just now,
and where? Then
+do a thorough recheck to confirm whether that potential issue is real and
needs reporting.
+
+Return value: your final text is consumed by the main agent, not by a human.
Return exactly:
+- the path of your ledger file,
+- the list of candidate IDs you added with a one-line summary and severity for
each,
+- the literal token NO_NEW_VALUABLE_FINDINGS when you added no new candidate
this round.
+```
+
+---
+
+## B. Full-review subagent (1-3 per round)
+
+Append to the shared preamble:
+
+```
+Your assigned coverage for round {ROUND}: {FOCUS}
+
+Review that coverage completely. The union of all subagents' coverage must
span the whole PR,
+so do not skip a file that falls inside your slice because it looks boring,
and do not wander
+into another subagent's slice except to establish upstream/downstream evidence
for your own.
+
+Work through the `Critical Checkpoints` of the code-review skill (Part 1.3)
against your slice
+and reach an explicit conclusion for each applicable item. Use Parts 2-7 of
the skill and the
+module AGENTS.md files as supporting material for finding bugs, stale
assumptions, and missing
+coverage. For optimizer/Nereids changes, follow the plan-tree output style in
Part 1.2.1.
+
+Read the actual surrounding code, not only the diff hunks: a change is only
correct with
+respect to its real call chain, its concurrency, and its lifecycle.
+```
+
+---
+
+## C. Risk-focused subagent (0..N per round, driven by the main risk scan)
+
+Append to the shared preamble:
+
+```
+You are a RISK-FOCUSED subagent, separate from the full-review subagents. You
are not
+reviewing a slice of the PR; you are answering one specific question about one
specific
+mechanism the main agent flagged as suspicious.
+
+Risk item ID : {RISK_ID}
+Mechanism : {MECHANISM}
+Why suspicious : {WHY}
+Files/lines : {FILES}
+Upstream/downstream to inspect : {NEIGHBOURS}
+Question you MUST answer: {QUESTION}
+
+Investigate the mechanism end to end, including its upstream callers and
downstream consumers,
+until you can answer the question with concrete code evidence. Then record in
your ledger file:
+- the answer (confirmed bug / not a bug / cannot determine, with what is
missing),
+- the exact call chain or state sequence you traced, with `path:line`
citations,
+- a concrete trigger scenario if you claim a bug,
+- the strongest counter-argument you considered and why it does or does not
hold.
+
+Report a candidate finding only when the evidence is concrete. "Not a bug" is
a valid and
+useful answer, but it must come with the code evidence that rules the concern
out — the main
+agent records that evidence as a dismissal.
+```
diff --git a/skills/doris-repo-review/scripts/align-to-pr.sh
b/skills/doris-repo-review/scripts/align-to-pr.sh
new file mode 100755
index 0000000..8f800d5
--- /dev/null
+++ b/skills/doris-repo-review/scripts/align-to-pr.sh
@@ -0,0 +1,243 @@
+#!/usr/bin/env bash
+# Align the CURRENT worktree to the exact head commit of a GitHub PR.
+#
+# Usage:
+# align-to-pr.sh <PR-URL | owner/repo#N | N> [--repo <owner/repo>] [--check]
[--out <file>]
+#
+# --repo <owner/repo> upstream repo when the argument is a bare number
(default apache/doris)
+# --check report the diagnosis and change nothing
+# --out <file> write align.env here (default: stdout only)
+#
+# Scope: the worktree the command runs in. No other directory is looked at, no
+# worktree is created, no environment variable selects a different checkout.
+#
+# Safety: the switch is refused outright when the worktree has modified tracked
+# files or an operation in progress - the user commits or stashes, never this
+# script. The switch itself is `git checkout --detach`, so no branch ref moves
+# and nothing is reset or deleted. Untracked files are left alone: git never
+# deletes them on checkout and refuses when it would overwrite one.
+set -euo pipefail
+
+PR_ARG=""
+UPSTREAM_REPO=""
+OUT_FILE=""
+CHECK_ONLY=0
+
+while [ $# -gt 0 ]; do
+ case "$1" in
+ --repo) UPSTREAM_REPO="$2"; shift 2 ;;
+ --check) CHECK_ONLY=1; shift ;;
+ --out) OUT_FILE="$2"; shift 2 ;;
+ -h|--help) sed -n '2,20p' "$0"; exit 0 ;;
+ -*) echo "unknown option: $1" >&2; exit 2 ;;
+ *) PR_ARG="$1"; shift ;;
+ esac
+done
+
+[ -n "$PR_ARG" ] || { echo "ERROR: no PR given. Pass a PR URL, owner/repo#N,
or a number." >&2; exit 2; }
+command -v gh >/dev/null 2>&1 || { echo "ERROR: gh CLI is required." >&2; exit
1; }
+
+# ---------------------------------------------------------------- parse the
PR ref
+PR_NUMBER=""
+if [[ "$PR_ARG" =~
^(https?://)?(www\.)?github\.com/([^/]+)/([^/]+)/pull/([0-9]+) ]]; then
+ UPSTREAM_REPO="${BASH_REMATCH[3]}/${BASH_REMATCH[4]}"
+ PR_NUMBER="${BASH_REMATCH[5]}"
+elif [[ "$PR_ARG" =~ ^([^/]+)/([^#]+)#([0-9]+)$ ]]; then
+ UPSTREAM_REPO="${BASH_REMATCH[1]}/${BASH_REMATCH[2]}"
+ PR_NUMBER="${BASH_REMATCH[3]}"
+elif [[ "$PR_ARG" =~ ^#?([0-9]+)$ ]]; then
+ PR_NUMBER="${BASH_REMATCH[1]}"
+else
+ echo "ERROR: cannot parse '$PR_ARG' as a PR reference." >&2
+ exit 2
+fi
+[ -n "$UPSTREAM_REPO" ] || UPSTREAM_REPO="apache/doris"
+
+WORKDIR="$(git rev-parse --show-toplevel)"
+
+# ------------------------------------------------------------------ PR
metadata
+read_pr() {
+ gh api "repos/${UPSTREAM_REPO}/pulls/${PR_NUMBER}" \
+ --jq '[.title, .state, .head.ref, (.head.repo.full_name // "DELETED"),
.head.sha, .base.ref, .base.sha] | @tsv'
+}
+if ! PR_TSV="$(read_pr 2>/dev/null)"; then
+ echo "ERROR: cannot read ${UPSTREAM_REPO}#${PR_NUMBER} (wrong repo/number,
or gh is not authenticated)." >&2
+ exit 1
+fi
+IFS=$'\t' read -r PR_TITLE PR_STATE PR_HEAD_REF PR_HEAD_REPO PR_HEAD_SHA
PR_BASE_REF PR_BASE_SHA <<<"$PR_TSV"
+
+# ------------------------------------------------------------------ local
state
+HEAD_SHA="$(git rev-parse HEAD)"
+CURRENT_REF="$(git symbolic-ref --quiet --short HEAD 2>/dev/null || git
rev-parse --short=12 HEAD)"
+DIRTY="$(git status --porcelain --untracked-files=no | grep -c . || true)"
+UNTRACKED="$(git ls-files --others --exclude-standard | grep -c . || true)"
+
+IN_PROGRESS=""
+for d in rebase-merge rebase-apply MERGE_HEAD CHERRY_PICK_HEAD REVERT_HEAD
BISECT_LOG; do
+ if [ -e "$(git rev-parse --path-format=absolute --git-path "$d")" ]; then
IN_PROGRESS="$d"; break; fi
+done
+
+# ------------------------------------------------------------ fetch the PR
head
+# refs/pull/N/head works for fork PRs too, so no fork remote has to be added.
+lower() { printf '%s' "$1" | tr '[:upper:]' '[:lower:]'; }
+find_remote() {
+ local want slug url name
+ want="$(lower "$1")"
+ for name in $(git remote); do
+ url="$(git remote get-url "$name" 2>/dev/null || true)"
+ slug="$(printf '%s' "$url" | sed -E 's#^.*github\.com[:/]##;
s#\.git$##; s#/$##')"
+ if [ "$(lower "$slug")" = "$want" ]; then printf '%s' "$name"; return
0; fi
+ done
+ return 1
+}
+if REMOTE="$(find_remote "$UPSTREAM_REPO")"; then
+ FETCH_FROM="$REMOTE"; FETCH_VIA="remote '$REMOTE'"
+else
+ FETCH_FROM="https://github.com/${UPSTREAM_REPO}.git";
+ FETCH_VIA="URL (no local remote points at ${UPSTREAM_REPO})"
+fi
+
+PR_REF="refs/doris-review/pr-${PR_NUMBER}"
+if [ "$CHECK_ONLY" = "0" ]; then
+ git fetch --no-tags --quiet "$FETCH_FROM"
"+refs/pull/${PR_NUMBER}/head:${PR_REF}"
+
+ FETCHED_SHA="$(git rev-parse "$PR_REF")"
+ if [ "$FETCHED_SHA" != "$PR_HEAD_SHA" ]; then
+ # Mirrors the CI guard: the PR moved between the API read and the
fetch.
+ IFS=$'\t' read -r PR_TITLE PR_STATE PR_HEAD_REF PR_HEAD_REPO
PR_HEAD_SHA PR_BASE_REF PR_BASE_SHA <<<"$(read_pr)"
+ if [ "$FETCHED_SHA" != "$PR_HEAD_SHA" ]; then
+ echo "ERROR: the PR head moved while preparing (api=$PR_HEAD_SHA
fetched=$FETCHED_SHA)." >&2
+ echo " re-run the command." >&2
+ exit 1
+ fi
+ fi
+
+ if ! git cat-file -e "${PR_BASE_SHA}^{commit}" 2>/dev/null; then
+ git fetch --no-tags --quiet "$FETCH_FROM" \
+ "+refs/heads/${PR_BASE_REF}:refs/doris-review/base-${PR_NUMBER}"
|| true
+ fi
+fi
+BASE_PRESENT=1
+git cat-file -e "${PR_BASE_SHA}^{commit}" 2>/dev/null || BASE_PRESENT=0
+
+# ------------------------------------- how this worktree relates to the PR
head
+BRANCH_MATCHES_PR=0
+[ "$CURRENT_REF" = "$PR_HEAD_REF" ] && BRANCH_MATCHES_PR=1
+
+LOCAL_VS_PR="unknown"
+if git cat-file -e "${PR_HEAD_SHA}^{commit}" 2>/dev/null; then
+ if [ "$HEAD_SHA" = "$PR_HEAD_SHA" ]; then
+ LOCAL_VS_PR="same"
+ elif git merge-base --is-ancestor "$PR_HEAD_SHA" "$HEAD_SHA" 2>/dev/null;
then
+ LOCAL_VS_PR="ahead:$(git rev-list --count
"${PR_HEAD_SHA}..${HEAD_SHA}")"
+ elif git merge-base --is-ancestor "$HEAD_SHA" "$PR_HEAD_SHA" 2>/dev/null;
then
+ LOCAL_VS_PR="behind:$(git rev-list --count
"${HEAD_SHA}..${PR_HEAD_SHA}")"
+ elif git merge-base "$HEAD_SHA" "$PR_HEAD_SHA" >/dev/null 2>&1; then
+ LOCAL_VS_PR="diverged:$(git rev-list --left-right --count
"${PR_HEAD_SHA}...${HEAD_SHA}" | tr '\t' '/')"
+ else
+ LOCAL_VS_PR="unrelated"
+ fi
+fi
+
+# ------------------------------------------------------------------ the
decision
+PREV_REF=""
+PREV_SHA=""
+BLOCKED_REASON=""
+if [ "$HEAD_SHA" = "$PR_HEAD_SHA" ]; then
+ ALIGN_MODE="already"
+elif [ -n "$IN_PROGRESS" ]; then
+ ALIGN_MODE="blocked"
+ BLOCKED_REASON="'$IN_PROGRESS' is in progress; finish or abort it, then
re-run"
+elif [ "$DIRTY" -gt 0 ]; then
+ ALIGN_MODE="blocked"
+ BLOCKED_REASON="$DIRTY modified tracked file(s) would be carried onto the
PR commit and pollute the review; commit or stash them yourself, then re-run"
+else
+ ALIGN_MODE="switched"
+ PREV_REF="$CURRENT_REF"
+ PREV_SHA="$HEAD_SHA"
+ if [ "$CHECK_ONLY" = "0" ]; then
+ git checkout --detach --quiet "$PR_REF"
+ ACTUAL_SHA="$(git rev-parse HEAD)"
+ if [ "$ACTUAL_SHA" != "$PR_HEAD_SHA" ]; then
+ echo "ERROR: worktree is at $ACTUAL_SHA, expected the PR head
$PR_HEAD_SHA." >&2
+ exit 1
+ fi
+ fi
+fi
+
+# Refuse for real only when actually asked to act; --check still prints the
diagnosis.
+if [ "$ALIGN_MODE" = "blocked" ] && [ "$CHECK_ONLY" = "0" ]; then
+ echo "ERROR: cannot switch $WORKDIR to the PR head." >&2
+ echo " $BLOCKED_REASON." >&2
+ git status --porcelain --untracked-files=no | sed 's/^/ /' >&2
+ echo " This script never commits, stashes, resets or deletes
anything for you." >&2
+ exit 1
+fi
+
+emit() {
+ cat <<EOF
+UPSTREAM_REPO=$UPSTREAM_REPO
+PR_NUMBER=$PR_NUMBER
+PR_URL=https://github.com/${UPSTREAM_REPO}/pull/${PR_NUMBER}
+PR_TITLE=$PR_TITLE
+PR_STATE=$PR_STATE
+PR_HEAD_REF=$PR_HEAD_REF
+PR_HEAD_REPO=$PR_HEAD_REPO
+PR_HEAD_SHA=$PR_HEAD_SHA
+PR_BASE_REF=$PR_BASE_REF
+PR_BASE_SHA=$PR_BASE_SHA
+BASE_PRESENT=$BASE_PRESENT
+PR_LOCAL_REF=$PR_REF
+ALIGN_MODE=$ALIGN_MODE
+WORKDIR=$WORKDIR
+DOCS_ROOT=$WORKDIR
+PREV_REF=$PREV_REF
+PREV_SHA=$PREV_SHA
+BEFORE_REF=$CURRENT_REF
+BEFORE_HEAD_SHA=$HEAD_SHA
+DIRTY=$DIRTY
+UNTRACKED=$UNTRACKED
+BRANCH_MATCHES_PR=$BRANCH_MATCHES_PR
+LOCAL_VS_PR=$LOCAL_VS_PR
+EOF
+}
+
+if [ -n "$OUT_FILE" ] && [ "$CHECK_ONLY" = "0" ]; then
+ mkdir -p "$(dirname "$OUT_FILE")"
+ emit > "$OUT_FILE"
+fi
+
+if [ "$CHECK_ONLY" = "1" ]; then
+ echo "=== plan for ${UPSTREAM_REPO}#${PR_NUMBER} (check only, nothing was
changed) ==="
+else
+ echo "=== aligned to ${UPSTREAM_REPO}#${PR_NUMBER} ==="
+fi
+emit
+echo
+echo "worktree : $WORKDIR"
+if [ "$BRANCH_MATCHES_PR" = "1" ]; then
+ echo "branch check : '$CURRENT_REF' IS the PR head branch"
+else
+ echo "branch check : '$CURRENT_REF' is NOT the PR head branch
('$PR_HEAD_REF' of $PR_HEAD_REPO)"
+fi
+case "$LOCAL_VS_PR" in
+ same) echo "commit check : HEAD equals the PR head" ;;
+ ahead:*) echo "commit check : HEAD is ${LOCAL_VS_PR#ahead:} commit(s)
AHEAD of the PR head (that work is NOT reviewed)" ;;
+ behind:*) echo "commit check : HEAD is ${LOCAL_VS_PR#behind:} commit(s)
BEHIND the PR head" ;;
+ diverged:*) echo "commit check : HEAD has DIVERGED from the PR head
(pr/local = ${LOCAL_VS_PR#diverged:})" ;;
+ unrelated) echo "commit check : HEAD shares no history with the PR head"
;;
+ unknown) echo "commit check : PR head not present locally yet (fetched
when not running --check)" ;;
+esac
+echo "fetch source : $FETCH_VIA"
+WOULD=""; [ "$CHECK_ONLY" = "1" ] && WOULD="would "
+case "$ALIGN_MODE" in
+ already) echo "action : none - this worktree is already at the PR
head" ;;
+ blocked) echo "action : BLOCKED - $BLOCKED_REASON" ;;
+ *) echo "action : ${WOULD}detach this worktree from
'$CURRENT_REF' to the PR head" ;;
+esac
+[ "$UNTRACKED" -eq 0 ] || echo "note : $UNTRACKED untracked file(s)
are left in place (checkout never removes them)"
+[ "$BASE_PRESENT" = "1" ] || echo "WARNING : PR base commit $PR_BASE_SHA
is not available locally; the diff will fall back to a local base ref."
+echo
+echo "review docs : ${WORKDIR}/review-docs/"
+[ -z "$PREV_REF" ] || echo "to restore : git -C $WORKDIR checkout $PREV_REF"
+[ "$ALIGN_MODE" != "blocked" ] || exit 1
diff --git a/skills/doris-repo-review/scripts/prepare-review-context.sh
b/skills/doris-repo-review/scripts/prepare-review-context.sh
new file mode 100755
index 0000000..e37d764
--- /dev/null
+++ b/skills/doris-repo-review/scripts/prepare-review-context.sh
@@ -0,0 +1,437 @@
+#!/usr/bin/env bash
+# Prepare the authoritative local review context for the `doris-repo-review`
skill.
+#
+# Mirrors the "Prepare authoritative PR context and required AGENTS guides" +
+# "Fetch existing PR review threads" + "Prepare review prompt" steps of
+# apache/doris .github/workflows/code-review-runner.yml, but sources the diff
+# from the local worktree instead of a CI checkout.
+#
+# Usage:
+# prepare-review-context.sh [--ctx <dir>] [--align <align.env>] [--workdir
<dir>]
+# [--repo <owner/repo>] [--pr <N>] [--base <ref>]
+# [--focus <text>] [--fresh]
+#
+# --align consumes the file written by align-to-pr.sh and inherits the
checkout,
+# the PR number, the upstream repo and the PR base sha from it.
+#
+# Everything is written under <ctx>. Nothing inside the git worktree is
touched.
+set -euo pipefail
+
+CTX=""
+ALIGN_FILE=""
+WORKDIR=""
+UPSTREAM_REPO=""
+PR_NUMBER=""
+BASE_REF=""
+FOCUS=""
+FRESH=0
+
+while [ $# -gt 0 ]; do
+ case "$1" in
+ --ctx) CTX="$2"; shift 2 ;;
+ --align) ALIGN_FILE="$2"; shift 2 ;;
+ --workdir) WORKDIR="$2"; shift 2 ;;
+ --repo) UPSTREAM_REPO="$2"; shift 2 ;;
+ --pr) PR_NUMBER="$2"; shift 2 ;;
+ --base) BASE_REF="$2"; shift 2 ;;
+ --focus) FOCUS="$2"; shift 2 ;;
+ --fresh) FRESH=1; shift ;;
+ -h|--help) sed -n '2,25p' "$0"; exit 0 ;;
+ *) echo "unknown argument: $1" >&2; exit 2 ;;
+ esac
+done
+
+# ------------------------------------------------------- inherit from
align-to-pr
+ALIGN_MODE=""
+DOCS_ROOT=""
+ALIGN_BASE_SHA=""
+ALIGN_HEAD_SHA=""
+if [ -n "$ALIGN_FILE" ]; then
+ [ -f "$ALIGN_FILE" ] || { echo "ERROR: --align file not found:
$ALIGN_FILE" >&2; exit 2; }
+ read_align() { sed -n "s/^$1=//p" "$ALIGN_FILE" | head -n 1; }
+ [ -n "$WORKDIR" ] || WORKDIR="$(read_align WORKDIR)"
+ [ -n "$PR_NUMBER" ] || PR_NUMBER="$(read_align PR_NUMBER)"
+ [ -n "$UPSTREAM_REPO" ] || UPSTREAM_REPO="$(read_align UPSTREAM_REPO)"
+ [ -n "$BASE_REF" ] || BASE_REF="$(read_align PR_BASE_REF)"
+ ALIGN_MODE="$(read_align ALIGN_MODE)"
+ DOCS_ROOT="$(read_align DOCS_ROOT)"
+ ALIGN_BASE_SHA="$(read_align PR_BASE_SHA)"
+ ALIGN_HEAD_SHA="$(read_align PR_HEAD_SHA)"
+fi
+
+if [ -n "$WORKDIR" ]; then
+ cd "$WORKDIR"
+fi
+REPO_ROOT="$(git rev-parse --show-toplevel)"
+cd "$REPO_ROOT"
+
+if [ -n "$ALIGN_HEAD_SHA" ] && [ "$(git rev-parse HEAD)" != "$ALIGN_HEAD_SHA"
]; then
+ echo "ERROR: $REPO_ROOT is not at the aligned PR head $ALIGN_HEAD_SHA." >&2
+ echo " re-run align-to-pr.sh before preparing the context." >&2
+ exit 1
+fi
+
+if [ "$(git rev-parse --is-shallow-repository)" != "false" ]; then
+ echo "ERROR: shallow repository; an authoritative three-dot diff needs
full history." >&2
+ echo " run: git fetch --unshallow" >&2
+ exit 1
+fi
+
+HEAD_SHA="$(git rev-parse HEAD)"
+BRANCH="$(git symbolic-ref --quiet --short HEAD 2>/dev/null || true)"
+if [ -z "$BRANCH" ]; then
+ BRANCH="(detached at ${HEAD_SHA:0:12})"
+ SLUG_SOURCE="${PR_NUMBER:+pr-$PR_NUMBER}"
+ [ -n "$SLUG_SOURCE" ] || SLUG_SOURCE="detached"
+else
+ SLUG_SOURCE="$BRANCH"
+fi
+BRANCH_SLUG="$(printf '%s' "$SLUG_SOURCE" | tr -c 'A-Za-z0-9._-' '-')"
+
+# ---------------------------------------------------------------- context dir
+if [ -z "$CTX" ]; then
+ CTX="${TMPDIR:-/tmp}/doris-repo-review/${BRANCH_SLUG}-${HEAD_SHA:0:12}"
+fi
+if [ "$FRESH" = "1" ] && [ -d "$CTX" ]; then
+ rm -rf "$CTX"
+fi
+mkdir -p "$CTX/ledger"
+CTX="$(cd "$CTX" && pwd)"
+
+# ------------------------------------------------------------- resolve the PR
+if [ -z "$UPSTREAM_REPO" ]; then
+ UPSTREAM_REPO="apache/doris"
+fi
+
+PR_TITLE=""
+PR_URL=""
+PR_STATE=""
+PR_BASE_SHA=""
+if command -v gh >/dev/null 2>&1; then
+ # A detached checkout has no branch to look a PR up by; --pr / --align
supplies it.
+ if [ -z "$PR_NUMBER" ] && [ -n "$(git symbolic-ref --quiet --short HEAD
2>/dev/null || true)" ]; then
+ PR_NUMBER="$(gh pr list -R "$UPSTREAM_REPO" --head "$BRANCH" --state
all \
+ --json number,state --jq 'sort_by(.state == "OPEN" | not) |
.[0].number // empty' 2>/dev/null || true)"
+ fi
+ if [ -n "$PR_NUMBER" ]; then
+ if PR_JSON="$(gh api "repos/${UPSTREAM_REPO}/pulls/${PR_NUMBER}"
2>/dev/null)"; then
+ printf '%s' "$PR_JSON" > "$CTX/pr.json"
+ PR_TITLE="$(jq -r '.title // ""' <<<"$PR_JSON")"
+ PR_URL="$(jq -r '.html_url // ""' <<<"$PR_JSON")"
+ PR_STATE="$(jq -r '.state // ""' <<<"$PR_JSON")"
+ PR_BASE_SHA="$(jq -r '.base.sha // ""' <<<"$PR_JSON")"
+ [ -n "$BASE_REF" ] || BASE_REF="$(jq -r '.base.ref // ""'
<<<"$PR_JSON")"
+ else
+ PR_NUMBER=""
+ fi
+ fi
+fi
+[ -n "$PR_BASE_SHA" ] || PR_BASE_SHA="$ALIGN_BASE_SHA"
+[ -n "$BASE_REF" ] || BASE_REF="master"
+
+# --------------------------------------------------------- resolve the base
commit
+# Prefer the PR's real base sha (exactly what CI diffs against). Fall back to a
+# local ref tip when that commit is not present in this clone.
+BASE_SHA=""
+BASE_SOURCE=""
+if [ -n "$PR_BASE_SHA" ] && git cat-file -e "${PR_BASE_SHA}^{commit}"
2>/dev/null; then
+ BASE_SHA="$PR_BASE_SHA"
+ BASE_SOURCE="PR base sha (matches CI)"
+else
+ for cand in "upstream/$BASE_REF" "origin/$BASE_REF" "$BASE_REF"; do
+ if resolved="$(git rev-parse -q --verify "${cand}^{commit}"
2>/dev/null)"; then
+ BASE_SHA="$resolved"
+ BASE_SOURCE="local ref ${cand}"
+ break
+ fi
+ done
+fi
+if [ -z "$BASE_SHA" ]; then
+ echo "ERROR: cannot resolve a base commit for '$BASE_REF'." >&2
+ echo " pass --base <ref>, or fetch the base branch first." >&2
+ exit 1
+fi
+
+MERGE_BASE="$(git merge-base "$BASE_SHA" "$HEAD_SHA")"
+DIFF_RANGE="${BASE_SHA}...${HEAD_SHA}"
+
+# ------------------------------------------------------------------ worktree
state
+git status --porcelain > "$CTX/worktree_status.txt"
+DIRTY_COUNT="$(grep -c . "$CTX/worktree_status.txt" || true)"
+
+# ------------------------------------------------------------------ the diff
itself
+# A large move-heavy PR blows past the default rename limit and git degrades
to a
+# noisy warning plus add/delete pairs, so raise it once for every diff below.
+GIT_DIFF=(git -c diff.renameLimit=32768 diff --no-ext-diff --no-color)
+
+"${GIT_DIFF[@]}" "$DIFF_RANGE" >
"$CTX/pr.diff"
+"${GIT_DIFF[@]}" --name-only "$DIFF_RANGE" >
"$CTX/pr_changed_files.txt"
+"${GIT_DIFF[@]}" --name-status -M "$DIFF_RANGE" >
"$CTX/pr_changed_files_status.txt"
+"${GIT_DIFF[@]}" --stat "$DIFF_RANGE" >
"$CTX/pr_diffstat.txt"
+git log --oneline --no-decorate "${MERGE_BASE}..${HEAD_SHA}" >
"$CTX/pr_commits.txt"
+
+# --------------------------------------------- new-side line ranges (anchor
source)
+"${GIT_DIFF[@]}" -U0 "$DIFF_RANGE" > "$CTX/pr.u0.diff"
+python3 - "$CTX/pr.u0.diff" "$CTX/changed_line_ranges.txt"
"$CTX/changed_line_ranges.tsv" <<'PY'
+import re
+import sys
+from collections import OrderedDict
+
+u0_path, human_path, tsv_path = sys.argv[1], sys.argv[2], sys.argv[3]
+
+hunk_re = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@")
+files = OrderedDict()
+new_path = None
+old_path = None
+
+for line in open(u0_path, encoding="utf-8", errors="replace"):
+ if line.startswith("--- "):
+ old_path = line[4:].strip()
+ continue
+ if line.startswith("+++ "):
+ raw = line[4:].strip()
+ new_path = None if raw == "/dev/null" else raw[2:] if
raw.startswith("b/") else raw
+ key = new_path or (None if old_path in (None, "/dev/null")
+ else (old_path[2:] if old_path.startswith("a/")
else old_path))
+ if key:
+ files.setdefault(key, {"added": [], "removed": [], "deleted_file":
new_path is None})
+ continue
+ m = hunk_re.match(line)
+ if not m or new_path is None and old_path in (None, "/dev/null"):
+ continue
+ key = new_path or (old_path[2:] if old_path.startswith("a/") else old_path)
+ entry = files.setdefault(key, {"added": [], "removed": [], "deleted_file":
new_path is None})
+ old_start, old_len = int(m.group(1)), int(m.group(2) or 1)
+ new_start, new_len = int(m.group(3)), int(m.group(4) or 1)
+ if new_len > 0:
+ entry["added"].append((new_start, new_start + new_len - 1))
+ if old_len > 0 and new_len == 0:
+ # pure deletion: nothing exists on the new side, anchor to the seam
+ entry["removed"].append((max(new_start, 1), old_len))
+
+def fmt(ranges):
+ return ", ".join(f"{a}" if a == b else f"{a}-{b}" for a, b in ranges) or
"(none)"
+
+with open(human_path, "w", encoding="utf-8") as human, open(tsv_path, "w",
encoding="utf-8") as tsv:
+ human.write("# New-side (post-change) line ranges touched by this diff.\n")
+ human.write("# Cite findings as `path:line` / `path:start-end` using THESE
numbers.\n\n")
+ for path, entry in files.items():
+ if entry["deleted_file"]:
+ human.write(f"## {path} [FILE DELETED]\n\n")
+ continue
+ human.write(f"## {path}\n")
+ human.write(f" added/modified lines : {fmt(entry['added'])}\n")
+ if entry["removed"]:
+ seams = ", ".join(f"after line {a} (-{n})" for a, n in
entry["removed"])
+ human.write(f" pure deletions : {seams}\n")
+ human.write("\n")
+ for a, b in entry["added"]:
+ tsv.write(f"{path}\t{a}\t{b}\n")
+PY
+rm -f "$CTX/pr.u0.diff"
+
+# ---------------------------------------------------- required AGENTS.md
guides
+python3 - "$REPO_ROOT" "$CTX/pr_changed_files.txt" "$CTX/required_agents.txt"
"$CTX/required_agents_prompt.txt" <<'PY'
+import sys
+from pathlib import Path, PurePosixPath
+
+repo_root = Path(sys.argv[1]).resolve()
+changed = Path(sys.argv[2])
+out_list = Path(sys.argv[3])
+out_block = Path(sys.argv[4])
+
+agents, seen = [], set()
+
+def add(rel: PurePosixPath) -> None:
+ text = rel.as_posix()
+ if text in seen:
+ return
+ if (repo_root / text).is_file():
+ agents.append(text)
+ seen.add(text)
+
+add(PurePosixPath("AGENTS.md"))
+for line in changed.read_text(encoding="utf-8", errors="replace").splitlines():
+ value = line.strip()
+ if not value:
+ continue
+ parts = PurePosixPath(value).parts
+ for index in range(1, len(parts)):
+ add(PurePosixPath(*parts[:index]) / "AGENTS.md")
+
+out_list.write_text("".join(f"{a}\n" for a in agents), encoding="utf-8")
+out_block.write_text("".join(f"- {a}\n" for a in agents) or "- (none)\n",
encoding="utf-8")
+PY
+
+# ------------------------------------------------- existing PR review threads
+if [ -n "$PR_NUMBER" ] && command -v gh >/dev/null 2>&1; then
+ MAX_THREADS=30
+ MAX_BODY_CHARS=1200
+ if gh api --paginate --slurp
"repos/${UPSTREAM_REPO}/pulls/${PR_NUMBER}/comments" \
+ > "$CTX/pr_review_comments_pages.json"
2>"$CTX/pr_review_comments_error.log" \
+ && jq 'add | sort_by((.in_reply_to_id // .id), .id)'
"$CTX/pr_review_comments_pages.json" \
+ > "$CTX/pr_review_comments.json"
2>>"$CTX/pr_review_comments_error.log" \
+ && jq -r '
+ def shorten($limit):
+ if (. // "" | length) > $limit then .[0:$limit] +
"...(truncated)" else . // "" end;
+ if length == 0 then
+ "No existing inline review comments or replies were found for
this PR."
+ else
+ group_by(.in_reply_to_id // .id)
+ | sort_by(.[0].created_at // "") | reverse | .[:$max_threads]
+ | map(
+ . as $thread | $thread[0] as $root
+ | "### " + ($root.path // "(unknown path)") + ":" +
(($root.line // $root.original_line // "n/a") | tostring)
+ + "\nURL: " + ($root.html_url // "")
+ + "\nComments:\n"
+ + ($thread | map(
+ "- " + (.user.login // "unknown") + " at " +
(.created_at // "")
+ + (if .in_reply_to_id then " (reply):" else "
(original comment):" end) + "\n"
+ + ((.body | shorten($max_body_chars)) | split("\n") |
map(" " + .) | join("\n"))
+ ) | join("\n"))
+ ) | join("\n\n")
+ end
+ ' --argjson max_threads "$MAX_THREADS" --argjson max_body_chars
"$MAX_BODY_CHARS" \
+ "$CTX/pr_review_comments.json" > "$CTX/pr_review_threads.md"
2>>"$CTX/pr_review_comments_error.log"; then
+ :
+ else
+ printf '%s\n' \
+ 'Existing PR review threads could not be fetched for this run.' \
+ 'Proceed with the review without this auxiliary context.' >
"$CTX/pr_review_threads.md"
+ fi
+ rm -f "$CTX/pr_review_comments_pages.json"
+else
+ printf '%s\n' 'No PR is associated with this branch; there is no existing
inline review context.' \
+ > "$CTX/pr_review_threads.md"
+fi
+
+# ------------------------------------------------------------------ user focus
+if [ -n "$(printf '%s' "$FOCUS" | tr -d '[:space:]')" ]; then
+ printf '%s\n' "$FOCUS" > "$CTX/review_focus.txt"
+elif [ ! -f "$CTX/review_focus.txt" ]; then
+ printf 'No additional user-provided review focus.\n' >
"$CTX/review_focus.txt"
+fi
+
+# ---------------------------------------------------------------- ledger seed
+if [ ! -f "$CTX/ledger/README.md" ]; then
+ cat > "$CTX/ledger/README.md" <<'LEDGER'
+# Shared Subagent Review Ledger
+
+Shared source of truth for the subagent-assisted review. One file per owner so
+that concurrent subagents never patch the same file.
+
+| File | Owner | Purpose |
+|---|---|---|
+| `00-main-risk-scan.md` | main agent | initial risk scan, written BEFORE
any subagent is spawned |
+| `sub-<round>-<slug>.md` | one subagent | that subagent's candidate
findings, append-only |
+| `main-merged.md` | main agent | merged / verified / dismissed
candidates, final comment set |
+
+Rules:
+- Every subagent MUST read every file in this directory before reviewing.
+- A subagent writes ONLY its own `sub-*.md` file. It must not touch another
+ subagent's file, `00-main-risk-scan.md`, `main-merged.md`, repository source
+ files, or GitHub.
+- Avoid duplicates. If a candidate overlaps an existing one, record it in your
+ own file with a duplicate note referencing the existing candidate ID.
+- The main agent owns final status, deduplication, and the review documents.
+
+Candidate statuses: `proposed_by_subagent`, `accepted`,
`dismissed_with_evidence`, `duplicated`.
+
+Candidate format:
+
+- ID:
+ Owner:
+ Status:
+ Path:
+ Line: <new-side line or start-end, must match
changed_line_ranges.txt or be justified>
+ Severity: Blocker | Major | Minor | Nit
+ Claim:
+ Evidence: <call chain / concrete trigger scenario / file:line citations>
+ Duplicate relationship:
+ Recommendation:
+LEDGER
+fi
+
+if [ ! -f "$CTX/ledger/00-main-risk-scan.md" ]; then
+ cat > "$CTX/ledger/00-main-risk-scan.md" <<'SCAN'
+# Main Initial Risk Scan
+
+Written by the MAIN agent before any subagent is spawned. One entry per risk
focus.
+
+- ID:
+ Status: <open | assigned | resolved>
+ Changed files/lines:
+ Related mechanisms to inspect:
+ Why suspicious:
+ Required upstream/downstream files or paths:
+ Question for risk-focused subagent:
+ Final conclusion:
+SCAN
+fi
+
+if [ ! -f "$CTX/ledger/main-merged.md" ]; then
+ cat > "$CTX/ledger/main-merged.md" <<'MERGED'
+# Main Merged Findings
+
+Owned by the main agent.
+
+## Merged Findings
+
+- ID:
+ Source IDs:
+ Status: <accepted | dismissed_with_evidence
| duplicated>
+ Severity:
+ Path:
+ Line:
+ Claim:
+ Evidence:
+ Main verification:
+ Proposed finding body:
+
+## Dismissed Or Duplicate Points
+
+## Convergence Rounds
+
+| Round | Subagents | New candidates | Verdict |
+|---|---|---|---|
+MERGED
+fi
+
+# ------------------------------------------------------------------ meta +
report
+[ -n "$DOCS_ROOT" ] || DOCS_ROOT="$REPO_ROOT"
+{
+ echo "REPO_ROOT=$REPO_ROOT"
+ echo "DOCS_ROOT=$DOCS_ROOT"
+ echo "ALIGN_MODE=${ALIGN_MODE:-none}"
+ echo "CTX=$CTX"
+ echo "UPSTREAM_REPO=$UPSTREAM_REPO"
+ echo "BRANCH=$BRANCH"
+ echo "PR_NUMBER=${PR_NUMBER:-}"
+ echo "PR_TITLE=${PR_TITLE:-}"
+ echo "PR_URL=${PR_URL:-}"
+ echo "PR_STATE=${PR_STATE:-}"
+ echo "BASE_REF=$BASE_REF"
+ echo "BASE_SHA=$BASE_SHA"
+ echo "BASE_SOURCE=$BASE_SOURCE"
+ echo "HEAD_SHA=$HEAD_SHA"
+ echo "MERGE_BASE=$MERGE_BASE"
+ echo "DIFF_RANGE=$DIFF_RANGE"
+ echo "DIRTY_FILES=$DIRTY_COUNT"
+ echo "REVIEW_DATE=$(date +%Y-%m-%d)"
+} > "$CTX/meta.env"
+
+echo "=== doris-repo-review context ready ==="
+cat "$CTX/meta.env"
+echo
+echo "changed files : $(grep -c . "$CTX/pr_changed_files.txt" || true)"
+echo "commits : $(grep -c . "$CTX/pr_commits.txt" || true)"
+echo "diff lines : $(wc -l < "$CTX/pr.diff" | tr -d ' ')"
+echo "diffstat : $(tail -n 1 "$CTX/pr_diffstat.txt")"
+echo
+echo "required AGENTS.md:"
+sed 's/^/ /' "$CTX/required_agents.txt"
+if [ "${DIRTY_COUNT:-0}" -gt 0 ]; then
+ echo
+ echo "WARNING: $DIRTY_COUNT uncommitted path(s) in the worktree are NOT
part of the reviewed diff:"
+ sed 's/^/ /' "$CTX/worktree_status.txt"
+fi
diff --git a/skills/doris-repo-review/scripts/verify-anchors.py
b/skills/doris-repo-review/scripts/verify-anchors.py
new file mode 100755
index 0000000..8eefd3d
--- /dev/null
+++ b/skills/doris-repo-review/scripts/verify-anchors.py
@@ -0,0 +1,172 @@
+#!/usr/bin/env python3
+"""Verify the `path:line` anchors of the doris-repo-review output documents.
+
+Local review has no inline GitHub comments, so every finding must carry an
+anchor the reader can jump to. This script proves the anchors are real:
+
+ * the path exists in the worktree at HEAD;
+ * the line number is inside the file;
+ * (informational) whether the line falls inside a range this PR touched;
+ * the EN and ZH documents expose the same finding IDs, and every finding
+ carries at least one anchor.
+
+Anchors must be written inside backticks as `path:line` or `path:start-end`.
+
+Usage:
+ verify-anchors.py --ctx <context-dir> --doc <en.md> --doc <zh.md>
+
+Exits non-zero when an anchor is broken or the two documents disagree.
+"""
+
+from __future__ import annotations
+
+import argparse
+import re
+import sys
+from collections import defaultdict
+from pathlib import Path
+
+ANCHOR_RE =
re.compile(r"`([A-Za-z0-9_][A-Za-z0-9_./+-]*\.[A-Za-z0-9_+-]+):(\d+)(?:-(\d+))?`")
+FINDING_RE = re.compile(r"^#{2,4}\s+(?:\[)?(F-\d+)(?:\])?\b")
+CODE_FENCE_RE = re.compile(r"^\s*```")
+
+
+def parse_args() -> argparse.Namespace:
+ parser = argparse.ArgumentParser(description=__doc__,
formatter_class=argparse.RawDescriptionHelpFormatter)
+ parser.add_argument("--ctx", required=True, type=Path, help="Review
context directory (holds meta.env).")
+ parser.add_argument("--doc", required=True, action="append", type=Path,
help="Review document; repeat for EN and ZH.")
+ parser.add_argument("--repo-root", type=Path, default=None, help="Repo
root; defaults to REPO_ROOT from meta.env.")
+ return parser.parse_args()
+
+
+def read_meta(ctx: Path) -> dict[str, str]:
+ meta_path = ctx / "meta.env"
+ if not meta_path.is_file():
+ sys.exit(f"missing {meta_path}; run prepare-review-context.sh first")
+ meta = {}
+ for line in meta_path.read_text(encoding="utf-8").splitlines():
+ if "=" in line:
+ key, _, value = line.partition("=")
+ meta[key] = value
+ return meta
+
+
+def load_changed_ranges(ctx: Path) -> dict[str, list[tuple[int, int]]]:
+ ranges: dict[str, list[tuple[int, int]]] = defaultdict(list)
+ tsv = ctx / "changed_line_ranges.tsv"
+ if tsv.is_file():
+ for line in tsv.read_text(encoding="utf-8").splitlines():
+ parts = line.split("\t")
+ if len(parts) == 3:
+ ranges[parts[0]].append((int(parts[1]), int(parts[2])))
+ return ranges
+
+
+def scan_document(doc: Path) -> tuple[list[str], dict[str, list[tuple[int,
int, int | None]]], list[tuple[int, int, int | None]]]:
+ """Return (finding ids in order, anchors per finding, all anchors)."""
+ findings: list[str] = []
+ per_finding: dict[str, list[tuple[int, int, int | None]]] =
defaultdict(list)
+ all_anchors: list[tuple[str, int, int | None, int]] = []
+ current: str | None = None
+ in_fence = False
+
+ for lineno, line in
enumerate(doc.read_text(encoding="utf-8").splitlines(), start=1):
+ if CODE_FENCE_RE.match(line):
+ in_fence = not in_fence
+ continue
+ heading = FINDING_RE.match(line)
+ if heading:
+ current = heading.group(1)
+ if current not in findings:
+ findings.append(current)
+ if in_fence:
+ continue
+ for match in ANCHOR_RE.finditer(line):
+ path, start, end = match.group(1), int(match.group(2)),
match.group(3)
+ anchor = (path, start, int(end) if end else None, lineno)
+ all_anchors.append(anchor)
+ if current:
+ per_finding[current].append(anchor)
+ return findings, per_finding, all_anchors
+
+
+def in_changed_range(ranges: list[tuple[int, int]], start: int, end: int |
None) -> bool:
+ last = end or start
+ return any(not (last < lo or start > hi) for lo, hi in ranges)
+
+
+def main() -> int:
+ args = parse_args()
+ ctx = args.ctx.resolve()
+ meta = read_meta(ctx)
+ repo_root = (args.repo_root or Path(meta.get("REPO_ROOT", "."))).resolve()
+ changed_ranges = load_changed_ranges(ctx)
+
+ errors: list[str] = []
+ warnings: list[str] = []
+ finding_sets: dict[Path, list[str]] = {}
+
+ for doc in args.doc:
+ doc = doc.resolve()
+ if not doc.is_file():
+ errors.append(f"{doc}: document not found")
+ continue
+ findings, per_finding, anchors = scan_document(doc)
+ finding_sets[doc] = findings
+
+ print(f"\n=== {doc} ===")
+ print(f"findings: {len(findings)} anchors: {len(anchors)}")
+
+ for path, start, end, lineno in anchors:
+ target = repo_root / path
+ if not target.is_file():
+ errors.append(f"{doc}:{lineno}: anchor path does not exist ->
{path}")
+ continue
+ try:
+ total = sum(1 for _ in target.open(encoding="utf-8",
errors="replace"))
+ except OSError as exc:
+ errors.append(f"{doc}:{lineno}: cannot read {path}: {exc}")
+ continue
+ last = end or start
+ if start < 1 or last > total:
+ errors.append(f"{doc}:{lineno}: {path}:{start}{'-' + str(end)
if end else ''} out of range (file has {total} lines)")
+ continue
+ if path in changed_ranges and not
in_changed_range(changed_ranges[path], start, end):
+ warnings.append(f"{doc}:{lineno}: {path}:{start}{'-' +
str(end) if end else ''} is context, not a changed line (fine if intentional)")
+ elif path not in changed_ranges:
+ warnings.append(f"{doc}:{lineno}: {path} is not in this PR's
changed-file set (fine for upstream/downstream evidence)")
+
+ for finding in findings:
+ if not per_finding.get(finding):
+ errors.append(f"{doc}: finding {finding} has no `path:line`
anchor")
+
+ docs = list(finding_sets)
+ if len(docs) >= 2:
+ base_doc, base_ids = docs[0], set(finding_sets[docs[0]])
+ for other in docs[1:]:
+ other_ids = set(finding_sets[other])
+ missing = sorted(base_ids - other_ids)
+ extra = sorted(other_ids - base_ids)
+ if missing:
+ errors.append(f"{other}: missing finding IDs present in
{base_doc.name}: {', '.join(missing)}")
+ if extra:
+ errors.append(f"{other}: has finding IDs absent from
{base_doc.name}: {', '.join(extra)}")
+
+ if warnings:
+ print("\n--- notes ---")
+ for note in warnings:
+ print(f" note: {note}")
+
+ if errors:
+ print("\n--- errors ---")
+ for error in errors:
+ print(f" ERROR: {error}")
+ print(f"\nFAILED: {len(errors)} anchor problem(s).")
+ return 1
+
+ print("\nOK: every anchor resolves and both documents expose the same
findings.")
+ return 0
+
+
+if __name__ == "__main__":
+ sys.exit(main())
diff --git a/verify/README.md b/verify/README.md
index ed148ae..389fa4e 100644
--- a/verify/README.md
+++ b/verify/README.md
@@ -1,7 +1,7 @@
# doris-skills verification suite
-Regression suite that checks the **factual claims** in the skills against a
real
-Apache Doris cluster and against `doriscli`. Three layers:
+Regression suite that checks the **factual claims** in the cluster-side skills
against a
+real Apache Doris cluster and against `doriscli`. Three layers:
| Layer | What it proves | Tool | Status |
|---|---|---|---|
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
