refactor(src): execution_report を廃止し write_result_to_comment(stdout → PR コメント)に置き換え#82
Conversation
|
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🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/auto_fixer.py (1)
343-373:⚠️ Potential issue | 🟠 Major失敗した Claude 実行の出力が result log に残りません。
src/claude_runner.py:83-205は失敗時もstdout/stderrを例外に保持していますが、ここでは成功時にしかresult_blocksに追加していません。最初のフェーズで失敗するとresult_blocksが空のままなので、state comment に何も残らず、今回追加したwrite_result_to_commentの主目的を満たせません。少なくともClaudeCommandFailedError/ClaudeUsageLimitErrorのstdout/stderrからブロックを作ってからupsert_state_comment()する必要があります。Also applies to: 456-488, 573-595, 734-766
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/auto_fixer.py` around lines 343 - 373, The CI-fix exception handler currently doesn't persist failed Claude outputs into result_blocks, so when run_claude_prompt (used in the ci-fix phase) raises (e.g., ClaudeCommandFailedError / ClaudeUsageLimitError) the stdout/stderr carried by the exception are lost; update the except block around run_claude_prompt to detect if the caught exception exposes stdout/stderr, create a phase result block via build_phase_result_entry("ci-fix", stdout_or_stderr, ctx.state_comment_timezone) and append it to result_blocks (merging with load_state_comment as done now) before calling upsert_state_comment, and apply the same change to the equivalent handlers for the other phases (the similar try/except regions referenced at the other locations) so failed-run outputs are always saved to the state comment.
🤖 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 1140-1156: The upsert_state_comment failure is currently being
treated as a success path because state_saved isn't reliably updated on error;
initialize and keep state_saved = False before the write attempt, set
state_saved = True only inside the successful upsert_state_comment try block,
and in the except block explicitly leave it False (or set to False) so
downstream cacheability/CI-only flow (the code that decides skip/complete for
CI-only PRs) reads the correct failure state; update any subsequent cacheability
check that references state_saved to use this flag so failed writes won't be
treated as saved.
In `@src/result_report.py`:
- Around line 30-38: The current static triple-backtick fence in
result_report.py breaks the report when stdout_text contains ```; update the
stdout rendering to compute a fence string that does not appear in the output
(e.g., start with "`" and append backticks while the fence is found in
stdout_text/stripped_stdout), then use that dynamic fence variable instead of
the fixed "```" when building the lines list (refer to variables/fields fence,
stdout_text, stripped_stdout and the list construction around lines.extend that
adds the "<summary>stdout</summary>" block).
In `@src/state_manager.py`:
- Around line 211-232: The truncation currently slices raw characters in
_truncate_result_log_body_to_fit which can leave unclosed headings, <details>
blocks, or code fences created by the generator in result_report.py and breaks
strip_result_log_section()/extract_result_log_body(); change the logic to parse
result_log_body into its logical "phase" sections using the same
delimiters/structure that result_report.py emits (e.g., the "####" phase
headings plus their following <details> and fenced code blocks), then
iteratively drop entire trailing phases until the remaining joined phases plus
the state comment scaffold (computed via _build_state_comment_body) fit within
max_length, and finally append truncation_notice; ensure the parser guarantees
balanced tags/fences so returned text never contains partial/unclosed tags or
fences and handle the case where no full phase can be kept by returning "".
---
Outside diff comments:
In `@src/auto_fixer.py`:
- Around line 343-373: The CI-fix exception handler currently doesn't persist
failed Claude outputs into result_blocks, so when run_claude_prompt (used in the
ci-fix phase) raises (e.g., ClaudeCommandFailedError / ClaudeUsageLimitError)
the stdout/stderr carried by the exception are lost; update the except block
around run_claude_prompt to detect if the caught exception exposes
stdout/stderr, create a phase result block via
build_phase_result_entry("ci-fix", stdout_or_stderr, ctx.state_comment_timezone)
and append it to result_blocks (merging with load_state_comment as done now)
before calling upsert_state_comment, and apply the same change to the equivalent
handlers for the other phases (the similar try/except regions referenced at the
other locations) so failed-run outputs are always saved to the state comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 855552f3-3791-48c2-b768-441d75aec516
📒 Files selected for processing (15)
.refix.yaml.sampleREADME.ja.mdREADME.mdsrc/auto_fixer.pysrc/claude_runner.pysrc/config.pysrc/report.pysrc/result_report.pysrc/state_manager.pytests/test_auto_fixer.pytests/test_claude_runner.pytests/test_config.pytests/test_report.pytests/test_result_report.pytests/test_state_manager.py
💤 Files with no reviewable changes (2)
- tests/test_report.py
- src/report.py
🤖 Refix Status対応済みレビュー一覧
実行レポートCI 修正2026-03-13 11:00:00 UTC src/auto_fixer.py CI make ci diff の修正開始CIの失敗原因: 2026-03-13 11:05:00 UTC src/auto_fixer.py CI修正完了4箇所の行長超過(88文字超)をruffフォーマッタが期待する形式に整形してcommit & push完了。
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/auto_fixer.py (1)
1167-1187:⚠️ Potential issue | 🟠 Major
_cacheable判定にstate_savedを含めてください。Line 1168-1180 で保存失敗時に
state_saved=Falseになるよう修正されていますが、Line 1229-1234 の_cacheableがstate_savedを見ていないため、結果ログ保存失敗でも cacheable 扱いになる可能性が残っています。修正案
- _cacheable = ( - not dry_run - and not bool(active_rate_limit) - and not bool(active_review_failed) - and not _ci_grace - ) + _cacheable = ( + not dry_run + and state_saved + and not bool(active_rate_limit) + and not bool(active_review_failed) + and not _ci_grace + )Also applies to: 1206-1214, 1229-1234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/auto_fixer.py` around lines 1167 - 1187, The _cacheable determination must consider whether the state result log was actually saved; update the code paths that compute the _cacheable flag (the block that currently sets _cacheable around the later section and the similar blocks near where state_saved is set) to include state_saved (i.e., make _cacheable false when state_saved is False). Locate the state save logic that sets state_saved (the try/except using load_state_comment and upsert_state_comment) and the subsequent _cacheable computation, and change the boolean expression for _cacheable to AND with state_saved (or explicitly set _cacheable = False when state_saved is False) so failed result-log saves never produce a cacheable result; apply the same change to the other similar occurrence mentioned (the blocks around lines 1206-1214 and 1229-1234).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/auto_fixer.py`:
- Around line 1167-1187: The _cacheable determination must consider whether the
state result log was actually saved; update the code paths that compute the
_cacheable flag (the block that currently sets _cacheable around the later
section and the similar blocks near where state_saved is set) to include
state_saved (i.e., make _cacheable false when state_saved is False). Locate the
state save logic that sets state_saved (the try/except using load_state_comment
and upsert_state_comment) and the subsequent _cacheable computation, and change
the boolean expression for _cacheable to AND with state_saved (or explicitly set
_cacheable = False when state_saved is False) so failed result-log saves never
produce a cacheable result; apply the same change to the other similar
occurrence mentioned (the blocks around lines 1206-1214 and 1229-1234).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ac6c1902-feff-4914-996f-940c89ec4f13
📒 Files selected for processing (1)
src/auto_fixer.py
…lse ブロック) 結果ログ保存失敗時(state_saved=False)でも cacheable 扱いになる問題を修正。
概要 / Summary
execution_report(Claude によるファイル書き込みレポート)を廃止し、Claude の stdout を直接 PR ステータスコメントに書き出すwrite_result_to_comment機能に置き換える。変更内容 / Changes
src/report.pyを削除し、src/result_report.pyを新規作成(format_phase_result_block,merge_result_log_body,build_phase_result_entry)src/config.py:execution_report: false(デフォルト)→write_result_to_comment: true(デフォルト)に差し替えsrc/claude_runner.py:report_path/report_enabledパラメータを削除、戻り値をstr→tuple[str, str](commits, stdout)に変更src/state_manager.py:report_body/REPORT_SECTION_*→result_log_body/RESULT_LOG_SECTION_*に置き換え、折りたたみ見出しを「実行レポート」→「実行ログ」に変更src/auto_fixer.py:PRContextからexecution_report_enabled/reports_dirを削除しwrite_result_to_commentを追加、各フェーズで stdout をresult_blocksに蓄積してupsert_state_comment(result_log_body=...)で保存、レビュー修正フェーズのみ対象コメント URL を付与.refix.yaml.sample/README.md/README.ja.md: 設定キーの説明を更新テスト / Testing
make ci通過済み(lint + 全 206 テスト)関連 issues / Related issues
なし
Summary by CodeRabbit
新機能
ドキュメント