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)
📝 WalkthroughWalkthroughPRはPRの 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)
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 3133-3147: The current flow writes updatedAt for any PR added to
modified_prs, which allows incomplete/failed PR processing paths to mark a PR as
safe-to-skip; change _process_single_pr() to return a tuple (or object)
including a boolean cacheable flag (e.g., cacheable) alongside existing results,
then update the loop that handles modified_prs and the call site that currently
calls set_cached_updated_at() so that set_cached_updated_at(updated_at_cache,
repo, pr_number_raw, current_updated_at) is invoked only when cacheable is true
(and updated_at_cache is not None); also ensure
get_cached_updated_at(updated_at_cache, repo, pr_number_raw) and the early-skip
check using current_updated_at == cached_updated_at remain unchanged so only
truly cacheable successes are written to the cache.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 76f5e3ff-5fac-4165-99a3-1a1884566f03
📒 Files selected for processing (5)
src/auto_fixer.pysrc/cache_manager.pytests/test_auto_fixer.pytests/test_cache_manager.pytests/test_imports.py
_process_single_pr now returns a cacheable flag (4th value). The caller only calls set_cached_updated_at when cacheable=True, which is limited to paths where processing fully completed (normal end or clean no-op). Paths where processing was skipped or incomplete (CodeRabbit rate limit active, review failed active, commit/claude limit reached, fetch/setup errors) all return cacheable=False, preventing next-run early-skip on unprocessed PRs.
Owner
Author
🤖 Refix Status対応済みレビュー一覧
|
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 2703-2706: The code currently sets _cacheable without considering
dry_run and also returns cacheable=True unconditionally on the review-fix path;
update both places so that when dry_run is True you never return cacheable=True.
Specifically, modify the _cacheable calculation (symbol: _cacheable) to include
a check for dry_run (e.g., _cacheable = not dry_run and not
bool(active_rate_limit) and not bool(active_review_failed)), and change the
return in the review-fix path (the unconditional return at lines around
3000–3002) to ensure it returns cacheable=False when dry_run is True.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 69b2851b-75a0-4fbe-b2ab-a7111fa7c3cc
📒 Files selected for processing (1)
src/auto_fixer.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/auto_fixer.py (1)
3256-3260:⚠️ Potential issue | 🔴 Critical失敗・未完了の PR でも
cacheable=Trueが返っていますLine 3257 は
not dry_runしか見ていないため、subprocess.CalledProcessErrorでreview_fix_failed=Trueになったケースや、state comment / thread resolve が完了せずstate_saved=Falseのケースでも、Line 3455 でupdatedAtが保存されます。次回同じupdatedAtのままだと未処理 PR を早期スキップしてしまいます。🐛 修正案
- _cacheable = not dry_run + _cacheable = ( + not dry_run + and state_saved + and not review_fix_failed + and not bool(active_rate_limit) + and not bool(active_review_failed) + ) if commits_by_phase: return False, True, (repo, pr_number, "\n".join(commits_by_phase)), _cacheable return False, True, None, _cacheable🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/auto_fixer.py` around lines 3256 - 3260, The code sets _cacheable = not dry_run unconditionally which marks failed or incomplete fixes as cacheable; change the logic so _cacheable is true only when not dry_run AND the fix succeeded and state was persisted — i.e., compute _cacheable = (not dry_run) and (not review_fix_failed) and (state_saved is True) and (commits_by_phase is empty or indicates success), so update the branch that returns (False, True, ...) to derive _cacheable from review_fix_failed and state_saved (and any commits_by_phase success indicator) before returning to avoid saving updatedAt for failed/incomplete PRs; adjust references to modified_prs.add, _cacheable, commits_by_phase, review_fix_failed, state_saved and updatedAt 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 `@src/auto_fixer.py`:
- Around line 3631-3635: In the finally block around
save_cache(updated_at_cache) only persist the cache when the run is actually
allowed to change state: check the dry-run and summarize-only flags (e.g.
args.dry_run or summarize_only) and also verify updated_at_cache is non-empty
and different from the cache returned by load_cache() (capture the original
loaded cache into a variable when calling load_cache()). Replace the
unconditional save_cache call with a guard that returns early for
dry-run/summarize-only or when there is no meaningful change to avoid
overwriting a valid existing cache.
---
Duplicate comments:
In `@src/auto_fixer.py`:
- Around line 3256-3260: The code sets _cacheable = not dry_run unconditionally
which marks failed or incomplete fixes as cacheable; change the logic so
_cacheable is true only when not dry_run AND the fix succeeded and state was
persisted — i.e., compute _cacheable = (not dry_run) and (not review_fix_failed)
and (state_saved is True) and (commits_by_phase is empty or indicates success),
so update the branch that returns (False, True, ...) to derive _cacheable from
review_fix_failed and state_saved (and any commits_by_phase success indicator)
before returning to avoid saving updatedAt for failed/incomplete PRs; adjust
references to modified_prs.add, _cacheable, commits_by_phase, review_fix_failed,
state_saved and updatedAt accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fb70c923-203a-4c75-bf43-8683b38e8de4
📒 Files selected for processing (2)
src/auto_fixer.pytests/test_auto_fixer.py
…acheable in review-fix path failure/incomplete PR でも cacheable=True が返っていた問題を修正。 review_fix_failed や state_saved=False のケースで次回実行時に未処理 PR を 早期スキップしてしまうのを防ぐ。
finally ブロックの save_cache を dry_run / summarize_only 時にスキップし、 非破壊モードの実行で既存キャッシュを上書きしないようにする。
HappyOnigiri
added
the
refix:auto-merge-requested
HappyOnigiri
deleted the
cursor/-bc-cd6e9f52-7c6c-4663-a9a5-9911e814570d-763a
branch
HappyOnigiri
added
refix:merged
This was referenced Mar 12, 2026
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
GitHub API呼び出し削減と実行時間短縮のため、ローカルキャッシュによるPR早期スキップ機能を追加。
変更内容 / Changes
src/cache_manager.pyを新規追加し、.refix_cache.jsonの読み書き、厳格なスキーマ検証、破損時の回復処理を実装。src/auto_fixer.pyにキャッシュのロード・保存ロジックを追加。process_repo内で、PRのupdatedAtがキャッシュと一致する場合、詳細取得やレビュー処理をスキップする早期リターンを導入。テスト / Testing
src/cache_manager.pyの単体テストで、正常なI/O、ファイル未存在、JSON破損、スキーマ不一致時の挙動を確認。src/auto_fixer.pyのオーケストレーションテストで、updatedAt一致時の早期スキップ、不一致時の詳細処理、処理後のキャッシュ更新が正しく行われることを確認。make ciで全体的な回帰テストを実施。関連 issues / Related issues
なし
Summary by CodeRabbit