feat(coderabbit): v2 canonical — fix retry-loop bug, autofix-first#16
Merged
Conversation
Incorporates forensic findings from ANcpLua.Analyzers PR #174 where CR
got stuck in a 3+ hour "Failed to post review comments" retry loop:
root cause: auto_apply_labels: true + many labeling_instructions
→ CR auto-labels the PR Nx
→ label-event triggers review re-fires
→ review POST hits GitHub's batch limit on large PRs
→ fails, auto-retries forever
Three fixes propagate to every repo via this canonical template:
1. disable_cache: true → false
Prior canonical disabled cache, forcing 5-30× cold-start slowdown on
assertive profile. Affected every repo synced from this template.
2. auto_apply_labels EXPLICIT false
suggested_labels stays on (recommendations in walkthrough) but CR
never mutates the PR's label set itself. Per-repo overrides that
need auto-application have to consciously flip this.
3. path_filters expanded (+9 entries)
snupkg/nupkg/sarif/trx/release-artifacts/Snapshots/AnalyzerReleases.*
excluded at FILTER level so CR doesn't have to walk megabyte-scale
generated files. Avoids the "review of 5.6 MB embedded JSON" failure
mode large-package repos hit.
Pro+ autofix-first additions:
4. finishing_touches.custom (5 generic recipes)
- autofix public XML doc / TSDoc gaps
- autofix sync-over-async / cancellation gaps
- autofix missing test coverage on changed public behavior
- autofix obvious simplifications on changed lines
- autofix security obvious-fixes (secrets, traversal, SQL concat, deser)
All repo-agnostic — no-op in repos where the patterns don't exist.
5. knowledge_base.code_guidelines.filePatterns (+8)
Directory.Build.props, Version.props, README.md, docs/**,
package.json, pnpm-workspace.yaml added so CR knows the framework
conventions every repo inherits.
Per-repo `.coderabbit.yaml` overrides (qyl, ANcpLua.Analyzers,
Qyl.OpenTelemetry.SemanticConventions) continue to extend this baseline
with repo-specific path_instructions + pre_merge_checks.custom_checks +
analyzer-discipline rules.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai autofix |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
Up to standards ✅🟢 Issues
|
ANcpLua
added a commit
to ANcpLua/ANcpLua.Analyzers
that referenced
this pull request
May 25, 2026
The local 695-line config in this repo (and its earlier v2 revision) was a full REPLACEMENT of the canonical baseline at ANcpLua/github-settings-automation/templates/coderabbit.yaml, not a merge — CR's per-repo config model is replace, not merge. After ANcpLua/github-settings-automation#16 lands and enforce-repo-settings.yml runs, this repo will receive the v2 canonical (which has the retry-loop fix that originally surfaced from this very PR's review failure). Analyzer-specific extensions (DiagnosticId ^AL\d{4}$ enforcement, AnalyzerReleases tracking, No ISourceGenerator check, per-path analyzer guidance) will land back as a focused per-repo override PR after the canonical sync, instead of being mixed in with this renumber PR. One source of truth > 100x local copies. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
This PR successfully updates the canonical CodeRabbit configuration to version 2, addressing the retry-loop bug by disabling automatic label application and improving performance through caching. All acceptance criteria, including the expansion of file filters and the addition of the five Pro+ custom autofix recipes, have been implemented.
Codacy analysis indicates the changes are up to standards. The primary concern identified during review relates to the 'sync-over-async / cancellation' autofix recipe; as currently written, it may encourage binary-breaking changes in .NET projects by modifying existing public method signatures. This should be refined to prefer method overloads for public APIs to maintain binary compatibility.
Test suggestions
- Verify YAML syntax is valid according to safe_load
- Verify reviews.auto_apply_labels is explicitly set to false
- Verify reviews.disable_cache is set to false
- Verify all 5 Pro+ autofix recipes are defined with correct instruction sets
- Verify path_filters excludes .nupkg, .snupkg, .sarif, and .trx files
- Verify knowledge_base includes Directory.Build.props and Directory.Packages.props
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
Comment on lines
+154
to
+159
| Where a public or internal async method lacks a CancellationToken | ||
| parameter, add `CancellationToken cancellationToken = default` and | ||
| thread it through. Where `.Result` / `.Wait()` / `.GetAwaiter().GetResult()` | ||
| blocks an async path, rewrite to await. Don't suppress with | ||
| `#pragma` — fix or leave a precise comment explaining why no fix | ||
| is possible. |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Adding a CancellationToken parameter to existing public methods is a binary-breaking change in .NET, even with a default value. While source-compatible for callers, it breaks binary compatibility and interface implementations.
For libraries requiring binary compatibility, it is safer to suggest an overload. Update the 'autofix sync-over-async / cancellation gaps' instructions to warn about binary-breaking changes and suggest that the agent prefer adding a method overload instead of just adding a parameter with a default value to existing public APIs.
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated CodeRabbit v2 YAML template configuration to prevent PR retry-loop risk by disabling automatic label application while keeping suggestions enabled. Expanded path filters to exclude generated and build artifacts. Added Pro+ finishing-touches recipes for common code improvements and extended knowledge-base patterns to recognize repository convention files. ChangesCodeRabbit Configuration Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@templates/coderabbit.yaml`:
- Around line 161-167: Update the "autofix missing test coverage on changed
public behavior" recipe so it infers the repo's existing test location before
hardfalling to "tests/" or "test/": scan the repo for common test
directories/patterns (e.g., tests/, test/, spec/, __tests__/, *.Tests/,
src/test/, tests/**/*, framework-specific folders) and pick the dominant
convention, use that path when generating new tests, and only fallback to
"tests/" or "test/" if no existing convention is found; make the detection logic
configurable or data-driven in the template so the recipe (instructions field)
references the detected testDir when emitting files.
- Around line 320-335: The filePatterns list in templates/coderabbit.yaml only
includes 'README.md' (root README) so per-package/nested README files are
excluded; update the filePatterns array (the entry that currently lists
'README.md') to also include a glob like '**/README.md' so nested package
READMEs are picked up into code_guidelines/knowledge base (modify the
filePatterns block where 'README.md' is declared).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 540663a7-44e6-420d-a0ec-8a1d03cc10f8
📒 Files selected for processing (1)
templates/coderabbit.yaml
📜 Review details
🔇 Additional comments (1)
templates/coderabbit.yaml (1)
152-159: Don't teach the autofixer to rewrite existing public async signatures in place.This recipe still instructs the agent to append
CancellationToken cancellationToken = defaultto existing public APIs. In .NET that is a binary-breaking change and can also invalidate interface or override contracts, so the safe default is “prefer an overload, or skip contract-bound members.”In .NET/C#, is adding an optional `CancellationToken cancellationToken = default` parameter to an existing public method a binary-breaking change? Does it also affect interface implementations and overrides?
Comment on lines
+161
to
+167
| name: autofix missing test coverage on changed public behavior | ||
| instructions: >- | ||
| For each new public function/class/endpoint added in this PR with | ||
| no corresponding test under `tests/` or `test/`, generate a focused | ||
| test using the repo's existing test stack (xUnit v3, vitest, pytest, | ||
| etc.). Cover the positive case + at least one negative case. Don't | ||
| generate snapshot tests unless the repo already uses them. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
The test autofix recipe is not actually repo-agnostic.
Hardcoding tests/ and test/ means repos that keep tests in *.Tests/, spec/, or framework-specific folders will be treated as uncovered and may get generated files in the wrong place. Infer the existing test location first, then fall back only when the repo has no established convention.
Suggested fix
- enabled: true
name: autofix missing test coverage on changed public behavior
instructions: >-
For each new public function/class/endpoint added in this PR with
- no corresponding test under `tests/` or `test/`, generate a focused
- test using the repo's existing test stack (xUnit v3, vitest, pytest,
- etc.). Cover the positive case + at least one negative case. Don't
- generate snapshot tests unless the repo already uses them.
+ no corresponding test in the repo's existing test location, generate
+ a focused test using the repo's existing test stack and file layout.
+ Infer the canonical test directory or test project from nearby tests
+ first; only fall back to `tests/` or `test/` if the repo has no
+ established convention. Cover the positive case + at least one
+ negative case. Don't generate snapshot tests unless the repo already
+ uses them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/coderabbit.yaml` around lines 161 - 167, Update the "autofix
missing test coverage on changed public behavior" recipe so it infers the repo's
existing test location before hardfalling to "tests/" or "test/": scan the repo
for common test directories/patterns (e.g., tests/, test/, spec/, __tests__/,
*.Tests/, src/test/, tests/**/*, framework-specific folders) and pick the
dominant convention, use that path when generating new tests, and only fallback
to "tests/" or "test/" if no existing convention is found; make the detection
logic configurable or data-driven in the template so the recipe (instructions
field) references the detected testDir when emitting files.
Comment on lines
+320
to
+335
| # v2 EXPAND: include build infra + per-package READMEs so CR knows the | ||
| # framework conventions every repo inherits. | ||
| filePatterns: | ||
| - 'CLAUDE.md' | ||
| - 'AGENTS.md' | ||
| - '**/CLAUDE.md' | ||
| - '**/AGENTS.md' | ||
| - '.editorconfig' | ||
| - 'README.md' | ||
| - 'docs/**/*.md' | ||
| - 'Directory.Build.props' | ||
| - 'Directory.Packages.props' | ||
| - 'global.json' | ||
| - 'Version.props' | ||
| - 'package.json' | ||
| - 'pnpm-workspace.yaml' |
There was a problem hiding this comment.
Per-package READMEs are still not included in code_guidelines.
Line 328 only matches the repository-root README.md, so nested package READMEs never feed the knowledge base. That contradicts the comment on Lines 320-321 and drops conventions from monorepos with package-level docs.
Suggested fix
filePatterns:
- 'CLAUDE.md'
- 'AGENTS.md'
- '**/CLAUDE.md'
- '**/AGENTS.md'
- '.editorconfig'
- 'README.md'
+ - '**/README.md'
- 'docs/**/*.md'
- 'Directory.Build.props'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/coderabbit.yaml` around lines 320 - 335, The filePatterns list in
templates/coderabbit.yaml only includes 'README.md' (root README) so
per-package/nested README files are excluded; update the filePatterns array (the
entry that currently lists 'README.md') to also include a glob like
'**/README.md' so nested package READMEs are picked up into
code_guidelines/knowledge base (modify the filePatterns block where 'README.md'
is declared).
ANcpLua
added a commit
to ANcpLua/ANcpLua.Analyzers
that referenced
this pull request
May 25, 2026
* feat(analyzers): renumber to 9 domain bands, 2.0.0 break 89 ids were sprawled across AL0001..AL0139 with sparse gaps (~64% density, big holes at AL0061-0079, AL0085-0093, AL0096-0100, AL0131-0136). Renumbered to 9 100-wide bands in AL1000..AL1899 — deliberately skipping AL0xxx because sibling packages already occupy that space: ANcpLua.AotReflection AL0097..0100 ANcpLua.ExtensibleEnumMirror AL0200..0202 ANcpLua.DiscriminatedUnion AL0300..0303 Bands (full mapping at eng/analyzer-renumber-plan.md): AL1000..1099 Correctness / language pitfalls (13 rules) AL1100..1199 ASP.NET Core (10) AL1200..1299 Roslyn Utilities (21) AL1300..1399 Async / threading / reliability (15) AL1400..1499 AOT / trim (10) AL1500..1599 Roslyn-author hygiene (6) AL1600..1699 Package / version (7) AL1700..1799 Style (4) AL1800..1899 Agent governance ([LoomTool]) (3) Reserved AL0xxx for sibling-package use; reserved AL1900..1999 for future bands. Each band leaves ~79 free slots — no future re-renumber required. Wave delivered by a parallel agent team: planner (488-line plan with 9-band scheme + 89→89 deterministic mapping + cross-team migration scan), renumber executor (186 git mv + 2,489 substitutions), README author (single-screen ANcpLua.Agents style, 89-row table grouped by band), consumer rewirer (330 substitutions across 4 sibling repos), cleanup (132 reorder ops + 11 stale-id deletions). Side cleanup: 44 orphan AL resx prefix groups deleted (~132 keys + ~117 Designer entries). Those were the OTel-shaped rules that previously migrated to Qyl.OpenTelemetry.SemanticConventions.Analyzers — forensic evidence of the cross-package migration that no consumer depends on. Cross-team migration candidates flagged: zero. Pure ANcpLua-internal renumber. ASP.NET Core rules (AL1105-1108) are resilience/health-check contracts, not OTel semconv; GenAI rules (AL1800-1802) enforce [LoomTool] agent governance, not gen_ai.* semconv. Verified locally: 0/0 build, 757/757 tests pass, dotnet pack src/ANcpLua.Analyzers/ANcpLua.Analyzers.csproj -p:PackageId=ANcpLua.Analyzers -p:Version=2.0.0 produces the canonical nupkg (per nuget-publish.yml). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(coderabbit): rewrite .coderabbit.yaml — fix retry-loop bug, autofix-first Root cause for PR #174's CodeRabbit getting stuck (3+ hours in "Review in progress" / "Failed to post review comments" loop): auto_apply_labels: true + 17 labeling_instructions → CR labeled the PR 9× → 8 "Review skipped" dedup'd + 1 leaked through → review-comments POST exceeded GitHub's batch size on the 215-file diff → "Failed to post review comments" → auto-retry forever Fixes in this rewrite: 1. auto_apply_labels: true → false (PR labels never trigger event re-fires) 2. labeling_instructions: 17 → 5 (high-signal labels only) 3. disable_cache: true → false (5-30× CR speedup) 4. path_instructions: 17 → 7 (dedup'd via multi-glob arrays; same coverage) 5. linked_repositories: 17 → 7 (only truly-related repos) 6. path_filters: +9 entries (Designer.cs, Snapshots, AnalyzerReleases.md, etc. excluded at filter level instead of instruction level) 7. finishing_touches.custom: 5 recipes (rewritten review-prose → autofix-first actions) 8. pre_merge_checks.custom_checks: 12 → 18 (added 3 analyzer-discipline checks the renumber wave proved necessary) Net delta: 695 → ~520 lines, prompt size massively smaller, autofix posture stronger. Should let CR actually complete on big PRs without hitting GitHub's review-comment batch limit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: apply CodeRabbit auto-fixes Fixed 35 file(s) based on 40 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * Revert "fix(coderabbit): rewrite .coderabbit.yaml — fix retry-loop bug, autofix-first" This reverts commit e5fdd5b. * chore(coderabbit): delete local .coderabbit.yaml — canonical owns it The local 695-line config in this repo (and its earlier v2 revision) was a full REPLACEMENT of the canonical baseline at ANcpLua/github-settings-automation/templates/coderabbit.yaml, not a merge — CR's per-repo config model is replace, not merge. After ANcpLua/github-settings-automation#16 lands and enforce-repo-settings.yml runs, this repo will receive the v2 canonical (which has the retry-loop fix that originally surfaced from this very PR's review failure). Analyzer-specific extensions (DiagnosticId ^AL\d{4}$ enforcement, AnalyzerReleases tracking, No ISourceGenerator check, per-path analyzer guidance) will land back as a focused per-repo override PR after the canonical sync, instead of being mixed in with this renumber PR. One source of truth > 100x local copies. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: apply CodeRabbit auto-fixes Fixed 5 file(s) based on 40 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * fix(build): revert CR autofix bump of ANcpLuaAnalyzersVersion Commit 2fe348d (CR autofix bot, 2026-05-25) bumped Version.props line <ANcpLuaAnalyzersVersion> from 1.29.4 -> 2.0.0. That value drives the SDK's GlobalPackageReference self-injection (the analyzer references itself as an analyzer via the SDK template). Asking for >= 2.0.0 failed NU1102 because ANcpLua.Analyzers 2.0.0 doesn't exist on nuget.org yet (it's the version THIS PR is producing). CR autofix conflated VersionPrefix (the version BUILT by this PR) with ANcpLuaAnalyzersVersion (the version CONSUMED for self-injection from nuget.org) despite the in-file comment on Directory.Build.props warning against exactly that pattern. Restore 1.29.4 (last-published) and add an inline INVARIANT comment so future autofix-style passes can't repeat the mistake. After this PR publishes 2.0.0 to nuget.org, a follow-up PR can bump this value to 2.0.0 — but never before. This incident is what got CR uninstalled across the orgs. See https://github.com/ANcpLua/github-settings-automation/blob/main/qyl-PRD.md Appendix 3 for the full freeze rationale + re-enable runbook. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(codefixes): undo CR autofix breakage on AL1201 + restore missing AL1010 resx The earlier CR autofix commit 5e81267 made two changes that broke the build after the renumber: 1. AL1201UseHasAttributeCodeFixProvider — switched base class from CodeFixProvider to AlCodeFixProvider<InvocationExpressionSyntax>, but kept the existing GetFixAllProvider / RegisterCodeFixesAsync overrides. The base class has those methods `sealed override`, so the result was CS0239 + CS0534. Revert the inheritance to CodeFixProvider; the actual refactor to the AL base class is a separate non-trivial change (need to implement CreateCodeAction and delete the existing overrides) that doesn't belong in a build-break-fix PR. 2. AL1010PatternMatchingCodeFixProvider — title argument changed from the hardcoded string "Use pattern matching" to CodeFixResources.AL1003CodeFixTitle (an unrelated AL1003 resx key). A later renumber-aware pass rewrote AL1003 -> AL1010, but the AL1010CodeFixTitle resx entry doesn't exist (only AL1009 / AL1011 do). Add the AL1010CodeFixTitle resx entry with the original "Use pattern matching" text + the matching Designer.cs property. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
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
Fixes the CodeRabbit retry-loop bug observed on ANcpLua.Analyzers PR #174 (CR stuck 3+ hours in "Failed to post review comments" loop) + adds Pro+ autofix-first recipes that propagate to every repo via this canonical sync.
Root cause forensics (PR #174)
Three structural fixes
disable_cachetruefalse(5-30× CR speedup on assertive)auto_apply_labelsfalse(prevents label-event review re-fires)path_filtersTwo Pro+ additions
finishing_touches.custom: 5 autofix-first recipes (XML docs, async, tests, simplifications, security obvious-fixes)knowledge_base.code_guidelines.filePatterns: +8 framework filesTest plan
enforce-repo-settings.yml(workflow_dispatch) to sync the new canonical into all reposAfter merge
enforce-repo-settings.ymlruns weekly Mon 06:00 UTC OR can be manually triggered. Per-repo.coderabbit.yamloverrides (qyl + analyzer repos) remain untouched — they extend this baseline.🤖 Generated with Claude Code