Merged
Conversation
Co-authored-by: HappyOnigiri <nodemeld+happy-onigiri@gmail.com>
|
Cursor Agent can help with this pull request. Just |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughトップレベル設定 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/auto_fixer.py (2)
2633-2649:⚠️ Potential issue | 🟠 Major失敗したフェーズの実行レポートが PR コメントに残りません。
_capture_state_comment_report()が_run_claude_prompt()成功後にしか呼ばれていないうえ、例外時はそのまま再送出しているので、Claude が途中で失敗したフェーズは CI ログにしか出ずreport_blocks/ state comment に保存されません。実行レポート機能の中でも一番見たい失敗ケースが欠落します。Also applies to: 2728-2748, 2970-2981, 3124-3126
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/auto_fixer.py` around lines 2633 - 2649, The exception path currently re-raises before saving the execution report so failed Claude phases are never recorded; ensure _capture_state_comment_report(report_blocks, "ci-fix", ci_report_path) is invoked even on failure by moving the call into a finally block or by calling it from the except handler before re-raising the exception for the ci-fix section around _run_claude_prompt (symbols: ci_commits, _run_claude_prompt, _capture_state_comment_report, report_blocks, ci_report_path); apply the same fix pattern to the other similar blocks referenced (the blocks around the other _run_claude_prompt calls at the ranges you noted) so every phase captures its state comment/report before the exception is propagated.
3097-3122:⚠️ Potential issue | 🟠 Majorこの state comment 更新ロジックは既存レポートを消しつつ、失敗ケースを完了扱いにできます。
Line 3097 で
execution_report_enabledがFalseのときreport_body_to_save = ""にしており、Line 3109 でそれをupsert_state_comment()に渡すと、src/state_manager.pyの実装上は既存report_bodyを保持せず空で上書きします。さらに Line 3104-3115 ではstate_entriesが 0 件でもレポート差分だけでshould_write_state_commentが真になり、更新成功後にstate_saved = Trueになります。インライン comment の resolve が失敗してany_comment_failed=Trueでも、_update_done_label_if_completed()側で完了扱いに進める経路ができています。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/auto_fixer.py` around lines 3097 - 3122, The logic overwrites existing report text when execution_report_enabled is False and can mark state as completed incorrectly; change the assignment and comparison so we only attempt to update the report when reporting is enabled: keep report_body_to_save as state_comment.report_body.strip() (or None) instead of "" when execution_report_enabled is False, and only include the report-diff check in should_write_state_comment if execution_report_enabled is True; ensure upsert_state_comment is only called when should_write_state_comment is true and only set state_saved = True after a successful upsert (and not merely because the report string changed while reporting is disabled). Reference variables/functions: report_body_to_save, execution_report_enabled, state_comment.report_body, should_write_state_comment, upsert_state_comment, any_comment_failed, and _update_done_label_if_completed.
🤖 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/auto_fixer.py`:
- Around line 618-629: Replace the English "no report was recorded." messages
with the required Japanese text "レポート出力なし" for both the missing-file and
empty-content cases: update the print calls guarded by report_file.exists() (the
branch that currently prints at the check if not report_file.exists()) and the
subsequent check if not content (after reading report_file.read_text()) so they
write "レポート出力なし" to sys.stderr; keep the existing exception handling and stderr
target intact.
---
Outside diff comments:
In `@src/auto_fixer.py`:
- Around line 2633-2649: The exception path currently re-raises before saving
the execution report so failed Claude phases are never recorded; ensure
_capture_state_comment_report(report_blocks, "ci-fix", ci_report_path) is
invoked even on failure by moving the call into a finally block or by calling it
from the except handler before re-raising the exception for the ci-fix section
around _run_claude_prompt (symbols: ci_commits, _run_claude_prompt,
_capture_state_comment_report, report_blocks, ci_report_path); apply the same
fix pattern to the other similar blocks referenced (the blocks around the other
_run_claude_prompt calls at the ranges you noted) so every phase captures its
state comment/report before the exception is propagated.
- Around line 3097-3122: The logic overwrites existing report text when
execution_report_enabled is False and can mark state as completed incorrectly;
change the assignment and comparison so we only attempt to update the report
when reporting is enabled: keep report_body_to_save as
state_comment.report_body.strip() (or None) instead of "" when
execution_report_enabled is False, and only include the report-diff check in
should_write_state_comment if execution_report_enabled is True; ensure
upsert_state_comment is only called when should_write_state_comment is true and
only set state_saved = True after a successful upsert (and not merely because
the report string changed while reporting is disabled). Reference
variables/functions: report_body_to_save, execution_report_enabled,
state_comment.report_body, should_write_state_comment, upsert_state_comment,
any_comment_failed, and _update_done_label_if_completed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6cc4b717-1ba3-47ce-8bd5-04fb99e451fb
📒 Files selected for processing (7)
.refix.yaml.sampleREADME.ja.mdREADME.mdsrc/auto_fixer.pysrc/state_manager.pytests/test_auto_fixer.pytests/test_state_manager.py
_run_claude_prompt() が例外を送出した場合でも _capture_state_comment_report() が呼ばれるよう、ci-fix・merge-conflict-resolution・review-fix の各フェーズで finally ブロックに移動した。
execution_report_enabled が False のとき report_body_to_save が "" になり既存の report_body を上書きしていた問題を修正。state_comment.report_body.strip() を維持 するよう変更し、should_write_state_comment のレポート差分チェックも execution_report_enabled が True の場合のみ評価するよう修正した。
Owner
Author
🤖 Refix Status処理済みレビュー一覧 (System Use Only)
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/auto_fixer.py`:
- Around line 2781-2787: Race condition: before calling
_persist_state_comment_report_if_changed (which ultimately calls
upsert_state_comment that replaces report_body), re-fetch the latest
state_comment.report_body from the source of truth and re-run the merge to
incorporate concurrent updates so we don't overwrite other runs' appended
reports. Concretely, in the block around merged_report_body =
_merge_state_comment_report_body(...), call the same reader used to obtain
state_comment (or a get_state_comment/get_latest_state_comment function) to
obtain the current report_body, re-merge that latest body with report_blocks
(e.g. via _merge_state_comment_report_body) to produce a final
merged_report_body, then pass that into
_persist_state_comment_report_if_changed; apply the same fix to the other
occurrences referenced (around the blocks at ~2852-2858 and ~3101-3119).
- Around line 2649-2650: The finally block currently only calls
_capture_state_comment_report(report_blocks, "ci-fix", ci_report_path) which
only appends to report_blocks but does not persist to GitHub because exceptions
are re-raised before upsert_state_comment runs; update the flow so the
accumulated report is persisted even on failure—either call
upsert_state_comment(report_blocks, ...) inside this finally (or in the outer
_process_single_pr() finally) before re-raising, or move the persistence logic
into _capture_state_comment_report so it both captures and calls
upsert_state_comment; target references: _capture_state_comment_report,
upsert_state_comment, _process_single_pr, and report_blocks, and apply the same
change to the other occurrences noted (around the ci-fix /
merge-conflict-resolution / review-fix finally blocks).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca2d3640-bf33-4383-864e-639a4436c8d3
📒 Files selected for processing (1)
src/auto_fixer.py
The source was changed to output "レポート出力なし" in 5a52687 but the test was not updated accordingly.
ci-fix / merge-conflict-resolution / review-fix の各フェーズで例外が発生した際、 _capture_state_comment_report が finally で report_blocks に追加するだけで upsert_state_comment が呼ばれないまま例外が再送出されていた問題を修正。 - ci-fix: finally を削除し、except で capture + 保存後に raise、成功時は try/except 後で capture - merge-conflict-resolution: 同様のパターンを適用 - ClaudeCommandFailedError / subprocess.CalledProcessError: 再送出・終了前に保存ロジックを追加 いずれも load_state_comment で最新状態を再取得してから _persist_state_comment_report_if_changed を呼び出すため、後述の race condition 対策も兼ねている。
_persist_state_comment_report_if_changed や upsert_state_comment を呼ぶ直前に load_state_comment で最新状態を再取得し、古い report_body への上書きによる 別 run のレポート消去リスクを低減する。 対象箇所: has_review_targets=False パス、skip_review_fix パス、 および通常完了時の report_body_to_save 構築（3 か所）。
HappyOnigiri
deleted the
cursor/-bc-cc2d5d07-d935-42ff-a224-d30ec9dc8c07-b4b0
branch
HappyOnigiri
added
refix:auto-merge-requested
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
2 participants
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
Refix が PR に出力するステータス管理コメントと実行レポートの表示・管理方法を改善します。
変更内容 / Changes
(System Use Only)を削除しました。execution_report設定を追加し、レポート機能の ON/OFF を制御できるようにしました（デフォルト OFF）。OFF の場合は Claude へのレポート指示も行いません。README.md,README.ja.md,.refix.yaml.sampleにexecution_report設定の説明を追加しました。テスト / Testing
pytest tests/test_state_manager.py tests/test_auto_fixer.py -qで変更点近傍のテストを実行し、回帰がないことを確認済み。make ciを実行し、lint、型検査、全テストがパスすることを確認済み。関連 issues / Related issues
なし
Summary by CodeRabbit
新機能
ドキュメント
テスト