Skip to content

Commit ffe9264

Browse files
committed
docs(pr-review): thin host skill around review packets
1 parent bf1d9f6 commit ffe9264

5 files changed

Lines changed: 209 additions & 401 deletions

File tree

docs/reference/protocols/pr-review-command-v0.md

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ observation to this same command. It reuses the existing GitHub scan and
1717
normalized review queue; it does not introduce a second crawler or a new write
1818
authority.
1919

20+
The capability also owns the review-depth contract. The shared
21+
`agent_response_contract.review_execution_contract` defines required evidence,
22+
completion, freshness, finding, and verdict rules. Each PR carries a compact
23+
`review_plan` that binds those rules to one exact head and marks code-symbol and
24+
negative-walkthrough applicability. Host skills route and publish this packet;
25+
they must not maintain a second explanation checklist.
26+
2027
Codex agents should use the dedicated `loopx-pr-review` skill for this slash
2128
command. Do not route `/loopx-pr-review` through the broader `loopx-project`
2229
workflow or the merge-focused `loopx-pr-merge` skill.
@@ -109,8 +116,35 @@ authority; callers must use normal LoopX Todo authority, `loopx-pr-review`, and
109116
Do not pipe that first packet through `jq` or another projection that only
110117
keeps `.summary` and `.review_sequence`; that drops
111118
`agent_response_contract`, `review_groups`, `pull_requests[].review_template`,
112-
and `pull_requests[].evidence_commands`, which are the fields that make the
113-
command a guided review instead of a statistics table.
119+
`pull_requests[].review_plan`, and `pull_requests[].evidence_commands`, which
120+
are the fields that make the command a guided review instead of a statistics
121+
table.
122+
123+
## Capability-Owned Review Execution
124+
125+
`pull_request_review_execution_contract_v1` is shared once per packet to avoid
126+
duplicating a large prompt for every PR in a 100-item queue. It requires these
127+
typed evidence groups before a verdict:
128+
129+
- problem context and active caller;
130+
- architecture and ownership flow;
131+
- exact changed-line classification across production, tests/fixtures, docs,
132+
generated output, and mechanical moves;
133+
- a 2-5 item exact-head symbol map for code-changing PRs, including caller,
134+
state, branch, side effect, consumer, and failure ownership;
135+
- positive and applicable negative execution walkthroughs;
136+
- validation tied to changed invariants and failure cases;
137+
- strongest regression path, blast radius, recovery, minimum repair, and
138+
regression test;
139+
- code-volume necessity and the highest-value behavior-preserving
140+
simplification.
141+
142+
The per-PR `pull_request_review_plan_v1` records the exact target, applicability,
143+
required evidence ids, and an initially `unverified`
144+
`pull_request_review_result_v1` skeleton. Metadata, labels, file counts, risk
145+
hints, and green CI cannot upgrade evidence to `verified`. A stale-head verdict
146+
is prohibited. Missing evidence remains `unverified` with a reason instead of
147+
being replaced by confident prose.
114148

