Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved

[MarketingChronoAi]

Problem

Reviewer templates require GitHub comments to include review_round: ${REVIEW_ROUND}, but dispatch_reviewers() does not pass REVIEW_ROUND into the template render environment. Rendered prompts can therefore contain the literal placeholder, causing malformed reviewer metadata such as blank, unset, or literal ${REVIEW_ROUND}.

Confirmed missing behavior

• REVIEW_ROUND is not supplied by dispatch_reviewers().
• Reviewer prompt rendering does not fail closed when REVIEW_ROUND remains unresolved.

Evidence

• /Users/chronoai/.claude/skills/consensus-loop/prompts/reviewer-architect.md requires review_round: ${REVIEW_ROUND}.
• /Users/chronoai/.claude/skills/consensus-loop/prompts/reviewer-quality.md requires review_round: ${REVIEW_ROUND}.
• /Users/chronoai/.claude/skills/consensus-loop/prompts/reviewer-tests.md requires review_round: ${REVIEW_ROUND}.
• /Users/chronoai/.claude/skills/consensus-loop/scripts/codex_refactor_loop/controller_actions.py: reviewer render env includes PR/head/output fields but not REVIEW_ROUND.

Suggested fix

• Add "REVIEW_ROUND": str(round_number) to reviewer prompt render env.
• Add a test that renders architect/tests/quality reviewer prompts and asserts there is no literal ${REVIEW_ROUND}.

Acceptance criteria

• Rendered reviewer prompts contain numeric review_round: <N>.
• No reviewer prompt contains ${REVIEW_ROUND} after rendering.
• Tests cover all reviewer roles.

Review status

This was independently reviewed in /Users/chronoai/Desktop/proposed-main-repo-issues-review-gate.md and classified high confidence.

⟦AI:AUTO-LOOP⟧
ISSUE_RAISED:review-gate-hardening:review-round-rendering

[ctkm-aelf]

🤖 Minimal solver proposes explicit REVIEW_ROUND binding

TL;DR

• What this is: Minimal-change solver output for issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43.
• Current state or conclusion: The issue is real; dispatch_reviewers() computes round_number but does not bind REVIEW_ROUND for reviewer prompt rendering.
• What the controller does next: Use the proposed one-field env addition plus existing dispatch reviewer test coverage as the implementation boundary.

---

Minimal design conclusion

REVIEW_ROUND is the prompt variable used by reviewer prompts to write GitHub-visible review metadata, and dispatch_reviewers() is the dispatcher that already owns the numeric round_number. The smallest viable fix is to pass "REVIEW_ROUND": str(round_number) in the same render environment that already carries PR_NUMBER, HEAD_SHA, and REVIEW_OUTPUT_PATH.

Recommended verification is to extend the existing dispatch reviewer test so all three reviewer roles prove they receive numeric round metadata. A second assertion in the existing stale redispatch round-2 test can cover the non-initial round path with almost no extra test surface.

📎 Full raw codex output (archival)

---

solver: minimal
issue: 43
cluster: issue-43
verdict: propose

Recommended framing

最小可行边界是在 dispatch_reviewers() 已经计算出 round_number 的同一处，把它作为 "REVIEW_ROUND": str(round_number) 传入 reviewer prompt render env，并在现有 dispatch reviewer 测试中断言三个角色都收到对应数字；这直接修复 reviewer GitHub 评论元数据模板中的未解析占位符，不需要新抽象、规则变更或 .refactor-loop/host.env 生产配置扩展。Reference-frame note: fail-closed template rendering / explicit context binding is the applicable frame; required template variables should be supplied by the dispatcher that owns the value.

Concrete plan

• Files:
  • /Users/chronoai/.claude/skills/consensus-loop/scripts/codex_refactor_loop/controller_actions.py: in dispatch_reviewers(), add "REVIEW_ROUND": str(round_number) to the env mapping passed to self.render_template(...).
  • /Users/chronoai/.claude/skills/consensus-loop/scripts/test_controller_actions.py: extend test_dispatch_reviewers_renders_three_role_prompts_with_pr_facts to assert every rendered role prompt env contains REVIEW_ROUND == "1"; optionally extend the existing stale redispatch round-2 test to assert the tests role receives "2".
• LOC delta: ~+3 / -0.
• Tests to add/modify: modify existing test_dispatch_reviewers_renders_three_role_prompts_with_pr_facts; optional one assertion in test_dispatch_reviewers_redispatch_uses_next_round_after_completed_stale_logs.
• Governance/ruleset change (if any): none.
• Migration path: no migration needed; future reviewer prompts receive the explicit review round at render time.

Risks

• This minimal fix relies on current render_prompt_text(...).safe_substitute(...) behavior; unrelated missing template variables can still remain unresolved unless broader prompt validation is added later.
• If a reviewer prompt is rendered outside dispatch_reviewers(), that separate path would still need its own explicit REVIEW_ROUND binding, but the verified issue evidence points to dispatch_reviewers() as the dispatch path.
• Adding only env-level assertions proves dispatcher binding, not the full prompt file text; a broader render-level regression test could catch template text drift but is larger than the smallest necessary change.

Escalation triggers (if any)

• none

Reasoning trace (internal — short, for meta-judge)

• Why this is the minimum: the three reviewer templates all require review_round: ${REVIEW_ROUND}, and dispatch_reviewers() already owns round_number; one env entry removes the unresolved placeholder for all roles.
• What I considered but rejected: adding a generic fail-closed unresolved-placeholder validator for all prompt rendering would be useful but broader than issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43 because render_prompt_text is shared by many prompt families and may currently rely on safe_substitute for optional placeholders.
• What I cannot decide alone: whether the broader renderer should eventually reject all unresolved required metadata fields; not needed for this minimal fix.

⟦AI:AUTO-LOOP⟧
SOLVER_DONE:minimal:propose:Add REVIEW_ROUND to reviewer render env and assert numeric rounds in existing dispatch tests

