feat(post-merge-feedback): takt workflow + 4 facets で L1 Floor を実装 (ADR-030 Phase B)#77
Merged
feat(post-merge-feedback): takt workflow + 4 facets で L1 Floor を実装 (ADR-030 Phase B)#77
Conversation
…DR-030 Phase B)
ADR-030 で起案した決定論的 post-merge-feedback 機構の L1 Floor を実装する。
旧 ADR-029 の pending file → Stop hook → skill 経路を、cli-merge-pipeline が
takt workflow を同期 spawn する経路に置き換えた。silent loss が起きていた
層 3-4 (session lifecycle 依存) を Rust の決定論実行で取り除く。
主な変更:
- .takt/workflows/post-merge-feedback.yaml: 4 facets 順次 chain (loop_monitors なし)
- .takt/facets/instructions/{analyze-pr,analyze-session,analyze-prepush-reports,aggregate-feedback}.md
- src/cli-merge-pipeline/src/feedback.rs (新規): transcript 時刻 range filter
+ context handoff + takt spawn + report copy + failure marker
- src/cli-merge-pipeline/src/main.rs: run_ai_step を pending file 書込から
feedback::run に置換
- 旧 pending_file module は #![allow(dead_code)] で Phase E まで残置
- .gitignore に .claude/feedback-reports/ + .takt/post-merge-feedback-* を追加
- docs/todo.md: Phase B 完了に伴い該当セクションを削除
Phase C (UserPromptSubmit hook L2 Recovery) は別 PR で実装する。
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughADR-030に基づき、ペンディングファイル方式を廃止して、決定論的な同期taktワークフロー（PR解析・セッション解析・プレプッシュ解析・集約）を追加し、CLIから直接実行・タイムアウト管理・成果物の配置を行う統合実装を導入しました。 Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (feedback.rs)
participant GH as GitHub CLI
participant FS as File System
participant TAKT as Takt Workflow
participant ANA1 as analyze-pr
participant ANA2 as analyze-session
participant ANA3 as analyze-prepush
participant AGG as aggregate-feedback
CLI->>GH: gh pr view -> first_commit_time, mergedAt
GH-->>CLI: 時刻範囲
CLI->>FS: transcript source dir を読み込み
FS-->>CLI: JSONL 行
CLI->>CLI: 範囲内の user/assistant をフィルタ
CLI->>FS: .takt/post-merge-feedback-transcript.jsonl に書込
CLI->>FS: 最新の *-pre-push-review/reports/ を検出
FS-->>CLI: prepush_reports_dir
CLI->>FS: .takt/post-merge-feedback-context.json を生成
CLI->>TAKT: takt -w post-merge-feedback を実行
activate TAKT
TAKT->>ANA1: PR解析を実行
ANA1->>FS: PR diff / コメント / メタデータ取得
ANA1-->>TAKT: pr-analysis.md
TAKT->>ANA2: セッション解析を実行
ANA2->>FS: フィルタ済みトランスクリプト読み込み
ANA2-->>TAKT: session-analysis.md
TAKT->>ANA3: プレプッシュ解析を実行
ANA3->>FS: pre-push レポート読み込み
ANA3-->>TAKT: prepush-analysis.md
TAKT->>AGG: 集約を実行
AGG->>FS: 3レポート読み込み・マッチング・統合
AGG-->>TAKT: feedback-report.md
deactivate TAKT
TAKT-->>CLI: 完了
CLI->>FS: 最新実行の reports/feedback-report.md を .claude/feedback-reports/<pr>.md にコピー
CLI->>FS: <pr>.md.failed を削除（成功時）
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli-merge-pipeline/src/main.rs (1)
636-640:⚠️ Potential issue | 🟡 Minor警告メッセージが旧経路の文言のまま残っている。
ADR-030 では post_steps の AI ステップは pending file を書かず、takt workflow を同期実行するよう切り替わっている。
pending file を書き込めずスキップという記述は実装と乖離しており、運用時のログ追跡で誤解を招く。📝 提案する修正 diff
let owner_repo = detect_owner_repo(); if owner_repo.is_none() { log_info( - "警告: owner_repo を検出できませんでした (gh repo view 失敗)。post_steps の AI ステップは pending file を書き込めずスキップします。", + "警告: owner_repo を検出できませんでした (gh repo view 失敗)。post_steps の AI ステップ (post-merge-feedback workflow) はスキップします。", ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli-merge-pipeline/src/main.rs` around lines 636 - 640, owner_repo が None のときに出す警告メッセージが旧仕様（pending file を書き込めずスキップ）を伝えているので、log_info 呼び出しのメッセージ文言を ADR-030 に合わせて更新してください（参照: owner_repo 変数と log_info 関数呼び出し）—具体的には「post_steps の AI ステップは pending file を書き込まず、takt ワークフローを同期実行する」旨に差し替え、誤解を招かない短い日本語メッセージにしてください。
🧹 Nitpick comments (1)
src/cli-merge-pipeline/src/feedback.rs (1)
383-611:#[cfg(test)] mod testsは通常ファイル末尾に置くのが慣例。現在
mod tests(383-611) が public エントリポイントrun(617-664) より前に置かれており、ファイルを開いたときに「テストの海」を越えないと本体エントリに辿り着けない。Rust の慣例どおりrunの後（ファイル末尾）へ移動すると読みやすさが大きく改善する。挙動には影響しない並べ替えのみ。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli-merge-pipeline/src/feedback.rs` around lines 383 - 611, テストモジュール (#[cfg(test)] mod tests { ... }) が公開エントリポイント run の前に置かれているため、ファイル末尾（run 関数の後）に移動して可読性を改善してください；具体的にはファイル内の mod tests ブロック全体を切り取り、run 関数（run の定義と終了括弧）を見つけた後ろ、ファイルの末尾に貼り付けてテストモジュールを配置し、既存のインポートやスコープに影響がないことを確認してください。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli-merge-pipeline/src/feedback.rs`:
- Around line 624-639: The current use of unwrap_or(0) when calling
filter_transcripts hides I/O errors (e.g. File::create failures) — change the
match to handle the Result explicitly: call filter_transcripts(transcript_dir,
&range, &transcript_path) and on Err e, emit a clear error log including e and
transcript_path (eprintln! or the crate's logger) and propagate the error back
to the caller (return Err(...)) instead of turning it into 0; ensure the
surrounding function's signature is adjusted to return Result so main.rs can
convert the error into a concrete write_failed_marker reason; references:
filter_transcripts, transcript_path, input.transcript_source_dir.
- Around line 215-220: The current string comparison between timestamp and
range.first_commit_time / range.merged_at can misorder millisecond vs
second-only formats; instead parse all three values into chrono::DateTime (e.g.,
using chrono::DateTime::parse_from_rfc3339 or parse_from_rfc3339 then
.with_timezone(&Utc)) and perform DateTime comparisons; update the block that
reads timestamp and the final comparison (the variables timestamp,
range.first_commit_time, range.merged_at) to handle parse errors (return false
on parse failure) and compare the parsed DateTime values rather than raw
strings.
---
Outside diff comments:
In `@src/cli-merge-pipeline/src/main.rs`:
- Around line 636-640: owner_repo が None のときに出す警告メッセージが旧仕様（pending file
を書き込めずスキップ）を伝えているので、log_info 呼び出しのメッセージ文言を ADR-030 に合わせて更新してください（参照: owner_repo
変数と log_info 関数呼び出し）—具体的には「post_steps の AI ステップは pending file を書き込まず、takt
ワークフローを同期実行する」旨に差し替え、誤解を招かない短い日本語メッセージにしてください。
---
Nitpick comments:
In `@src/cli-merge-pipeline/src/feedback.rs`:
- Around line 383-611: テストモジュール (#[cfg(test)] mod tests { ... }) が公開エントリポイント run
の前に置かれているため、ファイル末尾（run 関数の後）に移動して可読性を改善してください；具体的にはファイル内の mod tests
ブロック全体を切り取り、run 関数（run
の定義と終了括弧）を見つけた後ろ、ファイルの末尾に貼り付けてテストモジュールを配置し、既存のインポートやスコープに影響がないことを確認してください。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73c71a82-7c21-403c-a97c-a6a2efd197db
📒 Files selected for processing (10)
.gitignore.takt/facets/instructions/aggregate-feedback.md.takt/facets/instructions/analyze-pr.md.takt/facets/instructions/analyze-prepush-reports.md.takt/facets/instructions/analyze-session.md.takt/workflows/post-merge-feedback.yamldocs/todo.mdsrc/cli-merge-pipeline/src/feedback.rssrc/cli-merge-pipeline/src/main.rssrc/cli-merge-pipeline/src/pending_file.rs
💤 Files with no reviewable changes (1)
- src/cli-merge-pipeline/src/pending_file.rs
Resolved findings: - [Minor] src/cli-merge-pipeline/src/feedback.rs:220 境界付近で精度差による取りこぼしが起き得る（minor）。 - [Major] src/cli-merge-pipeline/src/feedback.rs:639 `filter_transcripts` のエラーが握り潰されている。
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
7 tasks
aloekun
added a commit
that referenced
this pull request
Apr 25, 2026
* fix(post-merge-feedback): task labeling 規約導入で run dir 検索を決定論化 PR #77 の merge dogfood で発覚した `copy_feedback_report` の bug を修正する。 takt の run dir 命名は `<ts>-<sanitized-task-label>` 形式で workflow 名そのものは 含まないため、`ends_with("-<workflow>")` での suffix match が context suffix (例: `-for-77`) 付きの dir に対して fail していた。 対策: 「task label は workflow 名を必ず prefix として含む」という命名規約を ADR-030 に追記し、Rust 側のマッチングを `name.contains("-<workflow>")` の 1 ルールに統一。context suffix の有無に関わらず一律にマッチする。 主な変更: - src/cli-merge-pipeline/src/feedback.rs: find_latest_run_dir を contains match に変更、TAKT_TASK_PREFIX を workflow 名と sanitization 後一致する形に 修正、新規テスト 5 件追加 - src/cli-pr-monitor/src/config.rs + pr-monitor-config.toml + templates/pr-monitor-config.toml: post-pr-review の task を "analyze PR review comments" → "post-pr-review" に揃え、latent bug を予防 - docs/adr/adr-030-deterministic-post-merge-feedback.md: §task labeling convention セクション追加。規約・制約 (workflow 名同士の部分文字列禁止)・ 採用根拠を明記 Phase B 内の bugfix として完結し、Phase C には bug を持ち越さない。 * docs(adr-030): 制約例を実装と一致させ、partial substring の取り違えを明示 CodeRabbit (PR #78) が指摘した不整合を修正。 旧記述: "merge と post-merge-feedback は OK" は誤り。 - find_latest_run_dir は name.contains("-{workflow}") で照合 - workflow=merge の needle "-merge" は "<ts>-post-merge-feedback-..." に 内部出現するため誤マッチが起きる - したがって merge も post-merge と同様 NG 修正内容: - 「部分文字列関係」の定義を実装メソッド (name.contains) と紐付けて明記 - NG 例を 2 件 (merge, post-merge) と OK 例 (build) を併記 - 実装ファイル feedback.rs へのリンクを追加
6 tasks
aloekun
added a commit
that referenced
this pull request
Apr 26, 2026
) Phase B dogfood で発見されたタイムアウト経路の残存不具合と、ボトルネック分析で 判明した sequential 構造を併せて修正する。 ## 主な変更 ### 1. workflow の並列化 (.takt/workflows/post-merge-feedback.yaml) 3 つの analyze facet (analyze-pr / analyze-session / analyze-prepush-reports) は 独立した情報源を扱うため parallel block で並列実行する。 PR #78 計測 (sequential 12m13s) → parallel 想定 ~7m30s に短縮。 ### 2. TAKT_TIMEOUT_SECS: 600s → 1200s PR #77 (14m21s)、#78 (12m13s) 観測実績を踏まえた暫定値。並列化後の想定 7m30s に対し 2x の安全係数。docstring に「band-aid」と明記し、本質解は workflow 構造で 時間を抑えることだと残した。 ### 3. caller-side reconciliation (run() 末尾) Windows の child.kill() が takt の descendants を殺せず、Rust が timeout で kill した後も takt が orphan として走り続けて report を完成させるケースを観測した (PR #78 で kill 後 2m13s で feedback-report.md 完成)。 run_takt_workflow の戻り値に関わらず copy_feedback_report を必ず試行し、report が 存在すれば success 扱いとする。既存の .failed marker は cleanup_failed_marker で 除去する。 ### 4. 並行起動 guard (CONCURRENT_RUN_GUARD_SECS = 1500s) cross-invocation context overwrite race を発生源で予防。 .takt/post-merge-feedback-context.json が 1500s 以内に書かれていれば、orphan takt が走行中の可能性が高いとみなして新規実行を refuse する。 ### 5. テスト追加 (5 件) concurrent_run_guard / context_age_secs の振る舞いを検証。 ### 6. ADR-030 更新 - レイテンシモデル (parallel 想定 = max(analyze) + aggregate) - Reconciliation 設計の根拠 - 並行起動 guard の意図と Phase B 外切り出し方針 ## 残存リスク (Phase B 外) - Windows の job object でプロセスツリー全体を kill する根本対策 - 完全な per-invocation isolation (takt context dir 連携)
9 tasks
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.
Summary
ADR-030 で起案した決定論的 post-merge-feedback 機構の L1 Floor (Phase B) を実装する。旧 ADR-029 の
pending file → Stop hook → skill経路を、cli-merge-pipelineが takt workflow を同期 spawn する経路に置き換え、silent loss を取り除く。post-merge-feedbackをcli-merge-pipelineから同期実行 (session lifecycle 非依存の決定論実行)analyze-pr→analyze-session→analyze-prepush-reports→aggregate-feedback.takt/post-merge-feedback-context.json+.takt/post-merge-feedback-transcript.jsonl(Rust が事前 filter).claude/feedback-reports/<pr>.md、失敗時は<pr>.md.failedmarker (soft fail、merge は成功扱い)L2 Recovery (UserPromptSubmit hook) は Phase C で別 PR として実装する。
主な変更
.takt/workflows/post-merge-feedback.yaml: 4 facet を順次 chain (loop_monitors なし — コードではなくレポート生成のため fix loop は不要).takt/facets/instructions/{analyze-pr,analyze-session,analyze-prepush-reports,aggregate-feedback}.md: 旧/analyze-prskill +/post-merge-feedbackskill Phase 4 から portsrc/cli-merge-pipeline/src/feedback.rs(新規): transcript 時刻 range filter + context handoff + takt spawn + report copy + failure markersrc/cli-merge-pipeline/src/main.rs:run_ai_stepを pending file 書込からfeedback::runに置換src/cli-merge-pipeline/src/pending_file.rs: 削除 (takt-fix の simplification 提案を受け入れ、Phase E の dead code 削除を前倒し).gitignore:.claude/feedback-reports/+.takt/post-merge-feedback-*を除外docs/todo.md: Phase B 該当セクションを削除 (運用ルール「完了タスクは ADR/仕組みに反映後に削除」)設計判断
pass_previous_response: false:aggregate-feedbackは前 step の response ではなく Report Directory 経由で 3 つの先行レポートを Read する (supervise.mdパターンに揃える)gitBranchはHEAD固定のため、時刻 range filter ([first_commit_time, merged_at]) が唯一の確定的経路 (Phase 0 で実証済み)cli-merge-pipelineは exit 0。merge 完了済の PR を巻き戻せないため、.failedmarker + L2 recovery (Phase C で実装) で取りこぼしを拾うfeedback-report.mdを Rust が.claude/feedback-reports/<pr>.mdにコピーする (ADR-022「機械的 = Rust」原則に整合、facet は出力先 convention を知らない)Test plan
pnpm exec takt prompt post-merge-feedbackで workflow YAML が parse 成功dogfood (Phase F) は Phase C の L2 Recovery 実装後の数回マージで検証する。
References
Summary by CodeRabbit
新機能
ドキュメント
その他