115149
When `--state all` is used, the command must preserve both lifecycle groups.
116150
The `--limit` value is applied per group so a busy open queue cannot consume the
@@ -308,7 +342,10 @@ absolute paths, private source bodies, or hidden CI artifacts.
308342
"queue_table_role": "preface_only",
309343
"required_packet_fields_to_preserve": [
310344
"agent_response_contract",
345+
"agent_response_contract.review_execution_contract",
346+
"result_completeness",
311347
"review_groups",
348+
"pull_requests[].review_plan",
312349
"pull_requests[].review_template",
313350
"pull_requests[].evidence_commands"
314351
],
@@ -343,17 +380,20 @@ The packet should let a reviewer move through PRs in order:
343380
2. Then use `review_groups.merged` for post-merge audit and follow-up quality.
344381
3. Use `evidence_commands`, key files, changed-file scale, and checks to open
345382
the actual PR body and diff.
346-
4. Read `main_regression_analysis` before filling risk prose. It is the CLI's
383+
4. Execute the PR's `review_plan` against
384+
`agent_response_contract.review_execution_contract`; keep unavailable
385+
evidence explicitly unverified.
386+
5. Read `main_regression_analysis` before filling risk prose. It is the CLI's
347387
concrete, generated view of potential main regressions, bug risks, and
348388
focused validation.
349-
5. Follow `agent_response_contract.explanation_depth_contract`, then let
350-
agentloop fill the blank five-block template:
389+
6. Render the verified structured result through the blank five-block template:
351390
`动机`, `改动思路`, `具体改动`, `对主干的风险`, `我的整体评价`.
352391
Use each section's range as a depth signal for a reader unfamiliar with the
353392
subsystem, not as filler.
354-
6. Treat `metadata_risk_hint` only as queue-ordering metadata. It must not be
393+
7. Treat `metadata_risk_hint` only as queue-ordering metadata. It must not be
355394
copied as the final risk judgement.
356-
7. Decide `approve`, `request changes`, `defer`, or `merge after checks`.
395+
8. Recheck the exact head, then decide `approve`, `request changes`, `defer`, or
396+
`merge after checks`.
357397

358398
A response that only lists `Open` and `Merged` PRs, scale, and recommended next
359399
order is incomplete for `/loopx-pr-review`; it should continue into the
@@ -386,6 +426,10 @@ A first implementation is acceptable when:
386426
`main_regression_analysis`, evidence commands, explicit
387427
`review_groups.unmerged` / `review_groups.merged`, and a blank five-block
388428
review template;
429+
- the shared `pull_request_review_execution_contract_v1` owns typed evidence,
430+
completion, freshness, findings-first, and verdict policy, while every PR has
431+
a compact exact-head `pull_request_review_plan_v1` with an unverified result
432+
skeleton;
389433
- the packet includes `agent_response_contract.table_only_response_allowed=false`
390434
and `agent_response_contract.required_packet_fields_to_preserve` so
391435
slash-command agents know a table-only chat answer is incomplete;

examples/install-local-smoke.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -368,17 +368,18 @@ def main() -> int:
368368
pr_review_text = " ".join(pr_review_skill.read_text(encoding="utf-8").split())
369369
for phrase in (
370370
"loopx --format json pr-review --state all",
371-
"agent_response_contract",
371+
"thin host adapter",
372+
"agent_response_contract.review_execution_contract",
372373
"review_groups",
374+
"pull_requests[].review_plan",
373375
"pull_requests[].review_template",
374376
"pull_requests[].evidence_commands",
375-
"Do not pipe the first packet through `jq`",
376-
"Do not fill the five-block review from title, labels, changed-file counts, or metadata risk hints alone",
377-
"submit a formal `REQUEST_CHANGES` review",
378-
"A plain PR comment is not an adequate substitute for `REQUEST_CHANGES`",
379-
"keep the workflow read-only only when the user explicitly says `local-only`",
380-
"the GitHub review state must match the written verdict",
381-
"route approval, merge, self-merge, and admin-bypass actions to `loopx-pr-merge`",
377+
"Do not pipe the only copy through `jq`",
378+
"completion_gate",
379+
"never infer `verified` from title",
380+
"formal `REQUEST_CHANGES`",
381+
"Read the published review back",
382+
"approval still routes through `loopx-pr-merge`",
382383
):
383384
assert phrase in pr_review_text, phrase
384385
assert "Do not use this skill to approve" not in pr_review_text, pr_review_text

examples/pr-review-command-smoke.py

Lines changed: 69 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -50,48 +50,30 @@ def assert_public_safe(payload: dict[str, object]) -> None:
5050

5151