⟦AI:AUTO-LOOP⟧

[ctkm-aelf]

🤖 Structural solver proposes binding reviewer round metadata

TL;DR

• What this is: Structural design output for issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43, focused on unresolved REVIEW_ROUND in reviewer prompts.
• Current state or conclusion: Propose a bounded controller-render contract fix: bind REVIEW_ROUND from the computed round and test all three reviewer templates.
• What the controller does next: Compare this with the other solver outputs and select the implementation framing.

---

Structural framing

dispatch_reviewers() is the controller method that computes reviewer round numbers and renders reviewer prompts. It already uses the round in output paths and spawn intent IDs, so the structural fix is to make the render environment carry the same value as REVIEW_ROUND.

Option	Trade-off
Bind REVIEW_ROUND in dispatch_reviewers()	Smallest complete fix; one extra string per reviewer render and direct coverage of the broken contract.
Add a new reviewer render-env abstraction	Cleaner if more callers appear, but currently one production caller, so it adds structure without enough leverage.
Make all prompt rendering fail on unresolved placeholders	Broadly safer, but risky because the prompt corpus may intentionally include literal shell or env examples.

The proposed test shape is 2 focused checks: one controller dispatch test that captures the render environment and one prompt-rendering contract test that renders architect, tests, and quality reviewer templates and asserts review_round: 7 appears with no literal ${REVIEW_ROUND} left.

📎 Full raw codex output (archival)

---

solver: structural
issue: 43
cluster: issue-43
verdict: propose

PROJECT_RULES clause violated (quoted verbatim)

FI-006 Changes must be verifiable and evidence-based; failures, gaps, and out-of-scope promises must be exposed explicitly, and tests must not be disabled to force a pass.

Supporting prompt contract verified in the reviewer templates:

Your GitHub PR comment must include review_round: ${REVIEW_ROUND}, head_sha: ${HEAD_SHA}, the same REVIEW_DONE:${PR_NUMBER}:architect:<verdict> marker, and the final standalone AI sentinel.

The same contract appears for tests and quality reviewer roles.

Recommended framing

Make reviewer dispatch own a complete reviewer-render contract: every value that reviewer prompts require for GitHub-visible merge authority is bound by dispatch_reviewers() at the same point where the round number is computed. This removes the failure mode by construction because the prompt renderer receives REVIEW_ROUND from the controller-owned round calculation, and tests render all three reviewer templates to prove no ${REVIEW_ROUND} placeholder survives.

Reference-frame note: fail-fast contract rendering for generated operational prompts; generated metadata that gates workflow state should be explicitly bound and covered by template-contract tests.

Concrete plan

• New abstractions (if any): none. A named helper would have one caller today, so the clean structural boundary is the existing dispatch_reviewers() render-env construction plus a prompt-contract test.
• Files:
  • /Users/chronoai/.claude/skills/consensus-loop/scripts/codex_refactor_loop/controller_actions.py: add "REVIEW_ROUND": str(round_number) to the reviewer prompt render environment near the existing PR/head/output bindings.
  • /Users/chronoai/.claude/skills/consensus-loop/scripts/test_controller_actions.py: extend test_dispatch_reviewers_renders_three_role_prompts_with_pr_facts to assert the three captured render envs include REVIEW_ROUND == "1"; extend the existing source regression test to assert reviewer prompts include review_round: ${REVIEW_ROUND}.
  • /Users/chronoai/.claude/skills/consensus-loop/scripts/test_prompt_rendering.py: add one behavior test that renders reviewer-architect.md, reviewer-tests.md, and reviewer-quality.md with representative values including REVIEW_ROUND="7", then asserts each rendered prompt contains review_round: 7 and does not contain ${REVIEW_ROUND}.
• LOC delta: ~+28 / -0.
• Tests to add:
  • test_controller_actions.py::ControllerActionsTests::test_dispatch_reviewers_renders_three_role_prompts_with_pr_facts: behavior assertion that controller dispatch binds the computed round number into every reviewer render env.
  • test_prompt_rendering.py::PromptRenderingTests::test_reviewer_prompts_resolve_review_round_for_all_roles: behavior assertion that all reviewer templates render numeric GitHub-visible review_round metadata and leave no unresolved ${REVIEW_ROUND} token.
