Skip to content

docs(todo): PR #268/#269 post-merge feedback 採用 4 件を順位 288-291 に登録 #270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aloekun merged 2 commits into from
Jul 13, 2026
Merged

docs(todo): PR #268/#269 post-merge feedback 採用 4 件を順位 288-291 に登録 #270

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5f9ff8
docs(todo): PR #268/#269 post-merge feedback 採用 4 件を順位 288-291 に登録
aloekun Jul 13, 2026
daf0de5
fix(review): apply CodeRabbit fixes for #270
aloekun Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/todo-summary.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@
| 285 | 🔧 Tier 2 | **jj keyword を含む commit message の tokenization edge-case テスト (PR #267 post-merge-feedback T2-2 採用)** | todo13.md | S | なし (順位 283 と表裏。283 の着手有無に関わらず現行挙動を regression test で固定する価値が独立して残る。283 と同一 PR 消化が効率的) |
| 286 | 🔧 Tier 2 | **config path 解決の cwd 跨ぎ integration test (PR #267 post-merge-feedback T2-3 採用)** | todo13.md | M | なし (FIXED 済 cwd-config bug の regression guard。既存テストは pure parser のみで file-lookup 経路未カバー。Severity High、Adoption Risk = OS 依存) |
| 287 | 💎 Tier 3 | **「config 読み hook は exe-relative 解決必須」convention の明文化 (PR #267 post-merge-feedback T3-1 採用)** | todo13.md | XS | なし (順位 281 の文書層補完。**281 と同一 PR bundle 推奨**、別作業に切り出す価値は低い) |
| 288 | 🔧 Tier 2 | **post-merge feedback の pre-push reports を対象 PR の全 run 集約に拡張 (PR #268 post-merge-feedback T2-1 採用)** | todo13.md | M | なし (「最新 1 run」参照は複数 push した PR で分析が最終 push 分に偏る。PR #267 feedback の evidence-scope 注記で実観測。context の prepush_reports_dir 配列化 + facet 複数 dir 対応。独立 PR 推奨) |
| 289 | 💎 Tier 3 | **run_feedback_only の docstring 修正 — 検出失敗パスは marker を書かない旨を明記 (PR #268 post-merge-feedback T3-1 採用)** | todo13.md | XS | なし (現行 docstring「marker は通常経路と同様に残る」と実装 (owner_repo 失敗パスは marker なし = 意図的) の drift 修正。**順位 280 PR に同乗推奨**) |
| 290 | 🚀 Tier 1 | **cli-push-runner の bookmark 検出を ::@ (自 workspace 祖先) に限定 (PR #269 post-merge-feedback T1-1 採用)** | todo13.md | S | なし (jj bookmark list はリポジトリ全体対象のため並行 workspace の bookmark も -b 付与対象に含む。実 push で複数 bookmark 付与を実観測、feedback と dogfood が独立に同一検出。--all 廃止の仕上げ。**順位 280 PR で消化予定**) |
| 291 | 🔧 Tier 2 | **run_ai_step_for の Result 伝播 regression test (PR #269 post-merge-feedback T2-1 採用)** | todo13.md | S | なし (SIM-NEW-pipeline-L224 = path.exists() 偽陽性 PASS の修正を両呼び出し元で固定。stale report 存在下の再実行失敗が exit 0 にならないことを assert。順位 280 PR 同乗可) |

**戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で ADR-032 の前提 + rate-limit + convergence cost 削減を進める → Tier 3 で ADR-032 を land + ドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。

Expand Down
77 changes: 77 additions & 0 deletions docs/todo13.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,83 @@

---

### post-merge feedback の pre-push reports を対象 PR の全 run 集約に拡張 (PR #268 post-merge-feedback T2-1 採用)

> **動機**: `find_latest_prepush_reports_dir()` は「最新 1 run」のみを feedback の分析ソースにするため、複数回 push した PR では最後の push 分に分析が偏る。PR #267 の feedback でも「参照した pre-push run は WP-11 status 更新 (docs-only の最終 push) のみが対象」という evidence-scope 注記が付いた実観測あり。対象 PR の commit 範囲内の全 pre-push-review run を集約し、`post-merge-feedback-context.json` の `prepush_reports_dir` を配列化、`analyze-prepush-reports.md` facet も複数 dir 対応に更新する。
>
> **参照**: `.claude/feedback-reports/268.md` Tier 2 #1、`src/cli-merge-pipeline/src/feedback/context.rs` (`find_latest_prepush_reports_dir`)、`.takt/facets/instructions/analyze-prepush-reports.md`
>
> **実行優先度**: 🔧 Tier 2 — Effort M。context スキーマ変更 + facet 更新 + テストを伴うため独立 PR 推奨。

#### 作業計画

- [ ] 対象 PR の時刻 range 内の pre-push run dir を列挙する関数に拡張 (`.takt/runs/*-pre-push-review`)
- [ ] context json の `prepush_reports_dir` を配列化 + facet instruction を複数 dir 対応に
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- 複数 push した PR の feedback が全 pre-push run のレポートを分析対象にすること。

@coderabbitai coderabbitai Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

時刻範囲だけで対象PRのrunを集約しないでください。

同じ時間帯に並行PRのpre-push runが存在すると、対象PRに紐づかないレポートまで取り込む可能性があります。PR番号、bookmark、commit IDなどの安定した関連付け条件を明記し、並行PRのrunが混在する回帰テストも完了基準に追加してください。

🤖 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/todo13.md` around lines 1061 - 1067, Update the TODO entry describing
pre-push run discovery to require stable association with the target PR, such as
PR number, bookmark, or commit ID, rather than relying only on the time range.
Extend the completion criteria to include a regression test proving concurrent
PR runs are excluded while all relevant runs for the target PR are analyzed.


---

### run_feedback_only の docstring 修正 — 検出失敗パスは marker を書かない旨を明記 (PR #268 post-merge-feedback T3-1 採用)

> **動機**: 現行 docstring は「失敗時 (marker は通常経路と同様に残る)」と記すが、owner_repo 検出/validation 失敗パスでは marker を書かない (同期 CLI で人間が直接ログを見るため意図的)。spec-impl drift の芽を摘む。
>
> **参照**: `.claude/feedback-reports/268.md` Tier 3 #1、`src/cli-merge-pipeline/src/pipeline.rs` (`run_feedback_only` docstring)
>
> **実行優先度**: 💎 Tier 3 — Effort XS。**順位 280 の実装 PR に同乗推奨** (cli-merge-pipeline を触る同一 PR で消化)。

#### 作業計画

- [ ] docstring の終了コード契約の記述を実装に合わせて修正
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- docstring と実装の marker 挙動が一致していること。

---

### cli-push-runner の bookmark 検出を `::@` (自 workspace 祖先) に限定 (PR #269 post-merge-feedback T1-1 採用)

> **動機**: bookmark_check の検出 (`jj bookmark list`) はリポジトリ全体を対象とするため、並行 workspace の bookmark も拾い、push の `-b` 付与対象に含めてしまう。本セッションの実 push で `-b <PR#268の bookmark> -b <PR#269の bookmark>` と複数 bookmark が付与された実観測あり (両方自分のもので無害だったが、並行 workspace では他者の作業中 bookmark を巻き込む余地)。`--all` 廃止 (PR #267) の仕上げとして、検出を `::@ ~ trunk()` 等の revset で自 workspace の祖先に限定する。feedback pipeline とセッション内 dogfood が独立に同一問題を検出 (相互裏付け)。
>
> **参照**: `.claude/feedback-reports/269.md` Tier 1 #1、`src/cli-push-runner/src/stages/bookmark_check.rs`、ADR-045 § Known operational risks (bookmark conflicts)
>
> **実行優先度**: 🚀 Tier 1 — Effort S。**順位 280 の実装 PR で消化予定** (並列安全化の仕上げとして同一テーマ)。

#### 作業計画

- [ ] bookmark 検出を revset ベース (`jj log -r 'bookmarks() & ::@ ~ trunk()'` 等) に変更 + テスト
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- push の `-b` 付与対象が自 workspace の祖先にある bookmark に限定されること。

@coderabbitai coderabbitai Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

::@ だけでは「自workspace所有」の保証になりません。

bookmarks() & ::@ ~ trunk() はコミット祖先を絞る条件であり、並行workspaceのbookmarkが共有祖先やマージ済み履歴上にある場合まで除外できるとは限りません。「自workspace」の定義を明確化し、そのケースを含むテストを追加してください。

🤖 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/todo13.md` around lines 1090 - 1105, Clarify the ownership definition
used by the bookmark detection plan in docs/todo13.md: ancestor membership via
`bookmarks() & ::@ ~ trunk()` alone must not be treated as proof that a bookmark
belongs to the current workspace. Specify the repository/revset mechanism that
identifies current-workspace bookmarks, and add tests covering bookmarks on
shared ancestors or merged history from concurrent workspaces, ensuring push
targets include only bookmarks owned by the current workspace.


---

### run_ai_step_for の Result 伝播 regression test (PR #269 post-merge-feedback T2-1 採用)

> **動機**: PR #268 の pre-push review で REJECT された `path.exists()` 偽陽性 PASS (SIM-NEW-pipeline-L224) の修正 (`Result<PathBuf, String>` の直接伝播) を、両呼び出し元 (`run_feedback_only` / `run_ai_step`) で regression test として固定する。stale report 存在下での再実行失敗が exit 0 にならないことの検証が核心。
>
> **参照**: `.claude/feedback-reports/269.md` Tier 2 #1、`src/cli-merge-pipeline/src/pipeline.rs` (`run_ai_step_for`)
>
> **実行優先度**: 🔧 Tier 2 — Effort S。順位 280 の実装 PR に同乗可 (cli-merge-pipeline を触る場合)。

#### 作業計画

- [ ] Result 伝播の unit/regression test を追加 (stale report + Err ケースで exit 1 を assert)
- [ ] 本エントリ削除 + todo-summary.md 行削除

#### 完了基準

- 偽陽性 PASS バグの再発がテストで検出されること。

---

## 既知課題 (記録のみ、本セッションで未対応)

(現時点で本ファイルへの既知課題は無し。docs/todo10.md / todo9.md 末尾を参照。)
Loading