5252
def main() -> int:
53-
skill_text = " ".join(PR_REVIEW_SKILL.read_text(encoding="utf-8").split())
53+
skill_source = PR_REVIEW_SKILL.read_text(encoding="utf-8")
54+
skill_text = " ".join(skill_source.split())
5455
for phrase in (
55-
"Use when the visible request starts with `/loopx-pr-review`",
56+
"This skill is a thin host adapter",
5657
"loopx --format json pr-review --state all",
57-
"agent_response_contract",
58-
"review_groups",
58+
"agent_response_contract.review_execution_contract",
59+
"pull_requests[].review_plan",
5960
"pull_requests[].review_template",
6061
"pull_requests[].evidence_commands",
61-
"Do not pipe the first packet through `jq`",
62-
"Do not fill the five-block review from title, labels, changed-file counts, or metadata risk hints alone",
63-
"Each PR must receive its own evidence pass and standalone review card",
64-
"Do not compress individual cards to cover more of the queue",
62+
"Apply `completion_gate` literally",
63+
"Re-read the remote head immediately before verdict and publication",
64+
"formal `REQUEST_CHANGES`",
65+
"Read the published review back",
66+
"Route approval, merge, self-merge, and admin bypass to `loopx-pr-merge`",
67+
):
68+
assert phrase in skill_text, phrase
69+
assert len(skill_source.splitlines()) <= 140, len(skill_source.splitlines())
70+
for duplicated_contract_heading in (
6571
"Per-PR Evidence And Depth Gate",
66-
"build a compact internal evidence record for that PR",
67-
"Each card must stand on its own",
68-
"one concrete positive walkthrough",
69-
"one concrete negative or failure walkthrough",
7072
"Motivation Causal Chain",
71-
"who pays the cost",
7273
"Implementation Execution Chain",
73-
"authoritative input or state",
74-
"Key Code Explanation Gate",
75-
"`### 关键代码讲解` subsection inside `具体改动`",
76-
"2-5 behavior-bearing symbols",
77-
"exact-head `file:line` and symbol name",
78-
"critical condition, branch, transition, or invariant",
79-
"return value, receipt, projection, or downstream consumer",
80-
"Include 1-3 short excerpts from the exact reviewed head",
81-
"For a docs-only PR, use `### 关键内容讲解`",
82-
"relationship map",
83-
"state the minimum repair plus regression test",
8474
"Code Volume And Simplification Review",
85-
"Classify the volume as `necessary`, `partly avoidable`, or `not yet proven`",
86-
"A code-volume conclusion without diff and call-site evidence is incomplete",
87-
"submit a formal `REQUEST_CHANGES` review",
88-
"A plain PR comment is not an adequate substitute for `REQUEST_CHANGES`",
89-
"keep the workflow read-only only when the user explicitly says `local-only`",
90-
"the GitHub review state must match the written verdict",
91-
"After publication, include the GitHub review/comment URL",
92-
"route approval, merge, self-merge, and admin-bypass actions to `loopx-pr-merge`",
9375
):
94-
assert phrase in skill_text, phrase
76+
assert duplicated_contract_heading not in skill_source, duplicated_contract_heading
9577