• Schema/protocol changes (if any): none; no app-server, CLI, config, or persisted data format changes.
• Governance/ruleset changes (if any): none. The existing AGENTS FI-006 verification requirement and the reviewer prompt contract already cover the violation.
• Runtime cost: one extra string entry in the per-reviewer render environment, three allocations per full reviewer dispatch; no extra GitHub calls, no extra actor hop, and effectively 0 ms added latency.
• If the structural answer is issue decomposition, use the controller-private IssueDecompositionPlan boundary and child body artifacts; forbid solver/judge/worker GitHub lifecycle, public issue factories, and any wakeup_plan.py decompose action/projection.
• Do not author helper-owned decomposition tracking comments or child fingerprint lines; the checked-in refactor(history-overlay): split into modular functions & add tests (fixes #402) openai/codex#403 helper is the only writer for the parent tracking grammar and per-child idempotency fingerprints.

Risks

• The plan leaves Template.safe_substitute behavior unchanged, so unresolved placeholders in unrelated prompts can still survive if they are not covered by a role-specific contract test.
• A global unresolved-placeholder failure mode would be broader and cleaner for all prompts, but it risks breaking intentionally literal shell/env examples across the prompt corpus. Keep this issue bounded to reviewer prompt merge-authority metadata.

Escalation triggers (if any)

• none

Reasoning trace (internal — for meta-judge)

• Why this structure beats a documented exception: the failure is not a policy ambiguity; the controller computes round_number and already names it in prompt paths, output paths, and spawn intent IDs. Binding the same value into the render contract makes the GitHub-visible metadata and controller state share one source.
• Why I picked this abstraction over alternatives: a new ReviewerRenderEnv helper is not justified yet because there is one production caller. The existing primitive is the controller-owned render environment passed to render_template, and the structural fix is to make that boundary complete and tested.
• What I cannot decide alone: whether a future larger change should make render_prompt_text fail on unresolved placeholders globally. That is outside this issue because the current prompt corpus may intentionally include literal environment-variable examples.

⟦AI:AUTO-LOOP⟧
SOLVER_DONE:structural:propose:bind REVIEW_ROUND in reviewer dispatch render env and add all-role prompt contract coverage

⟦AI:AUTO-LOOP⟧

[ctkm-aelf]

🤖 Delete solver recommends removing the redundant review_round requirement

TL;DR

• What this is: delete-framing solver output for issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43.
• Current state or conclusion: I recommend deleting the GitHub-visible review_round requirement instead of adding REVIEW_ROUND to reviewer prompt rendering.
• What the controller does next: let minimal/structural consensus compare this against a direct fix before implementation.

---

Delete-framing result

The issue identifies a real unresolved template variable, but the narrowest deletion-first fix is to remove that variable from the reviewer comment contract. The review gate already has other evidence that identifies valid reviewer comments: head_sha binds the review to the PR head, REVIEW_DONE names the role/verdict, the final AI sentinel marks an AI-authored completed comment, and GitHub comment ordering handles later comments for the same role.

review_round is the round number metadata currently requested in reviewer comments. Local round numbers still matter for prompt/run/log filenames and redispatch identity, but they do not need to be exported into GitHub comments as merge authority. Existing tests already cover blank round comments completing the gate when the same-head evidence is present.

Option	Trade-off
Delete required review_round from reviewer prompt contract	Removes the unresolved placeholder and avoids duplicating round identity in GitHub-visible metadata.
Add REVIEW_ROUND to dispatch_reviewers()	Small direct fix, but preserves a redundant public comment field.
Make unresolved placeholders fail closed	Useful hardening, but still leaves the unnecessary field in the contract.

📎 Full raw codex output (archival)

---
solver: delete
issue: 43
cluster: issue-43
verdict: propose
---

## Classification
(c) Replaceable with existing

## Recommended action
删除 reviewer GitHub comment 合约里的 `review_round` 强制字段，而不是给 `dispatch_reviewers()` 增加 `REVIEW_ROUND` 模板变量：当前系统已经用 `.refactor-loop/prompts|runs|logs/review-pr<N>-<role>-r<R>.*` 文件名、`head_sha`、`REVIEW_DONE` marker、final AI sentinel、GitHub comment ordering metadata 来判定 reviewer 证据的新旧和同头有效性；`review_round` 在 GitHub comment 里是重复协议字段，且现有测试已经接受空 round 并用 GitHub ordering 完成 gate。保留解析旧评论中的 round 作为兼容输入，但删除三个 reviewer prompt 和 SKILL review-gate 文档中“必须包含 review_round”的要求。

## Concrete plan (if propose)
- Files to delete: none
- Caller migrations:
  - `/Users/chronoai/.claude/skills/consensus-loop/prompts/reviewer-architect.md` comment contract: remove `review_round: ${REVIEW_ROUND}` from the required GitHub PR comment fields; keep `head_sha: ${HEAD_SHA}`, the role marker, and final AI sentinel.
  - `/Users/chronoai/.claude/skills/consensus-loop/prompts/reviewer-tests.md` comment contract: same deletion.
  - `/Users/chronoai/.claude/skills/consensus-loop/prompts/reviewer-quality.md` comment contract: same deletion.
  - `/Users/chronoai/.claude/skills/consensus-loop/SKILL.md` review-gate consensus rules: change "posts a GitHub-visible PR comment with final AI sentinel, `review_round`, `head_sha`, and the same marker" to require only final AI sentinel, `head_sha`, and the same marker.
  - Keep `codex_refactor_loop.review_gate_selection.extract_review_round()` and its fallback-to-round-1 parsing for backward compatibility with existing comments and local artifacts.
  - Keep `ControllerActions._next_review_round()` and round-numbered prompt/run/log filenames; these remain the internal redispatch identity and are not the GitHub-visible merge authority.
- Tests to delete: none
- LOC delta: -4 to -8, depending on whether the four contract sentences are edited in place or split for readability
- Philosophy/CLAUDE.md/SPEC/Tier changes (if any): none

## Reverse-evidence (why this is safe to delete)
- No public API breaks in the repository checkout: `rg` found `review_round` usage in the consensus-loop skill prompts, tests, and helper scripts, not in a stable app-server or CLI API surface.
- No external repo dependency was touched; this design only removes a redundant generated-comment requirement from the consensus-loop skill contract.
- The review gate already treats GitHub-visible, same-head comments as merge/fix authority and local `.refactor-loop/runs` / `.refactor-loop/logs` files as diagnostics. That boundary is documented in `SKILL.md` and does not require GitHub-visible round metadata.
- `parse_github_review_evidence()` already parses missing or blank `review_round` as round 1, and selector logic uses GitHub ordering metadata when comments have ordering fields.
- Existing tests cover the deletion direction: `test_github_comment_empty_review_round_is_valid_and_gate_completes` and `test_review_gate_completion_uses_recency_resolved_github_comments_with_empty_rounds` show blank round comments can complete the gate when `head_sha`, marker, sentinel, and ordering are present.
- `dispatch_reviewers()` already computes `round_number` for filenames, pending checks, and spawn intent IDs. That internal round identity remains load-bearing, but it does not need to be exported into reviewer prompt text.
- Reference-frame note: protocol minimalism / single source of truth. Do not duplicate ordering identity across local filenames, controller intent IDs, and GitHub comment metadata when the merge authority is already bounded by `head_sha`, marker, sentinel, and comment ordering.

## Risks
- If a downstream monitor outside this checkout requires non-empty GitHub-visible `review_round`, deleting the prompt requirement could break that monitor. The local evidence points the other way: current parser and tests explicitly accept blank/missing rounds.
- If maintainers want round numbers as human-facing audit breadcrumbs rather than machine authority, the delete plan should be narrowed to "optional `review_round`" instead of fully removing it from the required comment contract.
- The consensus-loop skill directory is not a git repository in this environment, so local git blame/history for `controller_actions.py` could not be inspected there.

## Escalation triggers (if any)
- none

## Reasoning trace (internal — for meta-judge)
- Why I claim this can be deleted: the broken requirement is not the internal round mechanism; it is only a redundant GitHub-visible metadata field in reviewer prompt text. The system already has a working same-head evidence model without it.
- What I checked to verify safety: searched `REVIEW_ROUND`, `review_round`, and `dispatch_reviewers`; read the three reviewer prompts; read `dispatch_reviewers()`, `_next_review_round()`, `_review_round_is_pending()`, `parse_github_review_evidence()`, `select_latest_live_head_review_evidence()`, `highest_complete_required_review_round()`, and tests for empty-round GitHub comments.
- What I cannot decide alone: whether any unlisted external automation outside this checkout treats `review_round` as a required human-visible audit field.

⟦AI:AUTO-LOOP⟧
SOLVER_DONE:delete:propose:delete redundant GitHub-visible review_round requirement and keep existing head_sha/marker/sentinel/order based review evidence path

⟦AI:AUTO-LOOP⟧

[ctkm-aelf]

🤖 Meta-judge sends issue #43 to convergence round 2

TL;DR

• What this is: Round-1 design-consensus meta-judge result for issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43.
• Current state or conclusion: No implementation consensus yet because two solvers keep and bind review_round, while the delete solver removes or optionalizes it.
• What the controller does next: Re-run solver round 2 with the narrowed question below.

---

Convergence decision

The split is concrete and implementation-bearing:

Option	Trade-off
Keep review_round required and bind REVIEW_ROUND in dispatch_reviewers()	Smallest direct fix for the current prompt contract; leaves the GitHub-visible field as required metadata.
Delete or optionalize review_round from the reviewer comment contract	Simplifies the public comment protocol; requires proof that no parser, selector, monitor, prompt, or SKILL rule depends on a non-empty field.

Round 2 should answer one question: should review_round remain required reviewer-comment metadata and be supplied as REVIEW_ROUND, or should it be deleted/optionalized because head_sha, the reviewer marker, the final AI sentinel, and GitHub comment ordering are sufficient authority?

Each solver should explicitly choose one of those two paths, name the exact files to change, and give the smallest verification command set. This is a convergence result, so no implementation dispatch is authorized from round 1.

---

📎 Full raw codex output (archival)

---

issue: 43
cluster: issue-43
convergence_round: 1
solver_verdicts:
minimal: propose
structural: propose
delete: propose
decision: converge
action_mode: plan
canonical_key: #43
next_action: plan

Decision

本轮没有达到可派发实现的设计共识：minimal 和 structural 都主张保留 reviewer GitHub 评论中的 review_round 合约并在 dispatch_reviewers() 中绑定 REVIEW_ROUND，但 delete 主张删除该 GitHub 可见字段并依赖 head_sha、REVIEW_DONE marker、final AI sentinel 和 GitHub comment ordering；这两个方案改变的是同一协议字段的相反方向，不能按 3/3 propose consensus 自动实施。Reference-frame note: generated operational prompt contracts should have a single authority for required metadata, and a field must either be required and always bound, or explicitly optional/deleted with evidence that no consumer depends on it.

If converge

• Convergence question (specific): Should review_round remain required reviewer-comment metadata and be supplied as REVIEW_ROUND, or should it be deleted/optionalized from the reviewer GitHub comment contract because existing review-gate evidence already has sufficient ordering authority?
• What each solver should address explicitly:
  • State one of exactly two choices: keep-required-and-bind or delete/optionalize-from-contract.
  • If keeping it required, name the exact binding site, all prompt/template tests needed for architect/tests/quality, and whether any unresolved-placeholder fail-closed behavior is in or out of scope.
  • If deleting or optionalizing it, identify every parser, selector, monitor, prompt, and SKILL rule that consumes or promises review_round, and explain whether existing blank-round tests prove machine safety or only backward compatibility.
  • For either choice, list the exact files to change and the smallest verification command set.
• Round number this fires: 2; marker payload uses source round 1
• Progress since prior round: Round 1 established a concrete split: two solvers agree on adding "REVIEW_ROUND": str(round_number) in dispatch_reviewers(), while the delete solver proposes removing the required reviewer-comment field from the prompt/SKILL contract.
• Human-intervention threshold: not reached

Round audit trail (links to local artifacts)

• solver-minimal: .refactor-loop/logs/phase9-issue43-r1-minimal.log
• solver-structural: .refactor-loop/logs/phase9-issue43-r1-structural.log
• solver-delete: .refactor-loop/logs/phase9-issue43-r1-delete.log
• work-unit-producer: manual-issue (prompt-only provenance)
• work-unit-source-ref: gh-issue-43
• action_mode: plan
• canonical_key: Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43
• duplicate_search_summary: not searched beyond the router-scoped issue snapshot and the three listed solver artifacts; this judge was instructed not to inspect sibling artifacts or unlisted solver outputs
• branch: none
• PR: none
• commit: none
• unique_diff_status: not-applicable
• tests_verification: not run; design arbitration only and no source implementation was dispatched
• resolution: converge
• next_action: plan

⟦AI:AUTO-LOOP⟧
META_JUDGE_DONE:converge:round-1:Decide whether review_round remains required and gets REVIEW_ROUND binding, or is deleted/optionalized from the reviewer comment contract

⟦AI:AUTO-LOOP⟧

[ctkm-aelf]

🤖 Minimal solver keeps review_round required and binds it

TL;DR

• What this is: Round-2 minimal-change solver output for issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43.
• Current state or conclusion: Choose keep-required-and-bind; review_round remains required reviewer-comment metadata and dispatch_reviewers() should provide REVIEW_ROUND from its computed round number.
• What the controller does next: Compare this with the other round-2 solver outputs and, if consensus holds, implement the one-field render-env fix plus focused dispatch reviewer tests.

---

Minimal convergence answer

review_round should remain required for this issue's minimal path. The prompts and SKILL.md still define it as GitHub-visible required metadata, and dispatch_reviewers() already calculates the exact round_number used for prompt/run/log names and spawn intent IDs. The smallest complete fix is to bind the same value as REVIEW_ROUND during reviewer prompt rendering.

Option	Trade-off
Keep review_round required and bind REVIEW_ROUND	One env key plus focused tests; preserves the documented reviewer-comment contract.
Delete or optionalize review_round	Larger contract change across prompts, SKILL.md, and review-gate wording; blank-round parser tests prove compatibility, not that the field should be removed.

Implementation boundary: add "REVIEW_ROUND": str(round_number) to the env mapping in dispatch_reviewers(), then extend the existing controller-action tests so all three roles get "1" and stale redispatch gets "2". Broad unresolved-placeholder fail-closed behavior is intentionally out of scope for the minimal fix because the shared renderer has wider prompt-family behavior.

📎 Full raw codex output (archival)

---

solver: minimal
issue: 43
cluster: issue-43
verdict: propose

Recommended framing

保留 review_round 作为 reviewer GitHub 评论里的必填元数据，并在 dispatch_reviewers() 渲染 reviewer prompt 的同一处绑定 "REVIEW_ROUND": str(round_number)；这是最小可行边界，因为三个 reviewer prompt 和 SKILL.md 的 consensus rules 仍把 review_round 描述为 required same-round metadata，而当前 dispatch path 已经计算并使用同一个 round_number 来命名 prompt/run/log 和 spawn intent。Reference-frame note: generated operational prompt contracts should have one explicit producer for every required metadata field; compatibility parsing of missing legacy fields is not the same as deleting a required producer-side contract.

Concrete plan

• Files:
  • /Users/chronoai/.claude/skills/consensus-loop/scripts/codex_refactor_loop/controller_actions.py: in dispatch_reviewers(), add "REVIEW_ROUND": str(round_number) to the env mapping passed to self.render_template(...) beside PR_NUMBER, HEAD_SHA, and REVIEW_OUTPUT_PATH.
  • /Users/chronoai/.claude/skills/consensus-loop/scripts/test_controller_actions.py: extend test_dispatch_reviewers_renders_three_role_prompts_with_pr_facts to assert all three captured render envs include REVIEW_ROUND == "1", and extend test_dispatch_reviewers_redispatch_uses_next_round_after_completed_stale_logs to assert the redispatched tests role receives REVIEW_ROUND == "2".
  • Optional only if implementation wants full prompt-text verification rather than env-level verification: add or extend a prompt rendering test that renders reviewer-architect.md, reviewer-tests.md, and reviewer-quality.md with REVIEW_ROUND="7" and asserts review_round: 7 appears and ${REVIEW_ROUND} does not.
• LOC delta: required path ~+3 / -0; with optional prompt-text test ~+25 / -0.
• Tests to add/modify:
  • Required: modify /Users/chronoai/.claude/skills/consensus-loop/scripts/test_controller_actions.py in the two existing dispatch reviewer tests named above.
  • Smallest verification command set: from /Users/chronoai/.claude/skills/consensus-loop/scripts, run python3 -m pytest test_controller_actions.py -k 'dispatch_reviewers_renders_three_role_prompts_with_pr_facts or dispatch_reviewers_redispatch_uses_next_round_after_completed_stale_logs'.
  • If the optional prompt-text test is added, also run its target test file with the focused -k REVIEW_ROUND selector.
• Governance/ruleset change (if any): none. The existing rule remains coherent: reviewer comments are required to include final AI sentinel, review_round, head_sha, and the review marker; dispatch simply needs to provide the missing required value.
• Migration path: no migration needed. Existing blank or missing round comments can remain accepted by parser fallback for backward compatibility, while newly rendered reviewer prompts receive a numeric producer-side round.

Risks

• This keeps review_round GitHub-visible even though head_sha, marker, sentinel, and GitHub ordering also carry review authority; the trade-off is one redundant-looking field in exchange for preserving the current documented contract.
• Env-level assertions prove the dispatcher binds REVIEW_ROUND; they do not independently prove the prompt text uses it unless the optional prompt-text test is also added.
• Broad unresolved-placeholder fail-closed behavior remains out of scope because render_template() is shared across prompt families and currently uses tolerant substitution patterns.

Escalation triggers (if any)

• none

Reasoning trace (internal — short, for meta-judge)

• Why this is the minimum: the violation is one missing render-env key, and dispatch_reviewers() already owns the exact numeric round_number; adding one env binding fixes all three reviewer roles through the existing loop.
• What I considered but rejected: deleting or optionalizing review_round would require coordinated edits to the three reviewer prompts, SKILL.md consensus rules, and the review-gate invalid/incomplete wording, plus proof that every parser/selector/monitor treats missing round as intentional rather than legacy compatibility.
• What I cannot decide alone: whether a later cleanup should remove review_round from the public reviewer comment protocol entirely; that is a broader contract change and not the minimal fix for issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43.

⟦AI:AUTO-LOOP⟧
SOLVER_DONE:minimal:propose:keep review_round required and bind REVIEW_ROUND from dispatch_reviewers round_number

⟦AI:AUTO-LOOP⟧

[ctkm-aelf]

🤖 Structural solver keeps review_round required and binds REVIEW_ROUND

TL;DR

• What this is: Round-2 structural solver output for issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43.
• Current state or conclusion: Keep review_round as required reviewer-comment metadata and bind REVIEW_ROUND from dispatch_reviewers().
• What the controller does next: Use this with the other round-2 solver outputs to decide the implementation consensus.

---

Structural convergence answer

I choose keep-required-and-bind.

review_round is currently part of the declared reviewer comment contract in the skill runbook and in all three reviewer prompts. The review-gate parser accepts missing or blank rounds as compatibility behavior, but that does not prove the generated prompt contract should be weakened. The clean boundary is the existing reviewer dispatch method: dispatch_reviewers() already computes round_number and uses it in prompt, run, log, and spawn-intent identity, so it should also pass that value to the template as REVIEW_ROUND.

Option	Trade-off
Keep review_round required and bind REVIEW_ROUND	Smallest structural fix; preserves the current GitHub-visible contract and makes generated prompts complete by construction.
Delete or optionalize review_round	Possible later protocol cleanup, but it must update the skill runbook, all reviewer prompts, parser expectations, and compatibility tests deliberately.

Planned implementation boundary: add one env entry in controller_actions.py, extend the existing dispatch tests for round 1 and redispatch round 2, and add one prompt-rendering test that renders architect, tests, and quality prompts with REVIEW_ROUND: "7" and checks no ${REVIEW_ROUND} remains.

📎 Full raw codex output (archival)

---

solver: structural
issue: 43
cluster: issue-43
verdict: propose

PROJECT_RULES clause violated (quoted verbatim)

FI-006 Changes must be verifiable and evidence-based; failures, gaps, and out-of-scope promises must be exposed explicitly, and tests must not be disabled to force a pass.

Recommended framing

Keep review_round required and bind it as REVIEW_ROUND in dispatch_reviewers(). The structural boundary is the reviewer prompt-render contract: dispatch_reviewers() already computes the per-role round_number, already uses that value in prompt paths, run paths, log paths, and spawn intent IDs, and is therefore the single owner that can provide the same value to the reviewer template. Deleting or optionalizing review_round would be a protocol change to the GitHub-visible reviewer comment contract in SKILL.md and three reviewer prompts; the current parser's blank-round fallback is compatibility behavior, not evidence that the declared contract should become weaker. Reference-frame note: contract rendering for operational workflow metadata; required generated fields should have one owner and one explicit binding point.

Concrete plan

• New abstractions (if any): none. A helper or data class would have one production caller today, and the existing dispatch_reviewers() render-env construction is the correct local contract boundary.
• Files:
  • /Users/chronoai/.claude/skills/consensus-loop/scripts/codex_refactor_loop/controller_actions.py: in dispatch_reviewers(), add "REVIEW_ROUND": str(round_number) to the env mapping passed to self.render_template(...), next to HEAD_SHA and REVIEW_OUTPUT_PATH.
  • /Users/chronoai/.claude/skills/consensus-loop/scripts/test_controller_actions.py: extend test_dispatch_reviewers_renders_three_role_prompts_with_pr_facts to assert all three captured render envs include REVIEW_ROUND == "1"; extend test_dispatch_reviewers_redispatch_uses_next_round_after_completed_stale_logs to assert the redispatched tests role receives REVIEW_ROUND == "2".
  • /Users/chronoai/.claude/skills/consensus-loop/scripts/test_prompt_rendering.py: add one behavior test that renders reviewer-architect.md, reviewer-tests.md, and reviewer-quality.md through render_prompt_text(...) with representative values including REVIEW_ROUND: "7", then asserts each rendered prompt contains review_round: 7 and does not contain ${REVIEW_ROUND}.
• LOC delta: ~+18 / -0.
• Tests to add:
  • 2 controller assertions in test_controller_actions.py: dispatcher-owned render env contains the numeric initial and redispatch review rounds.
  • 1 prompt-render behavior test in test_prompt_rendering.py: all three concrete reviewer templates consume REVIEW_ROUND and leave no unresolved placeholder.
• Schema/protocol changes (if any): none. This preserves the current GitHub-visible reviewer comment protocol instead of changing it.
• Governance/ruleset changes (if any): none. SKILL.md already says reviewer comments include review_round, and this plan makes the implementation satisfy that rule.
• Runtime cost: 1 additional small string allocation per rendered reviewer prompt (str(round_number)), normally 3 allocations per review dispatch; no extra subprocesses, GitHub calls, latency hops, disk reads, or actor handoffs.
• Verification command set:
  • cd /Users/chronoai/.claude/skills/consensus-loop/scripts && python3 -m unittest test_controller_actions.ControllerActionsTests.test_dispatch_reviewers_renders_three_role_prompts_with_pr_facts test_controller_actions.ControllerActionsTests.test_dispatch_reviewers_redispatch_uses_next_round_after_completed_stale_logs test_prompt_rendering.PromptRenderingTests.test_reviewer_prompts_render_numeric_review_round

Risks

• This intentionally does not add global fail-closed unresolved-placeholder validation. render_prompt_text(...) currently uses Template.safe_substitute(...), and changing that globally could break prompt text that intentionally carries literal shell, environment, or example placeholders.
• It preserves review_round as GitHub-visible metadata even though head_sha, REVIEW_DONE, the final AI sentinel, and GitHub comment ordering can make blank-round comments machine-acceptable. That is a deliberate distinction: parser compatibility stays permissive, while generated reviewer comments remain complete by contract.
• The new prompt-render test adds direct coverage for three concrete prompt files. If more reviewer prompt roles are added later, they should either be included in the same role list or covered by the existing prompt-contract tests.

Escalation triggers (if any)

• none

Reasoning trace (internal — for meta-judge)

• Why this structure beats a documented exception: The generated prompt contract already says review_round is required; binding the controller-owned value removes the unresolved-placeholder failure without weakening the public reviewer comment rule.
• Why I picked this abstraction over alternatives: There is exactly one production render owner for reviewer dispatch, so a new render-env abstraction would add indirection without multiple callers. A global template validator is valuable hardening but broader than issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43 because safe_substitute is shared across prompt families.
• What I cannot decide alone: Whether the long-term review-gate protocol should delete review_round from GitHub comments entirely. That would require an explicit protocol/ruleset change across SKILL.md, all reviewer prompts, parser expectations, and compatibility tests; it is not the smallest structural fix for this confirmed unresolved placeholder.

⟦AI:AUTO-LOOP⟧
SOLVER_DONE:structural:propose:keep review_round required and bind REVIEW_ROUND from dispatch_reviewers round_number with controller and prompt-render tests

⟦AI:AUTO-LOOP⟧

[ctkm-aelf]

🤖 Delete solver abstains on removing review_round

TL;DR

• What this is: Round-2 delete-framing solver output for issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43.
• Current state or conclusion: I do not recommend deleting or optionalizing review_round; it is still part of the review-gate evidence contract.
• What the controller does next: Prefer the keep-required-and-bind path: add REVIEW_ROUND to the reviewer render environment and test all three reviewer roles.

---

Round-2 delete conclusion

I checked the reviewer prompt contract, the dispatcher binding site, the parser/selector, wakeup planning, and review-gate tests. The blank or missing review_round path is compatibility fallback to round 1, not proof that the field is removable.

Option	Trade-off
Keep review_round required and bind REVIEW_ROUND	Matches the current SKILL contract and round-aware selector with a one-field dispatcher fix.
Delete or optionalize review_round	Would require rewriting the review-gate contract and tests because same-round evidence is currently documented and parsed.

The deletion-first answer is therefore abstain: do not delete the field in this issue. The smallest implementation remains adding "REVIEW_ROUND": str(round_number) in dispatch_reviewers(), then asserting architect/tests/quality prompts no longer render the literal placeholder.

📎 Full raw codex output (archival)

---
solver: delete
issue: 43
cluster: issue-43
verdict: abstain
---

## Classification
(e) Genuinely needed and right-sized, with no current deletion/collapse justified

## Recommended action
不建议删除或可选化 reviewer GitHub comment 合约里的 `review_round`。本轮收敛问题不是"round 能否从文件名推导",而是 GitHub-visible reviewer evidence 是否应携带同一轮次身份；当前 `SKILL.md` 明确要求 required roles 在 same review round 完成,`review_gate_selection.py` 解析 `review_round` 并用 `round_number` 选择最新 live-head evidence,`wakeup_plan.py` 同样用 round-numbered local reviewer artifacts/logs 计算完整轮次。空 round 被接受只是旧评论兼容 fallback-to-1,不能证明删除字段安全。delete solver 因此 abstain: 最小正确修复应保留 required metadata,并由 `dispatch_reviewers()` 绑定 `"REVIEW_ROUND": str(round_number)`。

## Concrete plan (if propose)
- Files to delete: none
- Caller migrations: none
- Tests to delete: none
- LOC delta: 0
- Philosophy/CLAUDE.md/SPEC/Tier changes (if any): none

## Reverse-evidence (why this is safe to delete)
- Deletion is not safe under the current contract: `/Users/chronoai/.claude/skills/consensus-loop/SKILL.md` says each reviewer comment includes final AI sentinel, `review_round`, `head_sha`, and the marker, and the consensus table requires every required reviewer comment for the same review round.
- Parser/selector consumption exists: `/Users/chronoai/.claude/skills/consensus-loop/scripts/codex_refactor_loop/review_gate_selection.py` defines `REVIEW_ROUND_RE`, `extract_review_round()`, defaults missing round to `1` for compatibility, and stores the parsed value in `ParsedGithubReviewEvidence.round_number`.
- Selection logic is round-aware: `select_latest_live_head_review_evidence()` chooses by ordering metadata when present and otherwise by highest `round_number`; complete reviewer evidence reports `complete_round`.
- Local review round identity remains load-bearing: `/Users/chronoai/.claude/skills/consensus-loop/scripts/codex_refactor_loop/wakeup_plan.py` computes `highest_complete_required_review_round()` from `review-pr<N>-<role>-r<R>` artifacts/logs and passes round-bearing evidence into the same selector.
- Existing blank-round tests prove backward compatibility only: tests write `review_round: ` and expect fallback handling for older/malformed comments, while other tests assert higher same-head rounds supersede lower complete rounds and incomplete higher rounds do not mask a complete lower reject.
- No public API deletion is justified: the observed bug is an unresolved template variable in reviewer prompts, and the dispatcher already owns `round_number`, so one explicit render-env binding resolves the inconsistency without protocol deletion.
- External repo dependency check: not performed beyond the local skill checkout and `/Users/chronoai/Desktop/codex` because this solver is not authorized to mutate or inspect external repositories; absence of external proof is another reason to abstain from deleting a GitHub-visible protocol field.

## Risks
- If maintainers intentionally want to simplify the public reviewer protocol, they must first update `SKILL.md`, the review-gate truth table, parser semantics, and tests so missing `review_round` is explicitly optional rather than compatibility fallback.
- Keeping `review_round` required still leaves broader unresolved-template hardening out of scope; this issue can be fixed by binding `REVIEW_ROUND`, but other placeholders may still depend on `safe_substitute` behavior.
- If a separate reviewer rendering path exists outside `dispatch_reviewers()`, it must also bind `REVIEW_ROUND`; the verified evidence for issue #43 points to `dispatch_reviewers()` as the broken path.

## Escalation triggers (if any)
- none

## Reasoning trace (internal — for meta-judge)
- Why I claim this cannot be deleted: `review_round` is part of the documented GitHub-visible review-gate contract and feeds a round-aware parser/selector; removing it would change merge/fix authority semantics rather than delete dead code.
- What I checked to verify safety: `rg` over `/Users/chronoai/.claude/skills/consensus-loop` and `/Users/chronoai/Desktop/codex`; `dispatch_reviewers()` render env; reviewer prompt contracts for architect/tests/quality; `review_gate_selection.py`; `wakeup_plan.py`; review-gate tests that cover blank-round fallback and higher/lower round selection.
- What I cannot decide alone: whether maintainers want a future protocol simplification that makes `review_round` optional by design; that is not the issue #43 acceptance criterion and would require a broader contract rewrite.
- Reference-frame note: operational protocol design favors a single explicit authority for required metadata; a required field should be bound by the dispatcher that computes it, while compatibility fallbacks should not be mistaken for deletion evidence.

⟦AI:AUTO-LOOP⟧
SOLVER_DONE:delete:abstain:review_round is load-bearing reviewer evidence metadata; keep it required and bind REVIEW_ROUND in dispatch_reviewers

⟦AI:AUTO-LOOP⟧

[ctkm-aelf]

🤖 Meta-judge reaches consensus to bind REVIEW_ROUND

TL;DR

• What this is: Round-2 design-consensus meta-judge result for issue Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43.
• Current state or conclusion: Consensus is reached to keep review_round required and bind REVIEW_ROUND from dispatch_reviewers().
• What the controller does next: Dispatch implementation for the agreed dispatcher and focused test scope.

---

Consensus Decision

The round-2 split resolved. Minimal and structural both propose the same implementation-bearing path: keep review_round as required reviewer-comment metadata and add "REVIEW_ROUND": str(round_number) where dispatch_reviewers() renders reviewer prompts. The delete solver abstains after finding deletion is not safe under the current review-gate contract, parser, selector, and wakeup planning behavior.

Option	Meta-judge result	Trade-off
Keep review_round required and bind REVIEW_ROUND	Chosen consensus	Small dispatcher fix, preserves current GitHub-visible reviewer contract, and adds focused coverage for all reviewer roles.
Delete or optionalize review_round	Rejected for this issue	Could simplify the protocol later, but would require a deliberate contract rewrite across prompts, skill rules, parser semantics, and tests.

Implementation is authorized only for the agreed scope: controller_actions.py, test_controller_actions.py, and test_prompt_rendering.py under the consensus-loop skill scripts. The expected behavior is that reviewer prompts render numeric review_round: <N> metadata and never leave ${REVIEW_ROUND} unresolved for architect, tests, or quality reviewers.

📎 Full raw codex output (archival)

---

issue: 43
cluster: issue-43
convergence_round: 2
solver_verdicts:
minimal: propose
structural: propose
delete: abstain
decision: consensus
action_mode: plan
canonical_key: #43
next_action: implement

Decision

达成实现共识：minimal 和 structural 都选择保留 review_round 作为 reviewer GitHub comment 的必填元数据，并在 dispatch_reviewers() 中用已计算的 round_number 绑定 REVIEW_ROUND；delete solver 在第二轮明确 abstain，认为删除或可选化该字段不安全，因此符合 manual-issue / gh-issue-43 的 Path A compatible-neutral 例外。Reference-frame note: operational prompt contract design; required workflow metadata should have a single explicit producer, while parser compatibility fallbacks must not be treated as permission to weaken the producer-side contract.

If consensus

• Chosen framing: structural
• Implement plan (structured fields read by wakeup-plan from this judge artifact only, not from solver artifacts or prompt-body free text):
  • scope_paths:
    • /Users/chronoai/.claude/skills/consensus-loop/scripts/codex_refactor_loop/controller_actions.py
    • /Users/chronoai/.claude/skills/consensus-loop/scripts/test_controller_actions.py
    • /Users/chronoai/.claude/skills/consensus-loop/scripts/test_prompt_rendering.py
  • old_pattern: Reviewer prompt templates require review_round: ${REVIEW_ROUND}, but the reviewer dispatch render environment omits REVIEW_ROUND, allowing literal or blank unresolved metadata in generated reviewer prompts.
  • new_principle: dispatch_reviewers() owns the computed reviewer round_number and must bind it as "REVIEW_ROUND": str(round_number) for reviewer prompt rendering; focused tests must prove initial dispatch, redispatch, and all three reviewer prompt templates render numeric review rounds without ${REVIEW_ROUND} left behind.
  • verification_hints: cd /Users/chronoai/.claude/skills/consensus-loop/scripts && python3 -m unittest test_controller_actions.ControllerActionsTests.test_dispatch_reviewers_renders_three_role_prompts_with_pr_facts test_controller_actions.ControllerActionsTests.test_dispatch_reviewers_redispatch_uses_next_round_after_completed_stale_logs test_prompt_rendering.PromptRenderingTests.test_reviewer_prompts_render_numeric_review_round
• Philosophy/CLAUDE.md/SPEC/Tier changes included: none
• Implementation owner: dispatch implement codex with cluster_id=issue-43, design_decision_path=.refactor-loop/runs/phase9-issue43-r2-judge.md
• Add crnd:triage:resume-requested label to issue 43

Round audit trail (links to local artifacts)

• solver-minimal: .refactor-loop/logs/phase9-issue43-r2-minimal.log
• solver-structural: .refactor-loop/logs/phase9-issue43-r2-structural.log
• solver-delete: .refactor-loop/logs/phase9-issue43-r2-delete.log
• work-unit-producer: manual-issue (prompt-only provenance)
• work-unit-source-ref: gh-issue-43
• action_mode: plan
• canonical_key: Consensus loop: reviewer prompts leave REVIEW_ROUND unresolved #43
• duplicate_search_summary: Router-scoped arbitration only; checked the three named round-2 solver artifacts and the router-injected issue snapshot. No sibling judge artifacts, lifecycle records, or external issue searches were used.
• branch: none
• PR: none
• commit: none
• unique_diff_status: not-applicable
• tests_verification: not run by meta-judge; implementation should run the focused unittest command listed in verification_hints.
• resolution: consensus
• next_action: implement

⟦AI:AUTO-LOOP⟧
META_JUDGE_DONE:consensus:structural:keep review_round required and bind REVIEW_ROUND in dispatch_reviewers with controller and prompt-render tests

⟦AI:AUTO-LOOP⟧