Skip to content

cross-review: PR に出ている指摘が記録に残らず、同じ論点が 2 つのスレッドに分かれる → GitHub と git へ書くのをレビューを回す側だけにし、途中で止まっても二度書かない(#730 #583) - #801

Merged
takemi-ohama merged 26 commits into
developfrom
feat/issue-730-writes-to-conductor
Sep 22, 2026

Conversation

@takemi-ohama

@takemi-ohama takemi-ohama commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Summary

レビューを任された担当が PR へ指摘を書き込んでから結果を残す前に止まると、PR には指摘が出ているのに記録には残らず、起動し直した担当が同じ論点をもう一度投稿していた。GitHub と git へ書くのをレビューを回す側だけにし、担当は結果を残すだけにした。書き込みと記録が同じ手順の中で続けて起きるため、途中で止まっても同じものを二度書き込まない。

設計は 設計文書(決定 1〜14)、受け入れ条件は 要求の文書(AC1〜AC32)、分解は 実装計画 にある。

Closes #730
Closes #583

何が変わるか

書き込み 変更前の担い手 変更後の担い手
レビューの投稿(総評とインライン) 担当の CLI が直接送る 指摘の取り込み(state.py read-result)が控えから組み立て、待ち行列を通して送る
指摘への返信・スレッドの決着・修正のまとめ 修正の担当 修正の取り込み(state.py merge-fix)が修正の結果ファイルから組み立てて送る
修正の送信 修正の担当がブランチ名だけを指定して送る 取り込みが現在の頭を指定して送り(HEAD:<ブランチ名>)、報告されたコミットが送り先に載ったことを確かめる
最終スイープの送信・返信・決着 スイープの担当 骨組みが共通層の 1 行で送る
  • 結果ファイルを投稿へ変える層を共通層へ新設したplugins/ndf/scripts/lib/result_posts.py)。本文は引数にも標準出力にも出さず、この層のプロセスの中だけを通る。cross-review から呼ぶ口と、修正を単独で行うときの部分命令(result_posts.py fix)の口が同じ実装を使う
  • 二度書かない照合を 4 種別すべてに掛けたpost_queue.py)。レビューの照合の鍵は本文の先頭行のラウンドと席までの前方一致で、判定の語を含めない。起動し直して判定が変わっても、同じラウンド・同じ席のレビューは増えない
  • 担当の申告と GitHub の実数の突き合わせをやめた。 記録の参照は送信の応答から、件数は送れたインラインの数から取る
  • 差分の外を指す指摘は投稿する側が総評へ移す。 契機は応答が行やファイルを解決できないこと(could not be resolved)を示すときだけで、判定の値や基準のコミットの誤りは失敗として止める
  • 担当は結果を一時の名前で書き、控え → 結果ファイルの順に改名する。 担当のプロンプトから投稿の手順・判定の格下げ・インラインの組み立てを外した
  • 起動し直しを初回と同じ経路へ通した。 骨組みの起動し直しの枝が自分の起動・監視・取り込みを持たず、繰り返しの先頭へ戻る。起動し直した担当の指摘も根拠の検証と反証を通る
  • 文書(SKILL.md / docs/0104 / references/context-budget.md / fix/SKILL.md)を新しい担い手に合わせて書き直した

実測で決めたこと