9678
assert _github_search_date("2026-06-28T00:00:00+08:00") == "2026-06-27"
9779
assert _github_search_date("2026-06-28T00:00:00Z") == "2026-06-28"
@@ -201,7 +183,7 @@ def fake_run_gh_json(args: list[str], *, cwd: Path | None = None) -> object:
201183
template = first["review_template"]
202184
assert template["schema_version"] == "pr_review_five_block_template_v0", template
203185
assert "Empty scaffold only" in template["purpose"], template
204-
assert "reader unfamiliar with the PR" in template["output_hint"], template
186+
assert "review_execution_contract" in template["output_hint"], template
205187
labels = [section["label"] for section in template["sections"]]
206188
assert labels == ["动机", "改动思路", "具体改动", "对主干的风险", "我的整体评价"], template
207189
for section in template["sections"]:
@@ -220,7 +202,7 @@ def fake_run_gh_json(args: list[str], *, cwd: Path | None = None) -> object:
220202
section for section in template["sections"] if section["label"] == "具体改动"
221203
)
222204
assert "### 关键代码讲解" in concrete_change["agent_instruction"], concrete_change
223-
assert "2-5 个行为关键符号" in concrete_change["agent_instruction"], concrete_change
205+
assert "2-5 behavior-bearing exact-head symbols" in concrete_change["agent_instruction"], concrete_change
224206
assert "headRefOid" in first["evidence_commands"][0], first["evidence_commands"]
225207
assert "headRefOid" in first["evidence_commands"][-1], first["evidence_commands"]
226208
assert template["review_order"][0] == "docs/guides/newcomer-command-path.md", template
@@ -406,8 +388,10 @@ def fake_run_gh_json(args: list[str], *, cwd: Path | None = None) -> object:
406388
assert response_contract["queue_table_role"] == "preface_only", response_contract
407389
assert response_contract["required_packet_fields_to_preserve"] == [
408390
"agent_response_contract",
391+
"agent_response_contract.review_execution_contract",
409392
"result_completeness",
410393
"review_groups",
394+
"pull_requests[].review_plan",
411395
"pull_requests[].review_template",
412396
"pull_requests[].evidence_commands",
413397
], response_contract
@@ -420,24 +404,56 @@ def fake_run_gh_json(args: list[str], *, cwd: Path | None = None) -> object:
420404
], response_contract
421405
depth = response_contract["explanation_depth_contract"]
422406
assert depth["schema_version"] == "pr_review_explanation_depth_v0", depth
407+
assert depth["authority"] == "agent_response_contract.review_execution_contract", depth
423408
assert "may not know" in depth["reader_profile"], depth
424-
assert len(depth["evidence_layers"]) == 4, depth
425-
assert len(depth["necessity_questions"]) == 3, depth
426-
assert depth["runtime_walkthroughs"]["positive"], depth
427-
key_code = depth["key_code_explanation"]
428-
assert key_code["schema_version"] == "pr_review_key_code_explanation_v0", key_code
429-
assert key_code["required_for_code_changes"] is True, key_code
430-
assert key_code["subsection"] == "关键代码讲解", key_code
431-
assert len(key_code["per_symbol_fields"]) == 7, key_code
432-
assert "short exact-head excerpts" in key_code["source_form"], key_code
433-
assert key_code["docs_only_alternative"], key_code
434-
assert "authority, permission, or scope bypass" in depth["risk_scan"], depth
435-
assert "head SHA" in depth["freshness"], depth
436-
assert any("Do not stop at the queue/table summary" in item for item in response_contract["instructions"])
437-
assert any("open, closed, merged, today" in item for item in response_contract["instructions"])
438-
assert any("drops agent_response_contract" in item or "Do not pipe the JSON packet" in item for item in response_contract["instructions"])
439-
assert any("intended checked-out LoopX worktree" in item for item in response_contract["instructions"])
409+
execution = response_contract["review_execution_contract"]
410+
assert execution["schema_version"] == "pull_request_review_execution_contract_v1", execution
411+
requirements = {
412+
item["evidence_id"]: item for item in execution["evidence_requirements"]
413+
}
414+
assert set(requirements) == {
415+
"problem_context",
416+
"architecture_flow",
417+
"changed_line_classification",
418+
"symbol_map",
419+
"walkthroughs",
420+
"validation_matrix",
421+
"failure_analysis",
422+
"code_volume",
423+
}, requirements
424+
assert requirements["symbol_map"]["item_count"] == {"minimum": 2, "maximum": 5}
425+
assert "caller_evidence" in requirements["symbol_map"]["item_fields"]
426+
assert requirements["changed_line_classification"]["categories"] == [
427+
"production",
428+
"tests_or_fixtures",
429+
"docs",
430+
"generated",
431+
"mechanical_moves",
432+
]
433+
assert "negative_fields" in requirements["walkthroughs"]
434+
assert "regression_test" in requirements["failure_analysis"]["fields"]
435+
assert requirements["code_volume"]["verdict_values"] == [
436+
"necessary",
437+
"partly_avoidable",
438+
"not_yet_proven",
439+
]
440+
assert execution["completion_gate"]["metadata_only_verdict_allowed"] is False
441+
assert execution["completion_gate"]["stale_head_verdict_allowed"] is False
442+
assert execution["finding_contract"]["findings_first"] is True
443+
first_plan = first["review_plan"]
444+
assert first_plan["schema_version"] == "pull_request_review_plan_v1", first_plan
445+
assert first_plan["applicability"]["docs_only"] is True, first_plan
446+
assert first_plan["applicability"]["symbol_map_required"] is False, first_plan
447+
assert "symbol_map" not in first_plan["required_evidence_ids"], first_plan
448+
assert first_plan["result_template"]["target_exact_head"] == (
449+
"773@7730000000000000000000000000000000000000"
450+
), first_plan
440451
merged = next(item for item in payload["pull_requests"] if item["number"] == 770)
452+
merged_plan = merged["review_plan"]
453+
assert merged_plan["applicability"]["code_change"] is True, merged_plan
454+
assert merged_plan["applicability"]["symbol_map_required"] is True, merged_plan
455+
assert merged_plan["applicability"]["negative_walkthrough_required"] is True, merged_plan
456+
assert "symbol_map" in merged_plan["required_evidence_ids"], merged_plan
441457
merged_risk_hint = merged["metadata_risk_hint"]
442458
assert merged_risk_hint["level"] == "medium", merged_risk_hint
443459
merged_main_risk = merged["main_regression_analysis"]
@@ -508,7 +524,8 @@ def fake_run_gh_json(args: list[str], *, cwd: Path | None = None) -> object:
508524
assert "final answer contract: queue/table is only a preface" in markdown, markdown
509525
assert "## Agent Output Contract" in markdown, markdown
510526
assert "Do not stop at the queue/table summary" in markdown, markdown
511-
assert "explanation_depth_contract" in markdown, markdown
527+
assert "agent_response_contract.review_execution_contract" in markdown, markdown
528+
assert "review plan: exact_head=" in markdown, markdown
512529
assert "remote head SHA" in markdown, markdown
513530
assert "Required card headings: `动机`, `改动思路`, `具体改动`, `对主干的风险`, `我的整体评价`" in markdown, markdown
514531
assert "`关键代码讲解`" in markdown, markdown

