Merged
Conversation
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) | ⏱️ ~45分 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 |
![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
🤖 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 2051-2104: The new "grace" time check (using
ci_empty_grace_minutes, head_sha, commit_dt, elapsed) can flip a PR from
False->True purely by time, but process_repo()'s skip uses only the cached
updatedAt so a PR handled once during the grace window will be cached and never
retried when the grace expires; fix by excluding the grace-only path from being
treated as a final skip: either (A) do not update/cache updatedAt when you
return False due solely to the grace check (i.e. when elapsed <
timedelta(minutes=ci_empty_grace_minutes)), or (B) change the function to return
a retry timestamp (next_retry = commit_dt +
timedelta(minutes=ci_empty_grace_minutes)) alongside the False result and have
process_repo() respect that retry time instead of treating the PR as fully
processed; locate the logic around ci_empty_as_success, head_result/head_sha,
commit_result/date_str and the elapsed calculation to implement this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fafec0e9-72c3-470a-92f9-34812fda8dbe
📒 Files selected for processing (3)
.refix.yaml.samplesrc/auto_fixer.pytests/test_auto_fixer.py
_are_all_ci_checks_successful がグレース期間中に None を返すよう変更し、 _update_done_label_if_completed が (label_updated, ci_grace_pending) の タプルを返すよう変更。ci_grace_pending が True の場合、呼び出し元の _process_single_pr が cacheable=False を設定するため、次回実行時に updatedAt が変わっていなくても PR が再チェックされる。
Owner
Author
🤖 Refix Status対応済みレビュー一覧
実行レポートCI 修正2026-03-12 21:00:00 UTC src/pr_reviewer.py / tests/test_auto_fixer.py 原因調査4 テストすべて
修正方針:
2026-03-12 21:05:00 UTC src/pr_reviewer.py / tests/test_auto_fixer.py 修正完了・push変更内容:
結果: レビュー修正2026-03-12 20:45:21 UTC src/auto_fixer.py レビューコメント確認開始コードの現状を確認した結果:
未修正の箇所 (要対応):
2026-03-12 20:46:00 UTC src/auto_fixer.py check-runs 失敗時の return None 修正修正内容r3939591452 の残課題として、以下3箇所を
コミット: 既修正の確認他の指摘 (head SHA 取得失敗, ci_empty_grace_minutes バリデーション, テストの side_effect 修正) は以前のコミットで対応済みだった。 |
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 2057-2109: The code currently treats transient GitHub API failures
as permanent by returning False in the head sha and commit date retrieval
blocks; update the handlers around head_result and commit_result in
auto_fixer.py (the blocks that print "CI checks unavailable for PR #{pr_number};
skip refix:done labeling.") to return None instead of False so callers treat
these as transient (retryable) and do not cache updatedAt; leave the existing
None return used for the recent-commit grace period (elapsed <
timedelta(minutes=ci_empty_grace_minutes)) unchanged.
- Around line 483-505: The ci_empty_grace_minutes parsing currently uses int()
which silently truncates floats; change the validation in the
ci_empty_grace_minutes handling block so only true integers are accepted: reject
float types and strings with decimal points, accept ints (but not bool), or
digit-only strings (e.g. using str.isdigit() or an integer regex) and then
convert to int into grace_int, preserving the negative check and the error
messages; ensure you still set config["ci_empty_grace_minutes"] = grace_int only
after this stricter validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 35b1d350-8765-4deb-ba6c-f355e18b245f
📒 Files selected for processing (2)
src/auto_fixer.pytests/test_auto_fixer.py
Combine PR #73's ci_empty_as_success/ci_empty_grace_minutes params and grace-period logic with origin/main's REST API (Fine-grained PAT) refactor. The empty-CI grace period block is now placed after REST API check runs are fetched, reusing the already-computed head_sha.
GitHub API の一時失敗時に False を返すと updatedAt キャッシュに登録され 次の PR 更新まで refix:done 再判定が止まる問題を修正。 head_result と commit_result のハンドラーを None (再試行) に変更。
int() が小数を黙って切り捨てる問題を修正。 float 型と小数点を含む文字列を明示的に reject し、 真の整数 (bool 除外) または数字のみの文字列のみ受け付けるよう検証を厳格化。
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/auto_fixer.py (2)
2039-2044:⚠️ Potential issue | 🟠 Major一時的な CI 取得失敗を
Falseに潰すと再試行不能になりますここで
Falseを返すと、呼び出し側でupdatedAtがキャッシュされ、同一更新の PR が再判定されません。head SHA 取得失敗・check-runs 取得失敗・commit date 取得/parse 失敗は、再試行可能状態としてNoneを返すべきです。🛠 修正例（再試行可能エラーは `None`）
if head_result.returncode != 0 or not ( head_sha := (head_result.stdout or "").strip() ): print(f"CI checks unavailable for PR #{pr_number}; skip refix:done labeling.") - return False + return None @@ if result.returncode != 0: print(f"CI checks unavailable for PR #{pr_number}; skip refix:done labeling.") - return False + return None @@ if commit_result.returncode != 0 or not ( date_str := (commit_result.stdout or "").strip() ): print( f"CI checks unavailable for PR #{pr_number}; skip refix:done labeling." ) - return False + return None @@ except (ValueError, TypeError): print( f"CI checks unavailable for PR #{pr_number}; skip refix:done labeling." ) - return False + return NoneAlso applies to: 2053-2056, 2093-2099, 2114-2118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/auto_fixer.py` around lines 2039 - 2044, The code currently returns False on transient CI/commit-data failures (e.g., when head_result.returncode != 0 or head_sha is empty around the head_result/ head_sha check referencing head_result and pr_number), which prevents retries; change these failure-return paths to return None instead of False so the caller treats them as retryable. Apply the same change to the other similar blocks mentioned (the check-runs retrieval block referencing check_runs/check_results, the commit date retrieval/parse block referencing commit_date/commit_result, and the later blocks around 2053-2056 and 2093-2099/2114-2118) so any temporary failures yield None; ensure callers still distinguish None (retryable) from False (definitively handled).
484-506:⚠️ Potential issue | 🟠 Major
ci_empty_grace_minutesが小数値を切り捨て受理しています
int()変換により0.5 -> 0,"2.9" -> 2が通るため、grace 設定が意図より短くなります。ここは整数のみを受け付けるべきです。🛠 修正例（整数のみ受理）
ci_empty_grace_minutes = parsed.get("ci_empty_grace_minutes") if ci_empty_grace_minutes is not None: - if isinstance(ci_empty_grace_minutes, bool): + if isinstance(ci_empty_grace_minutes, bool): print( "Error: ci_empty_grace_minutes must be a non-negative integer.", file=sys.stderr, ) sys.exit(1) - try: - grace_int = int(ci_empty_grace_minutes) - except (TypeError, ValueError): + if isinstance(ci_empty_grace_minutes, int): + grace_int = ci_empty_grace_minutes + elif ( + isinstance(ci_empty_grace_minutes, str) + and ci_empty_grace_minutes.strip().isdigit() + ): + grace_int = int(ci_empty_grace_minutes.strip()) + else: print( "Error: ci_empty_grace_minutes must be a non-negative integer.", file=sys.stderr, ) sys.exit(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/auto_fixer.py` around lines 484 - 506, The current parsing accepts floats because it uses int() which truncates; change validation around ci_empty_grace_minutes so only true integers are accepted: if ci_empty_grace_minutes is an int (but not bool) accept it; reject booleans and floats (e.g., isinstance(..., float)); if it's a string, only accept strings that match an integer pattern (e.g., only digits, no decimal point or sign) and then convert to int; otherwise print the same error and exit; finally set config["ci_empty_grace_minutes"] = int_value. Ensure you update the checks around the parsed variable name ci_empty_grace_minutes and the assignment to config["ci_empty_grace_minutes"] accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_auto_fixer.py`:
- Around line 2538-2601: テストのモックは関数 _are_all_ci_checks_successful の実際の gh api
呼び出し順（head SHA → check-runs → commit date）と合っていないため壊れています。対象は
TestAreAllCiChecksSuccessful クラスの各テストで、mocking を mock_run.return_value のままにせず
subprocess.run を side_effect にして呼び出し毎の戻り値を順序通り設定し、最終的に呼び出し回数のアサーションを
assert_called_once() から mock_run.call_count == N へ修正するか、個別の side_effect
配列に合わせて期待値を検証してください。該当シンボル: _are_all_ci_checks_successful,
TestAreAllCiChecksSuccessful, subprocess.run, mock_run.side_effect を使って gh pr
checks / gh api head sha / gh api commit date の順で戻り値を返すように修正してください。
---
Duplicate comments:
In `@src/auto_fixer.py`:
- Around line 2039-2044: The code currently returns False on transient
CI/commit-data failures (e.g., when head_result.returncode != 0 or head_sha is
empty around the head_result/ head_sha check referencing head_result and
pr_number), which prevents retries; change these failure-return paths to return
None instead of False so the caller treats them as retryable. Apply the same
change to the other similar blocks mentioned (the check-runs retrieval block
referencing check_runs/check_results, the commit date retrieval/parse block
referencing commit_date/commit_result, and the later blocks around 2053-2056 and
2093-2099/2114-2118) so any temporary failures yield None; ensure callers still
distinguish None (retryable) from False (definitively handled).
- Around line 484-506: The current parsing accepts floats because it uses int()
which truncates; change validation around ci_empty_grace_minutes so only true
integers are accepted: if ci_empty_grace_minutes is an int (but not bool) accept
it; reject booleans and floats (e.g., isinstance(..., float)); if it's a string,
only accept strings that match an integer pattern (e.g., only digits, no decimal
point or sign) and then convert to int; otherwise print the same error and exit;
finally set config["ci_empty_grace_minutes"] = int_value. Ensure you update the
checks around the parsed variable name ci_empty_grace_minutes and the assignment
to config["ci_empty_grace_minutes"] accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 761c2ef6-86f6-49e3-a7b4-49b8a0415d08
📒 Files selected for processing (2)
src/auto_fixer.pytests/test_auto_fixer.py
…クを現行 REST API 呼び出し順序に修正 - pr_reviewer.py: json.loads 結果が dict 以外の場合に AttributeError が発生していたため isinstance(data, dict) ガードを追加 - test_auto_fixer.py: TestAreAllCiChecksSuccessful の 4 テストを head SHA → check-runs → classic statuses → [commit date] の呼び出し順序に合わせてモックと call_count アサーションを更新
check-runs の API 呼び出し失敗 (returncode != 0) および JSON parse 失敗は 再試行可能な一時障害であるため、False ではなく None を返すよう修正する。 呼び出し側が updatedAt をキャッシュして同一PRの再判定が行われない問題を解消する。
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
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
CI チェックが空の PR を成功扱いにする設定を追加。最後のコミットから一定時間経過後に「CI なし」と判断し、refix:done と自動マージを許可する。
変更内容 / Changes
.refix.yamlにci_empty_as_success（デフォルト true）とci_empty_grace_minutes（デフォルト 5）を追加ci_empty_as_success: trueかつ check_runs が空のとき、head コミットの日時を取得し、経過がci_empty_grace_minutes分以上なら成功扱いci_empty_as_success: falseのときは従来どおり空を常に unavailable 扱い_are_all_ci_checks_successfulに新パラメータを追加し、process_repo→_process_single_pr→_update_done_label_if_completedへ伝播TestAreAllCiChecksSuccessfulを追加（ci_empty_as_success false / 経過時間による判定のテスト）テスト / Testing
make ciで全テストパス関連 issues / Related issues
なし / none
Summary by CodeRabbit
新機能
バグ修正
テスト