設計が未確認のまま残した 4 件を、実物の gh で 1 度ずつ動かして決め、設計文書の同じ節を書き直した(拒まれた要求は何も作らないため、確かめた跡は PR #794 に残っていない)。

何を 決めたこと
差分の外を指す指摘が拒まれるときの応答 要求ごとに全件が拒まれる。判定に使う語は could not be resolved
拒まれた応答から原因を 1 件ずつ特定できるか できない。拒まれた要求のインラインをすべて総評へ移す
すでに決着したスレッドをもう一度決着させたとき 成功し、決着済みを返す(冪等)
投稿者のアカウントが席ごとに違う環境 無い。作業環境の gh は 1 アカウント

やらないこと

受け入れ条件との対応

受け入れ条件 確かめ方
AC1〜AC3 test_launch_reviewer_prompt_context.py(投稿の手順の語が 0 件・書かせるファイル・改名の順序)
AC3・AC4 test_read_result_posts.py(控えだけ・何も書かなかった担当で投稿 0 件)
AC5・AC6・AC12・AC15・AC17・AC20・AC32 test_read_result_posts.py / test_result_posts.py
AC7〜AC9 test_merge_fix_posts.py / test_result_posts.py(切り離された頭の送信・載っていないコミットで停止)
AC10・AC11・AC13・AC14 test_queue_idempotency.py / test_result_posts.py
AC16 test_post_queue.py(実測した応答の形で区別)/ test_result_posts.py(退避と送り直し)
AC18・AC19 test_skill_layout.py(各段が 1 度だけ・起動し直しは先頭へ戻る・8 の枝が 7 より先)
AC21・AC22 test_result_posts.py(部分命令を直接呼ぶ)/ fix/SKILL.md の 1 行
AC23・AC24 既存テストを通す(終了コード・収束の判定と報告が読む変数・席の名前・結末の語彙・区分)
AC25〜AC29 test_writes_by_conductor_docs.py
AC30・AC31 下の Test plan

Test plan

実行した版はすべて 9927f24(2026-09-22 13:38〜13:45 UTC)。監視の環境変数(MONITOR_*)を外したシェルで実行した。

  • uv run --with pytest pytest scripts/tests plugins/ndf -q → 4927 passed、exit=0
  • bash scripts/build-runtime-plugins.sh --check → exit=0
  • claude plugin validate . → exit=0(未知フィールドの警告のみ)
  • python3 scripts/check-skill-frontmatter.py → exit=0
  • python3 plugins/ndf/scripts/instructions-check.py --root . → exit=0
  • 継続的統合(gh pr checks 801)→ 15 件すべて pass(pytest・runtime-smoke 4 ランタイムを含む)
  • 実物の PR で cross-review を 1 ラウンド回し、書き込みが回す側から行われることを確かめた。この作業ツリーの版の骨組みで、この PR に対してレビュー担当 1 席(kiro)で回した

検査の結果

工程 結果
構造改善 提案ラウンドの上限 2 で終了。テスト整備 10 件と構造改善 5 件を採用し、取り消しは 4 件。改修計画: #801 (comment)
実装レビュー 3 ラウンドで収束(新しい指摘 0)。指摘 4 件と最終スイープの 1 件をすべて修正し、未解決のスレッドは 0 件(GitHub 側で数え直し済み)
実装レビュー中に足した判断 単独の修正の口は、送り先のブランチを決められないときに返信へ進まず止まる(1f646aa7)

文書の検査

markdown-writing のセルフチェックを、この本文・実装計画(issues/issue-730-583-plan.md)・検査の工程で書き足した行(設計文書 issues/issue-730-583-design.mdplugins/ndf/skills/fix/SKILL.md の追加 4 行)に対して実行した。

検査 本文 実装計画 検査で足した行 扱い
識別子と略語の混入 11 行 7 行 2 行 すべてファイル名・コミットの指し示しか、業務用語に添えた括弧書き。説明文の主語・目的語には置いていない
検討痕跡・変更履歴の混入 0 件 0 件 0 件
強い否定語 0 件 0 件 0 件
過剰な装飾語 0 件 0 件 0 件
根拠の曖昧な断定 0 件 0 件 0 件
多義語(5 回以上) 0 語 0 語 0 語

🤖 Generated with Claude Code

takemi-ohama and others added 10 commits September 22, 2026 04:01
設計の時点で未確認だった 4 件を、実物の gh で 1 度ずつ動かして決めた。
拒まれた要求は何も作らないため、確かめた跡は残っていない。

- 差分の外を指す指摘が拒まれるときの応答の形(退避の契機に使う語)
- 拒まれた応答から原因を 1 件ずつ特定できるか(特定できない)
- すでに決着したスレッドをもう一度決着させたときの応答(冪等)
- 投稿者のアカウントが席ごとに違う環境があるか(無い)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGCedPTy818Zw7VYdmE4GB
レビューの照合の鍵から判定の語を外し、本文の先頭行のラウンドと席までの前方一致に
する。起動し直して判定が変わっても、同じラウンド・同じ席のレビューは増えない。
返信とまとめの照合も本文の先頭 80 文字で見る。

指した位置を差分の中に見つけられない拒まれ方を、同じ状態で返る別の拒まれ方
(判定の値の誤り・基準のコミットの誤り)と見分ける口を足した。応答の errors が
文字列の列でも失敗の説明に語が残るようにした。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGCedPTy818Zw7VYdmE4GB
担当が書いた指摘の控えと結果ファイルを読み、GitHub へ送る投稿を組み立てて待ち行列を
通す層を共通層へ置いた。本文は引数にも標準出力にも出さず、この層の中だけを通る。

- レビューの投稿: 先頭行にラウンドと席と本来の判定を置き、自分の Pull Request では
  送った形だけを落とす。位置を解決できずに拒まれたら、その要求のインラインをすべて
  総評へ移して送り直す。同じ状態の別の拒まれ方は退避せず失敗として残す
- 修正の投稿: 返信・決着・まとめを組み立てる。まとめの先頭 80 文字にラウンドと
  コミットを入れ、2 度積んでも増えないようにする
- 修正の送信: 現在の頭を指定して送り、報告されたコミットが送り先に載ったことを確かめる
- 単独で使う口として部分命令 fix を持つ

待ち行列の側に、拒まれ方を項目から読む口と、送る内容を変えて積み直すための取り除きを
足した。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGCedPTy818Zw7VYdmE4GB
取り込みの中を「控えを読む → 投稿を積む → 流す → 送信の応答を記録へ書き戻す →
指摘を取り込む」の順にした。記録に入る参照は送信の応答から、件数は送れた
インラインの数から取る。担当の申告を GitHub の実数と比べて中断する処理と、
担当が申告した投稿の失敗・参照を読んで結果なしにする処理を取り除いた。

取り込みの標準出力へ、投稿の結果(参照・インラインの件数・総評へ移した件数・
積んだ件数)と取り込んだ指摘の件数の行を足した。本文は出さない。

入口で前の取り込みが積んだ投稿を先に流す。積むのは取り込みだけで、積んだ時点で
その担当の記録を書くため、書き戻し先が揃っている。

旧い契約(担当が投稿して申告する)を前提にしたテストは、新しい契約の確かめへ
置き換えるか取り除いた。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
担当に書かせるのは指摘の控えと結果ファイルの 2 つだけにした。結果ファイルは判定と
重要度別の件数だけを持つ。投稿の呼び出し・判定の格下げ・インラインの組み立てと
差分の外への対処・投稿の失敗の申告をプロンプトから外した。総評は控えの summary へ
書かせ、送れた先(posted_to)は投稿する側が書く。

どちらのファイルも一時の名前で書かせ、控えを先・結果ファイルを後の順で改名させる。
起動の前に一時の名前の残りも消す。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
修正の取り込みが、現在の頭を指定して送り、報告されたコミットが送り先に載ったことを
確かめてから、返信・決着・まとめを待ち行列を通して送る。載っていなければ記録も
投稿もせずに止まる。まとめの参照は投稿の応答から記録へ書く。

修正の担当はコミットと戻り値ファイルまでを行う。単独で使うときは、戻り値ファイルを
書いた後に共通層の部分命令を 1 行実行する。返信・決着・まとめ・送信の実装は
cross-review から呼ぶ経路と同じものである。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
骨組みの起動し直しの枝が自分の起動・監視・取り込みを持たず、繰り返しの先頭へ戻る
形にした。起動し直した担当の指摘も、根拠の検証と反証を通ってから判定へ進む。
待ち行列に残りがあるときの枝は各判定の直後、起動し直しより先に見る。

設計方針の表から、担当が直接投稿する行と申告を突き合わせる行を外し、投稿の担い手と
その理由を入れた。修正の手順(docs/02)から担当の送信・返信・決着・まとめを外した。

最終スイープも /ndf:fix を使うため、担当はコミットと結果ファイルまでとし、骨組みが
共通層の 1 行で送信・返信・決着を行う。スイープは見送りのスレッドも決着させるため、
要素ごとに決着を求める印を読む。

担当のプロンプトの見出しを /ndf:pr-review から外した(そちらは担当が直接投稿する
手順を持つ)。送信の認証の退避は共通層の値を読み、失敗したときだけ 1 度やり直す。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- docs/01: 担当は投稿せず、取り込みが投稿して送信の応答を記録にする節へ書き直した。
  申告と実数の突き合わせの節を取り除いた
- docs/03: 差分の外を指す指摘は投稿する側が総評へ移すこと、担当に投稿させない理由
- docs/04: 担当が書く 2 ファイルの形(一時の名前と改名の順序)、記録に足した鍵、
  投稿の種別ごとの積む側・組み立ての元・照合の鍵
- context-budget: 工夫の 4 番目を「本文はプロセスの中だけを通る」へ書き直した

文書の受け入れ条件(AC25〜AC29)を検査にした。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tes-to-conductor

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
APIレート制限時のレビュー保留と、Queue.dropの一致・不一致の振る舞いを固定する。

Item-Id: R1-001
Round: 1
Impl-Runtime: codex
Impl-Model: default
@takemi-ohama

takemi-ohama commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

改修計画 — devbasex/ai-plugins #801

/ndf:cross-refactoring が提案し、適用した改善項目の記録である。
理由と手順は提案の時点でしか残らないため、公開の直前に書き出している。

  • 対象範囲: plugins/ndf/scripts/lib, plugins/ndf/skills/cross-review/scripts, plugins/ndf/scripts/tests, plugins/ndf/skills/cross-review/tests
  • 着手前のテスト: uv run --with pytest pytest scripts/tests plugins/ndf -q

ラウンド 1(実装 codex)

R1-001 — plugins/ndf/scripts/lib/result_posts.py#post_review

兆候・経路 手法・階層 重要度 提案元 状態 コミット
branch integration codex / agy 採用 1

なぜ: post_review において、API レート制限(HTTP 429)が発生した際に flushed.rate_limited の判定により failed=False かつ queued=1 として処理される分岐が固定されていない

手順: 1. インライン指摘を含む控えとレビュー結果、一時ディレクトリ上の Queue を作る
2. GitHub API の疑似実装から上限応答を返す
3. post_review を公開入口から実行する
4. queued が 1、failed が偽、投稿 URL と投稿済み件数が空であり、控えに posted_to が追加されず待ち行列に要求が残ることを比較する

R1-002 — plugins/ndf/scripts/lib/result_posts.py#push_fix

兆候・経路 手法・階層 重要度 提案元 状態 コミット
error unit agy / kiro 採用 1

なぜ: push_fix において、コミットなしや送り先不明、merge-base 失敗は固定されているが、git push そのものが失敗(非 0 終了コード)した際に中断し PushResult(False, False, False, ...) を返すエラー経路が固定されていない

手順: 1. 既存の _repo_with_remote と同じ形で作業ツリーを作り、origin を実在しない先へ差し替えるか、送れないブランチ指定で push を失敗させる
2. 返る PushResult の ok が False、pushed が False であることを比較する
3. detail が空でないこと(失敗理由が載ること)を比較する

R1-003 — plugins/ndf/scripts/lib/post_queue.py#Queue.drop

兆候・経路 手法・階層 重要度 提案元 状態 コミット
branch unit agy 採用 1

なぜ: Queue.drop は PR #801 で新設されたメソッドだが、指定された seq に一致する項目ファイルを削除して True を返す分岐と、一致しない場合や seq が None の場合に False を返してファイルを残す分岐が単体テストで固定されていない

手順: 1. 待ち行列ディレクトリに複数の項目ファイルを作成する
2. 存在する seq を指定して drop を呼び出し、True が返り対象ファイルのみが削除されることを確認する
3. 存在しない seq および None を指定して drop を呼び出し、False が返り他のファイルが保持されることを確認する

R1-004 — plugins/ndf/scripts/lib/post_queue.py#rejected_by_position

兆候・経路 手法・階層 重要度 提案元 状態 コミット
branch unit agy 採用 1

なぜ: rejected_by_position は PR #801 で新設された関数だが、last_status が 422 かつエラー文にキーワードが含まれる場合(True)と、422 だが別エラーの場合(False)、422 以外のステータス(False)の各分岐が直接単体テストで固定されていない

手順: 1. last_status が 422 かつ last_error に 'Line could not be resolved' を含む項目辞書を渡し True が返ることを確認する
2. last_status が 422 で last_error が別のエラー理由である項目辞書を渡し False が返ることを確認する
3. last_status が 404 や None である項目辞書を渡し False が返ることを確認する

R1-005 — plugins/ndf/scripts/lib/post_queue.py#retry

兆候・経路 手法・階層 重要度 提案元 状態 コミット
branch unit codex 採用 1

なぜ: retry の公開入口には成功・通常失敗・上限後の成功・待機上限到達の分岐があるが、対象テスト群には retry を呼ぶテストがなく、全経路が未固定である

手順: 1. 成功、通常失敗、上限後に成功、上限が続く応答列を作る
2. 待機関数を時間を進めず記録する疑似実装にする
3. 各応答列で retry を実行する
4. 成功と通常失敗は即時の結果、上限後の成功は再実行後の結果、待機上限到達は最後の上限応答を返し、待機時間列が指定した上限内であることを比較する

ラウンド 2(実装 agy)

R2-001 — plugins/ndf/scripts/lib/post_queue.py#retry

兆候・経路 手法・階層 重要度 提案元 状態 コミット
boundary unit codex 採用 1

なぜ: 上限から成功する再実行と待機上限を超える経路は固定されているが、interval が 0 の境界で待機も再実行もせず最初の上限応答を返す経路は固定されていない

手順: 1. 上限応答を返す実行入力と interval=0 を与える
2. retry を呼ぶ
3. 最初の応答がそのまま返り、待機が行われず実行回数が 1 の現在挙動を比較する

R2-002 — plugins/ndf/scripts/lib/result_posts.py#fix_posts

兆候・経路 手法・階層 重要度 提案元 状態 コミット
branch unit codex 採用 1

なぜ: 正しい comment_id を持つ resolved、deferred、rejected の返信は固定されているが、数値に変換できない comment_id を含む結果を読み飛ばして残りの投稿を組み立てる経路は固定されていない

手順: 1. resolved、deferred、rejected に数値化できない comment_id と有効な thread_id を持つ結果ファイルを作る
2. fix_posts を呼ぶ
3. 不正な返信が出力されず、現在どおり決着項目とまとめが残ることを種別と公開フィールドで比較する

R2-003 — plugins/ndf/scripts/lib/result_posts.py#post_review

兆候・経路 手法・階層 重要度 提案元 状態 コミット
branch unit codex 採用 1

なぜ: 送信応答に html_url がある経路と既存投稿を見つける経路は固定されているが、応答が id だけを持つときに review_url を pullrequestreview のフラグメントへ補う経路は固定されていない

手順: 1. レビュー一覧は空、レビュー作成応答は id だけを返す偽の GitHub 入力を作る
2. post_review を公開入口から呼ぶ
3. review_url が現在のフラグメント形式になり、投稿済み件数と待ち行列件数も現在の値になることを比較する

R2-004 — plugins/ndf/scripts/lib/statefile.py#register_after_save

兆候・経路 手法・階層 重要度 提案元 状態 コミット
branch unit kiro 採用 1

なぜ: register_after_save は「同じ関数を 2 度登録しても 1 度だけ呼ぶ」重複排除の分岐 (if hook not in _AFTER_SAVE) を持つが、範囲内でも cross-refactoring/tests でも固定されていない。cross-refactoring/tests が触るのは 1 度だけの登録である。

手順: 1. statefile を importlib で読み込む
2. 呼ばれた回数を控える差し込み口を用意する
3. 同じ差し込み口を register_after_save で 2 回登録し、テスト後に unregister する
4. save を 1 回呼ぶ
5. 控えた呼び出し回数が 1 であることを比較する

R2-005 — plugins/ndf/scripts/lib/statefile.py#save

兆候・経路 手法・階層 重要度 提案元 状態 コミット
normal unit kiro 採用 1

なぜ: test_statefile_emit.py は emit だけを固定し、save は共通層のテストで一度も通っていない。cross-refactoring/tests は差し込み口が失敗する error 経路だけを固定しており、正常保存(原子的な書き込みで読み出せる本文と、登録した差し込み口が (path, state) を受け取って呼ばれる正常系)は範囲内で固定されていない。

手順: 1. statefile を importlib で読み込む
2. register_after_save で受け取った引数を控える差し込み口を登録し、テスト後に unregister する
3. tmp_path 配下の存在しない親ディレクトリを含むパスへ save(path, {"a": 1}) を呼ぶ
4. path を load で読み戻し、保存した dict と一致することを比較する
5. 控えた引数が (path, 保存した state) と一致することを比較する

ラウンド 3(実装 kiro)

R3-001 — plugins/ndf/scripts/lib/result_posts.py#post_review

兆候・経路 手法・階層 重要度 提案元 状態 コミット
duplication consolidate_duplication minor agy / kiro 取り消し 1

なぜ: 最初の送信処理と、位置解決失敗(rejected_by_position)時の再送処理において、「review_posts で投稿項目を生成 → post_queue.enqueue で待ち行列へ追加 → read_item で連番 seq を取得 → queue.flush で送信」という一連の 4 手順が完全に同じ順序・同一構造で 2 回記述されている。送信や待ち行列処理の改修時に 2 箇所の足並みが崩れる原因となる。

手順: 1. 「項目を組み立てて enqueue し、seq と flush 結果を返す」局所ヘルパ(例: _enqueue_review)を抽出する。引数は evacuate_all の真偽で、payload_path / result_path / repo / pr / round_no / seat / head_sha / is_own_pr / actor は post_review のスコープから渡す
2. 初回送信をヘルパ呼び出しへ置き換える(item, seq, flushed を受け取る)
3. rejected_by_position の分岐内の送り直しも、drop の後に同じヘルパ呼び出し(evacuate_all=True)へ置き換える
4. test_result_posts.py の post_review のテスト(位置解決の失敗で退避する経路・別の拒まれ方で失敗として残す経路を含む)を実行し、review_url / posted_inline / posted_body / failed が変わらないことを確認する

R3-002 — plugins/ndf/scripts/lib/result_posts.py#fix_posts

兆候・経路 手法・階層 重要度 提案元 状態 コミット
duplication consolidate_duplication minor agy / kiro 採用 1

なぜ: resolved / deferred / rejected の 3 つのループが同型で、異なるのは返信本文の定型文と理由を取り出すキー(reason_for_deferral / reason_for_rejection)のみである。返信の組み立て処理や形式を変更する際に 3 箇所を同期して修正する必要があり、修正漏れや不整合を招きやすい。これらは「指摘に対する返信の生成」という同一の業務ルールに由来する重複である。

手順: 1. (種別, 理由キー列, 本文テンプレート) の対応表を関数内に置く(resolved は理由なしの固定句、deferred と rejected はそれぞれの理由キー列とテンプレート)
2. 3 つのループを、対応表を回す 1 つのループへ置き換え、各 entry から _reply を組み立てて items へ積む
3. closing の算出(resolved + resolve フラグの立った deferred/rejected)は現状の並び順(返信→決着→まとめ)を保つよう据え置く
4. test_result_posts.py の fix_posts のテスト(resolved_threads / deferred / rejected を与える経路)を実行し、返信本文・並び順・件数が変わらないことを確認する

R3-003 — plugins/ndf/scripts/lib/assignment.py#resolve_participants

兆候・経路 手法・階層 重要度 提案元 状態 コミット
long_method extract_method major codex 取り消し 1

なぜ: 入力の正規化、include/exclude の整合性検証、only の適用、認証 probe 結果の分類、require_all の失敗判定、Participants の組み立てという別々の段階を 1 関数が通しで行っている。各分岐は test_lib_participants.py から公開入口を通して固定されている。

手順: 1. test_lib_participants.py を現状固定テストとして実行する
2. pool/include/exclude の検証と参加者列の確定を、確定済みの list を受け取る関数へ抽出する
3. only の検証と適用を、参加者列を受けて参加者列を返す関数へ抽出する
4. probe の戻り値から available/unavailable を作る処理を抽出する
5. resolve_participants には各段階の呼び出しと Participants の組み立てだけを残し、同じテストを再実行する

R3-004 — plugins/ndf/skills/cross-review/tests/conftest.py#_no_github_state

兆候・経路 手法・階層 重要度 提案元 状態 コミット
mock_targets_implementation_detail fix_dependency_direction major codex 取り消し 1

なぜ: autouse fixture が state.py の非公開関数 _fetch_check_runs と _fetch_pr_metadata、および state_mod.result_posts の post_review・push_fix・post_fix を名前で直接差し替えている。内部関数の抽出や移動だけで多数のテスト基盤が壊れるため、テストが実装詳細を固定している。

手順: 1. GitHub へ接続しないことと既存テストの結果を現状固定する
2. state.py の呼ばれる側に GitHub 取得と投稿処理の依存をまとめた入出力境界を定義する
3. 本番経路では現在の関数群を束ねた実体を境界で渡す
4. state_mod fixture にはオフライン実装を境界から注入し、非公開関数名への monkeypatch を除く
5. real_github と fake_gh は同じ境界へ実装を渡す形に変更し、対象全体のテストを実行する

R3-005 — plugins/ndf/skills/cross-review/scripts/state.py#_init_new_state

兆候・経路 手法・階層 重要度 提案元 状態 コミット
long_method extract_method major codex 取り消し 1

なぜ: 200 行の関数内に PR 所有権の解決、レビュー指示の準備、worktree と既存コメントの準備、担当割当、初期 state 構築、保存と出力の 6 段階がローカル関数として同居している。各段階には名前が付いているが、ローカル定義のため全体を読まないと依存と実行順を追えず、段階単位の直接テストもしにくい。cmd_init を通る既存の初期化・worktree・fallback・再開テストがある。

手順: 1. 現在の cmd_init 経由の初期化テストを現状固定テストとして実行する
2. _resolve_pr_and_ownership、_prepare_review_instructions、_prepare_worktree_and_comments、_prepare_initial_assignment、_build_initial_review_state、_finalize_initial_state を同じモジュールのトップレベルへ 1 関数ずつ抽出する
3. 各抽出で使う値は既存の NamedTuple を引数と戻り値に用い、_init_new_state には段階の呼び出し順だけを残す
4. 抽出のたびに初期化関連テストを実行し、最後に対象全体のテストを実行する

ラウンド 4(実装 claude)

R4-001 — plugins/ndf/skills/cross-review/scripts/state.py#_guard_previous_round

兆候・経路 手法・階層 重要度 提案元 状態 コミット
long_method extract_method major codex 採用 1

なぜ: 前ラウンドの旧形式 verdict の復元、修正記録の必須判定、申告済み Resolve の GitHub 照合という独立した規則を 1 関数が担い、状態互換性の判断と外部照会が混在している

手順: 1. 旧形式の状態から verdict を復元する処理を resolve_previous_verdict として抽出する
2. changes_requested に fix が必要という検査を独立したメソッドへ抽出する
3. resolved_thread_ids と未解決スレッドを照合する処理を独立したメソッドへ抽出する
4. _guard_previous_round は 3 段階を順に呼ぶ調整役にし、既存の round guard テストで終了コードとメッセージを固定する

R4-002 — plugins/ndf/scripts/lib/post_queue.py#Queue.flush

兆候・経路 手法・階層 重要度 提案元 状態 コミット
long_method extract_method major codex 採用 1

なぜ: 1 メソッドが項目の読込失敗、既投稿の検出、送信成功、応答 JSON の解析、送信失敗の永続化、レート制限判定を順に扱い、各終了条件とファイル削除の責務が同居している

手順: 1. 1 項目の読込と既投稿判定を、送信要否を返すメソッドへ抽出する
2. send の結果から response または失敗情報を項目へ反映するメソッドを抽出する
3. flush は連番走査、sent/skipped の集約、最初の失敗での停止だけを担う形にする
4. 既存のキュー送信・冪等性テストで返却値、ファイル削除、失敗時の残存順序が不変であることを確認する

R4-003 — plugins/ndf/scripts/lib/result_posts.py#cmd_fix

兆候・経路 手法・階層 重要度 提案元 状態 コミット
long_method extract_method minor kiro 採用 1

なぜ: cmd_fix が「引数からの入力の解決(worktree・repo・head・result のパス・fix の読み込みと存在検査)」と「送信 push_fix とその出力」「actor の解決と post_fix とその出力」を 1 つの関数で通しに行っている。前半の入力解決は gh のフォールバックを含めて段階に名前が付けられ、後半の副作用(print / return)とは関心が分かれている。

手順: 1. worktree・repo・head・result・fix の解決までを _resolve_fix_inputs(args) として抽出し、解決した値(または失敗のとき None)を返す形にする(print と return 1 の失敗処理は呼び出し側に残すか、失敗理由を返り値へ載せる)
2. cmd_fix は _resolve_fix_inputs を呼び、その後の push_fix / post_fix と出力だけを担うようにする
3. 出力の文言・終了コードは変えない
4. uv run --with pytest pytest scripts/tests/test_result_posts.py -q を実行し、単独コマンドの端から端までのテストを含め 24 passed を確認する

R4-004 — plugins/ndf/skills/cross-review/scripts/state.py#cmd_collect_critiques

兆候・経路 手法・階層 重要度 提案元 状態 コミット
long_method extract_method minor codex 検証中 1

なぜ: コマンド入口が状態検証、対象 finding の索引作成、反証の取込、重複統合、不足反証の算出、完了印と保存まで複数段階を直列に実行している

手順: 1. 現ラウンドの finding 索引を作る処理を抽出する
2. reviewers ごとの不足 finding_id を算出する処理を抽出する
3. cmd_collect_critiques は取込、重複統合、不足時の処理、完了記録の順序だけを表す形にする
4. 既存の critiques、classify findings、pipeline wiring テストで取込件数、再取得判定、保存結果が不変であることを確認する

見送った項目

ラウンド 対象 兆候・経路 理由
1 plugins/ndf/scripts/lib/post_queue.py#review_match_key branch 1 ラウンドの採用上限 5 件を超えた
1 plugins/ndf/scripts/lib/result_posts.py#post_fix branch 1 ラウンドの採用上限 5 件を超えた
1 plugins/ndf/scripts/lib/result_posts.py#post_fix normal 1 ラウンドの採用上限 5 件を超えた
1 plugins/ndf/scripts/lib/result_posts.py#push_fix boundary 1 ラウンドの採用上限 5 件を超えた
2 plugins/ndf/skills/cross-review/scripts/state.py#cmd_merge_fix error 1 ラウンドの採用上限 5 件を超えた
2 plugins/ndf/skills/cross-review/scripts/state.py#cmd_read_result error 1 ラウンドの採用上限 5 件を超えた
3 plugins/ndf/skills/cross-review/scripts/state.py#cmd_check_oscillation magic_value 1 ラウンドの採用上限 5 件を超えた
3 plugins/ndf/scripts/lib/result_posts.py#post_review duplication テストの期待する振る舞いが変わっています(plugins/ndf/skills/cross-review/tests/conftest.py, plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py, plugins/ndf/skills/cross-review/tests/test_merge_fix_posts.py, plugins/ndf/skills/cross-review/tests/test_rejected_findings.py, plugins/ndf/skills/cross-review/tests/test_state_ci_classification.py, plugins/ndf/skills/cross-review/tests/test_state_init_changed_files_fallback.py, plugins/ndf/skills/cross-review/tests/test_state_init_worktree_creation.py, plugins/ndf/skills/cross-review/tests/test_state_judge_ci.py, plugins/ndf/skills/cross-review/tests/test_state_offline_fetch.py, plugins/ndf/skills/cross-review/tests/test_state_review_pool.py, plugins/ndf/skills/cross-review/tests/test_state_run_metrics.py)。構造改善では期待出力を変えません。振る舞いの変更は別の変更に分けてください
3 plugins/ndf/scripts/lib/assignment.py#resolve_participants long_method テストの期待する振る舞いが変わっています(plugins/ndf/skills/cross-review/tests/conftest.py, plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py, plugins/ndf/skills/cross-review/tests/test_merge_fix_posts.py, plugins/ndf/skills/cross-review/tests/test_rejected_findings.py, plugins/ndf/skills/cross-review/tests/test_state_ci_classification.py, plugins/ndf/skills/cross-review/tests/test_state_init_changed_files_fallback.py, plugins/ndf/skills/cross-review/tests/test_state_init_worktree_creation.py, plugins/ndf/skills/cross-review/tests/test_state_judge_ci.py, plugins/ndf/skills/cross-review/tests/test_state_offline_fetch.py, plugins/ndf/skills/cross-review/tests/test_state_review_pool.py, plugins/ndf/skills/cross-review/tests/test_state_run_metrics.py)。構造改善では期待出力を変えません。振る舞いの変更は別の変更に分けてください
3 plugins/ndf/skills/cross-review/tests/conftest.py#_no_github_state mock_targets_implementation_detail テストの期待する振る舞いが変わっています(plugins/ndf/skills/cross-review/tests/conftest.py, plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py, plugins/ndf/skills/cross-review/tests/test_merge_fix_posts.py, plugins/ndf/skills/cross-review/tests/test_rejected_findings.py, plugins/ndf/skills/cross-review/tests/test_state_ci_classification.py, plugins/ndf/skills/cross-review/tests/test_state_init_changed_files_fallback.py, plugins/ndf/skills/cross-review/tests/test_state_init_worktree_creation.py, plugins/ndf/skills/cross-review/tests/test_state_judge_ci.py, plugins/ndf/skills/cross-review/tests/test_state_offline_fetch.py, plugins/ndf/skills/cross-review/tests/test_state_review_pool.py, plugins/ndf/skills/cross-review/tests/test_state_run_metrics.py)。構造改善では期待出力を変えません。振る舞いの変更は別の変更に分けてください
3 plugins/ndf/skills/cross-review/scripts/state.py#_init_new_state long_method テストの期待する振る舞いが変わっています(plugins/ndf/skills/cross-review/tests/conftest.py, plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py, plugins/ndf/skills/cross-review/tests/test_merge_fix_posts.py, plugins/ndf/skills/cross-review/tests/test_rejected_findings.py, plugins/ndf/skills/cross-review/tests/test_state_ci_classification.py, plugins/ndf/skills/cross-review/tests/test_state_init_changed_files_fallback.py, plugins/ndf/skills/cross-review/tests/test_state_init_worktree_creation.py, plugins/ndf/skills/cross-review/tests/test_state_judge_ci.py, plugins/ndf/skills/cross-review/tests/test_state_offline_fetch.py, plugins/ndf/skills/cross-review/tests/test_state_review_pool.py, plugins/ndf/skills/cross-review/tests/test_state_run_metrics.py)。構造改善では期待出力を変えません。振る舞いの変更は別の変更に分けてください
4 plugins/ndf/scripts/lib/result_posts.py#post_review duplication 過去のラウンドで見送った項目のため対象外

takemi-ohama and others added 9 commits September 22, 2026 10:23
…#push_fix, plugins/ndf/scripts/lib/post_queue.py#rejected_by_position

push_fix で git push そのものが失敗したとき、載ったかを確かめずに
PushResult(False, False, False, 理由) を返す経路を固定する。
rejected_by_position が 422 と位置の語がそろうときだけ真を返す分岐
(422 の別理由・404・状態なしは偽)を固定する。

Item-Id: R1-002
Round: 1
Impl-Runtime: claude
Impl-Model: default
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
retry の公開入口の 4 分岐(成功・通常失敗・上限後の成功・待機上限到達)を
現状固定テストで固定する。run を応答列を返す疑似実装に、sleep を時間を進めず
待機秒数を記録する疑似実装に差し替え、結果と待機時間列が上限内であることを固定する。
対象コードは変更しない。

Item-Id: R1-005
Round: 1
Impl-Runtime: kiro
Impl-Model: default
retry の待機間隔ゼロ、不正な comment_id、保存後フックの重複登録を固定する。

Item-Id: R2-001
Round: 2
Impl-Runtime: codex
Impl-Model: default
post_review で応答が id のみの場合のフラグメント URL 補完と、
statefile.save での親ディレクトリ自動作成および登録後フック呼出を固定する。

Item-Id: R2-003
Round: 2
Impl-Runtime: agy
Impl-Model: default
…irection — cross-review 収束ループの構造改善

- result_posts.post_review: 初回送信と位置解決失敗時の送り直しの 4 手順の重複を
  _enqueue_review ヘルパへ束ねた(R3-001)
- assignment.resolve_participants: 検証・only 適用・probe 分類を
  _validate_and_build_participants / _apply_only / _classify_probe へ抽出(R3-003)
- state.py: GitHub 取得と投稿処理を GitHubGateway 境界へまとめ、副コマンドは
  GITHUB 越しに呼ぶ。テストは非公開関数名ではなく境界へオフライン実装を注入する
  形へ変更(R3-004)
- state._init_new_state: 6 段のローカル関数をトップレベルへ切り出し、本体には
  呼び出し順だけを残した(R3-005)

いずれも振る舞いは不変。全体テスト(uv run --with pytest pytest scripts/tests
plugins/ndf -q)が 4913 passed / exit=0。

Item-Id: R3-001
Round: 3
Impl-Runtime: kiro
Impl-Model: default
…ndency_direction — cross-review 収束ループの構造改善"

This reverts commit 6695c13.
…sts.py#fix_posts

resolved / deferred / rejected の返信を組み立てる同型の 3 つのループを、
(要素の列, 定型句, 理由キー) の対応表を回す 1 つのループへまとめた。
返信本文・並び順・件数は変わらない(決着・まとめの組み立ては据え置き)。

Item-Id: R3-002
Round: 3
Impl-Runtime: claude
Impl-Model: claude-opus-5
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
前ラウンド検査、投稿キュー送信、fix 入力解決の各段階を補助メソッドへ抽出する。

Item-Id: R4-001
Round: 4
Impl-Runtime: codex
Impl-Model: default
…ate.py#cmd_collect_critiques

現ラウンドの finding 索引の作成と、reviewers ごとの不足 finding_id の算出を
それぞれ _round_finding_index / _missing_critique_targets へ抽出した。
cmd_collect_critiques は取込・重複統合・不足時の処理・完了記録の順序だけを
表す形になる。振る舞いは不変。

Item-Id: R4-004
Round: 4
Impl-Runtime: kiro
Impl-Model: default

@takemi-ohama takemi-ohama left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 1 | kiro | REQUEST_CHANGES

新設の result_posts.py に、非整数の line でレビュー投稿全体が失われる経路が 1 件ある(インライン参照)。本 PR が塞ごうとしている「投稿が記録に残らず失われる」現象と同種のため修正を推奨する。他は設計レベルで追加の修正提案なし。

Comment thread plugins/ndf/scripts/lib/result_posts.py Outdated

@takemi-ohama takemi-ohama left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 1 | agy | REQUEST_CHANGES

単独実行時の環境依存と、不正な行番号入力に対する例外クラッシュの防止に関する 2 件の改善を提案します。

Comment thread plugins/ndf/scripts/lib/result_posts.py
Comment thread plugins/ndf/scripts/lib/result_posts.py Outdated
takemi-ohama and others added 3 commits September 22, 2026 12:11
担当が書き出す指摘の行は外部入力である。`"L42"` や `"40-45"` を素の int() へ
渡すと ValueError でレビュー全体が失われていた。`_line_no` で正の整数として
読めるときだけ位置を持つものとし、読めない指摘は総評へ退避する。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`--worktree` を解決していながら、`gh repo view` / `gh pr view` は呼び出し元の
cwd で動いていた。作業ツリーの外から呼ぶとリポジトリを決められずに止まるか、
頭が空になって送信を飛ばしたまま返信へ進む。`_sh` に cwd を渡せるようにし、
`gh pr view` には解決済みのリポジトリを `-R` で渡す。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@takemi-ohama

Copy link
Copy Markdown
Contributor Author

🔧 /ndf:fix サマリ (round 1)

対応件数: critical=0 / major=3 / minor=0(合計 3 件)
deferred: 0 件 / rejected: 0 件
commit: df1066b, b53a68a, 1f646aa
CI: SUCCESS(完了済みのジョブに失敗なし。実行中のジョブの完了は待っていない)

詳細

  • 行番号が整数にならない指摘の扱い(スレッドスレッド): 同じ原因の 2 件。行番号を正の整数として読めるときだけ位置として扱い、読めない指摘は例外で落とさず総評へ回すようにした(df1066ba)
  • 単独の修正の口でのリポジトリとブランチの解決(スレッド): 解決を作業ツリーの中で行うようにした(b53a68a4)。加えて、送り先のブランチを決められないときは返信へ進まず止めるようにした(1f646aa7)

@takemi-ohama takemi-ohama left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 2 | codex | REQUEST_CHANGES

待ち行列の再開経路に要修正が 1 件あります。

Comment thread plugins/ndf/scripts/lib/result_posts.py Outdated

@takemi-ohama takemi-ohama left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 2 | kiro | APPROVE

投稿と記録を read-result / merge-fix の 1 コマンドへ閉じる設計は妥当で、途中停止時の状態が「送れていない」「送れたが記録が無い」の 2 つに収束する点、review_match_key が判定の語を鍵に含めず起動し直し時の二重投稿を防ぐ点、位置解決失敗(could not be resolved)だけを退避対象とし判定値・commitOID の誤りは失敗として残す点、いずれも意図と実装が一致している。削除された _posted_comment_count / _verify_declared_comments / _resolve_result_aliases への残存参照は無く、intent alias と event/intent 欠落時の終了コードは既存テストで固定されている。cross-review 系 1100 件のテストは通過(手元で実行)。設計・PR 横断で修正を要する指摘は無い。

先客が位置エラーで残る再開では、先客を消して今回分を二重に積み、未投稿のまま
控えへ送れた先を書いて取り込みを成功扱いにしていた。退避は今回分の連番が
拒まれたときだけ行い、今回分が送れていない限り控えを書かず失敗として返す
(上限による待ちは従来どおり失敗にしない)。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@takemi-ohama

Copy link
Copy Markdown
Contributor Author

🔧 /ndf:fix サマリ (round 2)

対応件数: critical=0 / major=1 / minor=0(合計 1 件)
deferred: 0 件 / rejected: 0 件
commit: 4e2bc22
CI: PENDING(完了済みのジョブに失敗なし。15 件が実行中で、完了は待っていない)

詳細

  • 位置エラーの退避の対象(スレッド): 先に積まれた項目の失敗を今回の分と取り違えないよう、退避は今回積んだ項目が拒まれたときだけに限った。今回の分が送れていなければ控えを書かず失敗として返す(4e2bc224)

@takemi-ohama takemi-ohama left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 3 | codex | REQUEST_CHANGES

Comment thread plugins/ndf/scripts/lib/result_posts.py

@takemi-ohama takemi-ohama left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 3 | agy | APPROVE

要求仕様(#730 #583、AC1〜AC32)に対する実装・テスト・ドキュメントの網羅性を確認しました。
前ラウンドおよび直近の修正コミット(4e2bc224 による位置エラー退避判定の修正、1f646aa7 によるブランチ未確定時の安全停止、b53a68a4 の作業ツリー外実行対応、df1066ba の行番号パース例外防止など)により、エッジケースを含む安定性が確保されています。
単体テスト・静的検証(build-runtime-plugins.sh --check, claude plugin validate, check-skill-frontmatter.py)も正常に通過しており、ブロッカーとなる問題や新たな指摘事項はありません。

takemi-ohama and others added 2 commits September 22, 2026 13:17
書き戻しが途中で落ちると半端な控えが残り、再実行で空として読まれて記録済みの
指摘を 0 件で置き換えていた。状態ファイルの原子的な書き込みを共通層
(statefile.write_json_atomic)へ切り出して控えにも使い、書けなかったときは
失敗として返して取り込みを止める。再実行は既投稿として照合し直す。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@takemi-ohama takemi-ohama left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 1 | kiro | APPROVE

新設の result_posts.py と post_queue.py / statefile.py / state.py の変更を、正確性・堅牢性・冪等性・ドキュメント整合の観点で確認した。既存コメントで挙がっていた重大な不具合(非整数 line による例外、単独 fix の repo/head 解決の作業ツリー外実行、位置エラー退避の対象取り違え、控えの原子的書き戻し、回し直し実行のレビュー取り違え)はいずれも head(df1066ba / b53a68a / 1f646aa / 4e2bc22 / a3babd3 / 9927f24)で対応済みで、再指摘に値する残存経路は見当たらない。付随して確認した点: (1) since と GitHub の submitted_at は共に tz-aware で _parse_time の比較が成立する、(2) review-reply / review-post の再実行は posted_match の冪等照会で二重投稿を防ぐ、(3) SKILL.md / fix SKILL.md が記す result_posts.py fix の引数と失敗時の終了コードは実装と一致する(実行して確認)。設計・PR 横断で修正を要する指摘は無い。

@takemi-ohama
takemi-ohama marked this pull request as ready for review September 22, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant