address-review-comments

Files (11)

• SKILL.md
• guide.md
• github-lane-transcription.md · references/
• github_lane.md · scripts/
• github_review_lane.py · scripts/
• prepare_review_input.py · scripts/
• review_lane_core.py · scripts/
• review_lane_github.py · scripts/
• review_lane_rounds.py · scripts/
• github_review_lane_test.py · scripts/tests/
• prepare_review_input_test.py · scripts/tests/

SKILL.md (first lines)

Respond to `/review-plan`, `/review-pr`, or any free-form review. Reviewer and author are peer decision-makers in a convergence process: the reviewer raises a problem and may suggest a repair, the author independently judges the problem and the correct repair, and a later reviewer verifies the result; this skill is author-side adjudication + remediation and does not treat review as a command.

Resolve `skill_dir` as the absolute directory containing this loaded SKILL.md.

Before execution, read completely:

- [Review Response Guide](../_bus/docs/guides/review-response-guide.md): the SOT for verification and disposition judgment;
- [guide.md](guide.md): judgment **specific to the GitHub lane** (round identity, location basis, clean re-review, completion marker,
  ledger leak scan). Needed only when taking PASS 0a; other modes need not read it;
- the **specific sections** that guide's disposition classification table names for this run's mode: read only those sections, not the whole review guide.
  `plan-review-guide` / `code-review-guide` are **reviewer-side** documents; their
  review scope, three verdicts, adversarial stance, output format, and similar sections describe how findings are produced, which the author neither performs
  nor outputs; reading the whole document only tempts the author to apply reviewer actions (for example issuing a verdict on the reviewer's behalf);

```text
INPUT = [plan | pr] [--round latest|N] [--github] [--pr <n>] [--review-file <path>]...
        [--free-form-file <path>]... [--label <name>]... [--no-commit-and-push]
        [--scope <FILE_LIST|chunk.json>]

HARD RULES
- Automatically modify only claims finally adjudicated APPLY; REJECT / FLAG / HOUSEKEEPING do not change the repository,
  except that PR mode adjudicates saved reviewer test patches and cleans up any legacy leftover probes (PASS 4).
- The main agent exclusively owns deduplication, cross-claim comparison, disposition, triage, remediation grouping, and landing.
- Evidence gathering answers only local facts: no adjudication, repair suggestions or production changes; temporary test-only probe application/removal is allowed as below. Disposition, conflict resolution, and repair direction belong to main alone.
- Claim truth, goal scope, and repair design are judged separately: `SUPPORTED` does not approve the scope or the repair; main first rules on scope, then independently derives the repair for admitted claims.
- **Orchestration is main's own decision**: verify each claim itself, hand some to read-only subagents, or mix, depending on claim count, whether claims share files, and remaining context. This skill prescribes no shape and does not assume parallel is faster (reasoning in the guide's "Verification and adjudication authority").
- Unified adjudication barrier: no repair writes until every claim has a first-party truth assessment and main has completed unified adjudication. Before that barrier, main may temporarily apply a saved test-only probe patch to verify evidence, then remove only those additions and restore the pre-probe state; never edit production or weaken assertions during verification.
- **Never run repository-wide gates; run only validation within the diff.** Forbidden: `just ci`, `just check`, `just test`,
  `just lint`, an unfiltered `cargo test` / `cargo nextest run`, and any other full-tier run;
  only diff-only / scoped forms are allowed:
  `just test-one <filter>`, `python3 -m pytest <exact test file>` / `python3 -m unittest <exact module>`,
  `rustfmt --check --edition 2021 <changed .rs paths>`.
  Reasoning and execution details are in PASS 4 "Validation covers only the diff".
- plan/pr invoke commit-and-push by default.

========== PASS 0: RESOLVE & SANITIZE ==========

IF --review-file / --free-form-file is given explicitly: canonicalize in argument order and deduplicate.
ELSE: only legacy plan/pr lane discovery is allowed.

IF --scope: mode is pr; use exactly the same file list / manifest as review-pr (format and permissions
in that skill). Canonical files + test_files must match the review header's SCOPE_HASH. Findings may
reference unchanged chunk files; absence from <base>...HEAD is not a scope violation.
Read the declared whole-file target and production-review Goal, not a feature-diff-only assignment.
Do not mix branch artifacts, other chunks, or plan reviews. No GitHub diff-lane fetch occurs in scope
mode; --github / --pr are not chunk inputs. Use explicit --review-file, or discover only
temp/review-pr/<slug>/scopes/<SCOPE_HASH>/*/round-*/review.md with the same latest / round rules.
No work outside that chunk is admitted merely because it is a direct dependency.

========== PASS 0a: GITHUB LANE (mode == pr, no --scope, and no explicit --review-file / --free-form-file) ==========

IF mode == pr AND no --scope AND no explicit --review-file / --free-form-file:
  Read(<skill_dir>/scripts/github_lane.md) completely and execute its procedure.
  Continue below for sanitization unless the procedure returns normally or STOPs.

RUN `python3 "<skill_dir>/scripts/prepare_review_input.py" \
  [--review-file <path>...] [--free-form-file <path>...] [--mode plan|pr] \
  [--label <name>...] [--scope <FILE_LIST|chunk.json>] --output <run-root>/sanitized.md`
-- With `--github` this is verbatim `--review-file <roundDir>/review.md --mode pr`.
IF exit != 0: STOP; never bypass the script to read the original review.

Pre-migration review files are immutable historical evidence: pass them as `--free-form-file <path> --mode plan|pr`; never rewrite one to satisfy the parser.

Legacy discovery rules:
- The branch slug is the same as the review skills': detached uses `_detached_`; otherwise characters outside `[A-Za-z0-9_-]` are replaced with `-`;
- plan lanes come from `temp/review-plan/<slug>/*/*/round-*/review.md`, PR lanes from `temp/review-pr/<slug>/*/round-*/review.md`;
- An explicit mode uses only the matching lanes; when only one kind exists it is selected automatically; when both or neither exist, STOP; each lane takes the requested round or its latest valid round.
- **A lane whose verdict is Ready and whose sync list is empty does not enter this run**: that lane has signed off and left nothing to address.
  Lanes whose rounds stopped at different HEADs are not a problem: PASS 2 requires first-party evidence for every claim, so
  claims already fixed get `CONTRADICTED` there, which is exactly why the evidence step exists.

Read only sanitized's "新问题与建议" and the complete "同步清单（CONSISTENCY drift，非阻塞）":
- structured lanes must agree on mode, target, base, locked goal, and plan identity; any mismatch STOPs;
- structured chunk inputs require the matching --scope; sanitized provenance carries the canonical
  SCOPE_HASH, whole file list, and exact test allowlist. Cross-chunk / branch mixing fails closed;
- free-form requires an explicit mode; its round is `n/a`; without --scope its SCOPE_HASH is `n/a`.
  An explicit --scope supplies file membership, not prior-round provenance; the closed incremental
  gate remains undecidable. The locked goal must be given explicitly by the caller/developer, STOP
  if it cannot be obtained, and never guess it from the diff, commits, or PR description;
- free-form lacks round provenance and SCOPE_HASH, so the closed incremental gate (d) **has no mechanical criterion available**. This is not "this round happened not to hit it"; the gate is undecidable at this entry point and must be written explicitly in triage as one line `**闸 (d)**：free-form 输入不可判`, so the gap leaves a trace instead of silently taking effect. The guardrail's gate (a′) already has a similar skip clause, and gate (d) is recorded following the same precedent;
- plan reads the full plan text and archived decisions; PR reads `<base>...HEAD`, the locked goal, the optional associated plan, and round provenance, plus the complete files and direct dependency context each claim needs;
- scoped PR reads the committed whole chunk files and its previous-round snapshot / delta instead of
  using the branch diff as finding admission; all evidence still targets current HEAD. Closed-round
  gate (d), truth assessment, disposition, and author-selected repair rules remain unchanged;
- when plan/pr landing is needed while on master, create a feature branch first.

run root: plan/pr is `temp/address-review-comments/<slug>/<YYYYMMDD-HHmmss>/`.
With --scope it is `temp/address-review-comments/<slug>/scopes/<SCOPE_HASH>/<YYYYMMDD-HHmmss>/`;
put **范围哈希**：<SCOPE_HASH> in triage.md so author decisions stay tied to the chunk.

IF mode == pr: inventory saved test-only probe patches cited by the review, including green probes with no finding, and any legacy
uncommitted reviewer test additions (`git status --porcelain --untracked-files=all`). Record each patch/test path, source claim or "no finding",
and original red/green result; do not apply patches yet. Reviewers should have removed their additions before handoff. Distinguish any legacy
probe hunks from the author's work; never discard the latter. Verification and adoption follow review-response-guide "Taking ownership of PR reviewer tests".

========== PASS 1: ATOMIZE & DEDUPE (main only) ==========

1. Extract substantive findings and sync-only assertions; record source lane/round/file/id, location, dimension, observation, evidence, impact. The reviewer's possible repair is recorded separately as a non-binding candidate and must not be mixed into the problem claim or used as evidence-gathering input.
2. When one finding contains several independently true facts, atomize it first; every source assertion must belong to exactly one canonical claim.
3. Deduplicate across lanes by actual root cause and keep corroborating lanes; mutually exclusive guidance is only marked as a candidate conflict and must not be adjudicated early.
4. After deduplication, record for each canonical claim: claim id, source assertions, a precise context pointer (if you can give `path:start-end`, do not give only a file name: evidence cost is mostly spent reading whole files), and the **single** atomic fact to verify for that claim.
5. Atomization follows "can it stand independently", not "finer is better": assertions with the same root cause, same file, and same repair site merge into one claim. Splitting too finely makes the same context get rebuilt repeatedly without improving accuracy.
6. An empty claim set is valid; mechanical output continues with `0/0`.

========== PASS 2: VERIFY EVERY CLAIM ==========

Every canonical claim must receive one first-party truth assessment (`SUPPORTED | CONTRADICTED |
INCONCLUSIVE`) with the evidence location and uncertainty; if evidence is unavailable, say so; do not guess.
The assessment answers only whether the problem claim holds; it does not evaluate or endorse the reviewer's candidate repair.
PR evidence also establishes the behavior owner, goal relationship, and concrete consequence so main can independently rule on scope; follow the guide's "Bounded verification":
never skip verification based only on a title or path, and do not expand into exhaustive input enumeration or repair design for unrelated subsystems.

**How evidence is gathered is orchestrated by main**: read source/plan/archived decisions and run commands for each claim itself, hand some to read-only
subagents, or mix. The choice depends on claim count, whether context pointers overlap, and main's remaining context:
when claims cluster in the same few files, doing them back to back yourself is usually faster because the context is built only once.

Hard constraints when delegating (however many groups and however split):
- subagents are read-only for the repository and Git: no business-file writes, no add/commit/push/stash/checkout;
- report only first-party facts and the closed-set conclusion above: no disposition, no repair direction, no new issues discovered in passing;

From asura1234/bus, Apache-2.0.

More from asura1234/bus