examples/slash-command-catalog-smoke.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,17 @@ def main() -> int:
5050
assert pr_review["agent_contract"]["slash_prefix_dominates_intent"] is True, pr_review
5151
assert pr_review["agent_contract"]["stats_only_requires_explicit_opt_out"] is True, pr_review
5252
assert "agent_response_contract" in pr_review["agent_contract"]["authoritative_fields"], pr_review
53+
assert "agent_response_contract.review_execution_contract" in pr_review["agent_contract"]["authoritative_fields"], pr_review
5354
assert "agent_response_contract.explanation_depth_contract" in pr_review["agent_contract"]["authoritative_fields"], pr_review
5455
assert "review_groups.unmerged" in pr_review["agent_contract"]["authoritative_fields"], pr_review
5556
assert "review_groups.merged" in pr_review["agent_contract"]["authoritative_fields"], pr_review
5657
assert "agent_response_contract.required_final_sections" in pr_review["agent_contract"]["authoritative_fields"], pr_review
5758
assert pr_review["agent_contract"]["required_packet_fields_to_preserve"] == [
5859
"agent_response_contract",
60+
"agent_response_contract.review_execution_contract",
61+
"result_completeness",
5962
"review_groups",
63+
"pull_requests[].review_plan",
6064
"pull_requests[].review_template",
6165
"pull_requests[].evidence_commands",
6266
], pr_review

0 commit comments

Comments
 (0)