docs(adr): ADR-051 クロスシステム設定 coupling パターンを起票 (順位263) - #256
Merged
Conversation
📝 WalkthroughWalkthroughクロスシステム設定 coupling パターンに関する新規ADR-051を追加し、CLAUDE.mdのArchitecture Decisions一覧とADR-019の既知の制約節にADR-051への参照を追記した。また、対応する完了済みタスクをdocs/todo-summary.mdとdocs/todo13.mdから削除した。 ChangesADR-051 導入とドキュメント整合
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/adr/adr-051-cross-system-config-coupling.md (1)
55-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win「原理的に不可能」の適用範囲を少し絞ってください。
この書き方だと、将来の別 SaaS まで一律に「照会不能」と読めます。ここは「この統合では外部側の実効値を照会できないため」のように scope を限定すると、ADR-051 の汎用化として誤読されにくくなります。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/adr/adr-051-cross-system-config-coupling.md` around lines 55 - 58, The scope of the “原理的に不可能” statement is too broad and could be misread as applying to all future SaaS integrations. Update the ADR wording in the “ランタイム cross-validation の限界 (明示)” section to explicitly constrain it to this integration, using the existing cross-validation and server-side config discussion to say that the external side’s effective values cannot be queried here. Keep the emphasis on this system’s limitation rather than a universal rule, while preserving the explanation that enforcement relies on documentation and discipline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/adr/adr-051-cross-system-config-coupling.md`:
- Around line 55-58: The scope of the “原理的に不可能” statement is too broad and could
be misread as applying to all future SaaS integrations. Update the ADR wording
in the “ランタイム cross-validation の限界 (明示)” section to explicitly constrain it to
this integration, using the existing cross-validation and server-side config
discussion to say that the external side’s effective values cannot be queried
here. Keep the emphasis on this system’s limitation rather than a universal
rule, while preserving the explanation that enforcement relies on documentation
and discipline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2d33a9cd-681b-4c25-a5dd-56ef261eeb0d
📒 Files selected for processing (5)
CLAUDE.mddocs/adr/adr-019-coderabbit-review-hybrid-policy.mddocs/adr/adr-051-cross-system-config-coupling.mddocs/todo-summary.mddocs/todo13.md
💤 Files with no reviewable changes (2)
- docs/todo13.md
- docs/todo-summary.md
aloekun
added a commit
that referenced
this pull request
Jul 7, 2026
aloekun
added a commit
that referenced
this pull request
Jul 21, 2026
## 問題 `[diff] command` に `jj diff -r @` が直書きされており、AI レビュアーには **tip コミットの diff しか渡っていなかった**。祖先コミットは pre-push のセルフレビューを 一度も経ずに merge される。 同じパイプライン内で `pr_size_check` と `docs_only_routing` は `<base>..@` (PR 範囲) を見ており、`[diff]` だけが非対称だった。実際 PR #311 では 695 行の PR に対して 37 行だけがレビュー対象になり、レビュアーは渡された 37 行を見て正しく「docs-only」と 判定していた。**レビュアー側からは「渡された diff が PR 全体か」を検証できない**ため、 この誤りは誰にも検知されない。 docs/todo-summary2.md 順位 288 として既知で、Severity High で PR #268/#300/#301 に 続き #311 が 4 回目の再発。 ## 変更 - **範囲の真実源を 1 箇所に**: top-level `default_branch` を新設し、`diff` / `docs_only_routing` / `pr_size_check` の 3 stage が `Config::resolve_base_branch` 経由で同じ値を使う。従来は section ごとに独立した `default_branch` を持ち、 「値を同期する義務」を config コメントで課していた (docs_only_routing.rs の doc に 明記されていた) が、義務はコード上の不変条件ではないため非対称を許していた。 section 側は後方互換の override として残す (派生プロジェクトの既存 config 対策)。 - **config から revset を排除**: `[diff] command` は `{{PR_RANGE}}` プレースホルダを 使う。push-runner が `<base>..@` に展開するため、狭い範囲を書く余地が無くなる。 - **範囲カバレッジ検査 (fail-closed)**: 生成した diff が PR 範囲の全変更ファイルを 含むか `jj diff --summary` と突き合わせ、不足があれば exit 5 で中断する。config の 書き方に依存せず「レビュー範囲 < PR 範囲」を検知できるため、未更新の派生プロジェクト config も捕まえる。summary 取得失敗・`diff --git` ヘッダ不在 (= 収録ファイルを特定 できない) も「網羅している」に倒さずエラーにする (ADR-043)。 - **`--git` 形式へ切替 (順位 264)**: 範囲検査がヘッダを読む要件に加え、jj 既定形式は 色を落とすと `+`/`-` が消えて LLM レビュアーが削除を追加と誤読する (PR #256 で simplicity-review が todo 25 行の削除を追加と誤読し false positive REJECT、約 19 分 浪費)。 - `templates/push-runner-config.toml` も同時修正 (deploy:hooks で派生配布されるため)。 ## 実測で見つけた副次バグ 範囲 summary の取得を当初 shell 経由 (`[diff] command` と同じ経路) で実装したが、 実シェルで叩いたところ **cmd.exe はクォートを除去せず jj に渡す**ため `Revision '"<base>..@"' doesn't exist` で必ず失敗した。sh は除去するので Linux だけ 通る = Windows の全 push が fail-closed で止まる形。ユニットテストは summary 取得を 注入していたため検出できなかった。direct args 呼び出し (docs_only_routing の既存 `run_jj_diff_summary`) を共有する形に修正し、doc に理由を残した。両 stage は 「PR 範囲の変更ファイル一覧」という同一の問いを扱うため、別実装にすると本 PR が 排除した非対称を再導入することになる。 ## 検証 cargo test --workspace 全 pass / clippy clean。範囲検査は incident 形状 (PR 2 ファイル 変更に対し diff は tip の 1 ファイルのみ) の再現テストで固定し、fail-closed 経路 (summary 取得失敗 / ヘッダ不在) と過剰検知しない経路 (空 PR 範囲 / Windows パス 区切りの正規化) も併せて固定した。base branch 解決は 3 stage が同一範囲に解決される ことと override の優先順位を machine-enforce している。
aloekun
added a commit
that referenced
this pull request
Jul 21, 2026
) * fix(push-runner): AI レビュー対象 diff を PR 全体に修正し範囲を機械検査する (順位 288/264) ## 問題 `[diff] command` に `jj diff -r @` が直書きされており、AI レビュアーには **tip コミットの diff しか渡っていなかった**。祖先コミットは pre-push のセルフレビューを 一度も経ずに merge される。 同じパイプライン内で `pr_size_check` と `docs_only_routing` は `<base>..@` (PR 範囲) を見ており、`[diff]` だけが非対称だった。実際 PR #311 では 695 行の PR に対して 37 行だけがレビュー対象になり、レビュアーは渡された 37 行を見て正しく「docs-only」と 判定していた。**レビュアー側からは「渡された diff が PR 全体か」を検証できない**ため、 この誤りは誰にも検知されない。 docs/todo-summary2.md 順位 288 として既知で、Severity High で PR #268/#300/#301 に 続き #311 が 4 回目の再発。 ## 変更 - **範囲の真実源を 1 箇所に**: top-level `default_branch` を新設し、`diff` / `docs_only_routing` / `pr_size_check` の 3 stage が `Config::resolve_base_branch` 経由で同じ値を使う。従来は section ごとに独立した `default_branch` を持ち、 「値を同期する義務」を config コメントで課していた (docs_only_routing.rs の doc に 明記されていた) が、義務はコード上の不変条件ではないため非対称を許していた。 section 側は後方互換の override として残す (派生プロジェクトの既存 config 対策)。 - **config から revset を排除**: `[diff] command` は `{{PR_RANGE}}` プレースホルダを 使う。push-runner が `<base>..@` に展開するため、狭い範囲を書く余地が無くなる。 - **範囲カバレッジ検査 (fail-closed)**: 生成した diff が PR 範囲の全変更ファイルを 含むか `jj diff --summary` と突き合わせ、不足があれば exit 5 で中断する。config の 書き方に依存せず「レビュー範囲 < PR 範囲」を検知できるため、未更新の派生プロジェクト config も捕まえる。summary 取得失敗・`diff --git` ヘッダ不在 (= 収録ファイルを特定 できない) も「網羅している」に倒さずエラーにする (ADR-043)。 - **`--git` 形式へ切替 (順位 264)**: 範囲検査がヘッダを読む要件に加え、jj 既定形式は 色を落とすと `+`/`-` が消えて LLM レビュアーが削除を追加と誤読する (PR #256 で simplicity-review が todo 25 行の削除を追加と誤読し false positive REJECT、約 19 分 浪費)。 - `templates/push-runner-config.toml` も同時修正 (deploy:hooks で派生配布されるため)。 ## 実測で見つけた副次バグ 範囲 summary の取得を当初 shell 経由 (`[diff] command` と同じ経路) で実装したが、 実シェルで叩いたところ **cmd.exe はクォートを除去せず jj に渡す**ため `Revision '"<base>..@"' doesn't exist` で必ず失敗した。sh は除去するので Linux だけ 通る = Windows の全 push が fail-closed で止まる形。ユニットテストは summary 取得を 注入していたため検出できなかった。direct args 呼び出し (docs_only_routing の既存 `run_jj_diff_summary`) を共有する形に修正し、doc に理由を残した。両 stage は 「PR 範囲の変更ファイル一覧」という同一の問いを扱うため、別実装にすると本 PR が 排除した非対称を再導入することになる。 ## 検証 cargo test --workspace 全 pass / clippy clean。範囲検査は incident 形状 (PR 2 ファイル 変更に対し diff は tip の 1 ファイルのみ) の再現テストで固定し、fail-closed 経路 (summary 取得失敗 / ヘッダ不在) と過剰検知しない経路 (空 PR 範囲 / Windows パス 区切りの正規化) も併せて固定した。base branch 解決は 3 stage が同一範囲に解決される ことと override の優先順位を machine-enforce している。 * docs: ADR-027 に「diff 局所 = 観点の限定であって範囲の限定ではない」を明記 (順位 288/264) ## ADR-027 amendment ADR-027 が狭めたのは reviewer が使う criteria (cross-file 探索を要求しない) であり、レビュー対象に含めるコミットの範囲ではなかった。この 2 つが混同され、 `[diff] command` が tip コミット限定のまま運用されて 4 回の再発を招いたため、 射程を明文化した。 「レビュー対象は PR 範囲全体」と決定した根拠も併記: - 速度は理由にならない。同一 PR でレビュー対象を 37 行 → 1011 行 (27 倍) に 広げても 4m32s → 4m43s の +11 秒。ADR-027 の速度改善は arch-review facet の 除去 (219-270s/iter) によるもので、範囲縮小は寄与していなかった。 - 範囲が狭いことによる見落としはレビュアー側から検知できない (渡された diff が PR 全体かを検証する手段が無い)。 - CodeRabbit backstop はセルフレビューを省く理由にならない。独立した層として併用する。 ## todo 更新 - 順位 264 (`--git` 切替): 完了につきエントリと table 行を削除。 - 順位 288: `[diff]` 範囲修正の部分のみ完了として記録。**残タスク** (post-merge feedback の全 run 集約、bookmark_check.rs の祖先未レビュー穴の検証) は明示して 残す。エントリ全体を消すと未着手部分が失われるため削除しない。 * fix(push-runner): pre-push 範囲検査の欠陥修正 + CodeRabbit 指摘5件対応 (#313)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
順位263（PR #243 post-merge-feedback T3-2 採用）の作業。「クロスシステム設定 coupling パターン」の汎化 ADR を起票する。
.coderabbit.yaml（外部 SaaS 側、CodeRabbit が server-side で読む）とpr-monitor-config.tomlの[fix] trigger_review_after_push（内部 CLI 設定）のように、内部設定と外部 SaaS 設定が論理的に coupled しているとき、片側だけの変更が silent breakage（再レビュー欠落 / 二重投稿）を招く。ランタイム cross-validation は原理的に不可能なため、文書化・規律が mitigation の中心になる。ADR-019 の CodeRabbit 固有記述を汎化し、今後の外部 SaaS 統合（LLM service / CI provider 等）へ横展開する。変更内容
docs/adr/adr-051-cross-system-config-coupling.md— 汎化 ADR（設計規律3点 + ランタイム検証の限界 + 適用対象/アンチパターン）docs/adr/adr-019-...md— 新 ADR への cross-reference を追記CLAUDE.md— ADR index に ADR-051 を追加docs/todo13.md/docs/todo-summary.md— 順位263 エントリを削除（完了）採番の経緯（重要）
本作業は元々未 push commit として滞留しており、当初 ADR-047 を採番していた。しかし滞留中に ADR-047（refute facet, #250）/ 048 / 049 / 050 が先行 land したため番号衝突が発生。空き番号 ADR-051 に振り直して再構成した。todo エントリ自身が指定していた「ADR-NNN placeholder（land 時に採番確定）」方式に従った形。
レビュー結果
jj diff -r @のレンダリングバグ由来の false positive（実コミットは todo 削除済みを--git/--stat/file showで確認済み）補足（別途 todo 化候補）
pre-push simplicity-review が本 diff で false positive REJECT を出した。真因は
.takt/review-diff.txtの diff フォーマット可読性:[diff]config はjj diff -r @（jj デフォルト）を使うが、この形式は追加/削除を色 + 行番号2列で表現するため、色を落としてファイル化すると-/+マーカーが無く、LLM レビュアーが「左列のみ行番号＝削除」を「追加」と誤読しうる（今回まさに todo 削除を追加と誤読）。jj diff --git -r @（標準 unified+/-）へ切替で解消できる（本 PR scope 外、harness 改善候補）。